diff --git a/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcSpanDecorator.java b/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcSpanDecorator.java index 085a5613..74f3a91f 100644 --- a/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcSpanDecorator.java +++ b/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcSpanDecorator.java @@ -21,7 +21,6 @@ import io.grpc.Metadata.Key; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; -import io.opentelemetry.javaagent.instrumentation.hypertrace.com.google.protobuf.util.JsonFormat; import io.opentelemetry.javaagent.instrumentation.hypertrace.grpc.GrpcSemanticAttributes; import java.util.LinkedHashMap; import java.util.Map; @@ -35,16 +34,17 @@ public class GrpcSpanDecorator { private GrpcSpanDecorator() {} private static final Logger log = LoggerFactory.getLogger(GrpcSpanDecorator.class); - private static final JsonFormat.Printer PRINTER = JsonFormat.printer(); public static void addMessageAttribute(Object message, Span span, AttributeKey key) { if (message instanceof Message) { Message mb = (Message) message; try { String jsonOutput = ProtobufMessageConverter.getMessage(mb); - span.setAttribute(key, jsonOutput); + if (jsonOutput != null && !jsonOutput.isEmpty()) { + span.setAttribute(key, jsonOutput); + } } catch (Exception e) { - log.error("Failed to decode message as JSON", e); + log.debug("Failed to decode message as JSON: {}", e.getMessage(), e); } } else { log.debug("message is not an instance of com.google.protobuf.Message"); diff --git a/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/ProtobufMessageConverter.java b/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/ProtobufMessageConverter.java index bdd991e5..fda4ca1a 100644 --- a/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/ProtobufMessageConverter.java +++ b/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/ProtobufMessageConverter.java @@ -25,11 +25,14 @@ import io.opentelemetry.javaagent.instrumentation.hypertrace.com.google.protobuf.util.JsonFormat; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** Utility class to convert protobuf messages to JSON. */ public class ProtobufMessageConverter { private static final Logger log = LoggerFactory.getLogger(ProtobufMessageConverter.class); private static final Map fileDescriptorCache = new HashMap<>(); @@ -75,25 +78,73 @@ private static Descriptor getRelocatedDescriptor(Descriptors.Descriptor original String fileKey = unrelocatedFileDescriptor.getName(); if (fileDescriptorCache.containsKey(fileKey)) { FileDescriptor relocatedFileDescriptor = fileDescriptorCache.get(fileKey); - return relocatedFileDescriptor.findMessageTypeByName(originalDescriptor.getName()); + + // Check if the message type exists in the relocated descriptor + Descriptor messageType = + relocatedFileDescriptor.findMessageTypeByName(originalDescriptor.getName()); + if (messageType == null) { + log.debug("Message type not found in cached descriptor: {}", originalDescriptor.getName()); + } + + return messageType; } - // Process all dependencies first + // Process all dependencies recursively, including transitive ones + FileDescriptor fileDescriptor = + processFileDescriptorWithDependencies(unrelocatedFileDescriptor, new HashSet<>()); + + // Find the message type in the relocated descriptor + Descriptor result = fileDescriptor.findMessageTypeByName(originalDescriptor.getName()); + if (result == null) { + log.debug("Message type not found in new descriptor: {}", originalDescriptor.getName()); + } + + return result; + } + + /** + * Process a file descriptor and all its dependencies recursively. + * + * @param unrelocatedFileDescriptor The file descriptor to process + * @param processedFiles Set of file names that have already been processed to avoid circular + * dependencies + * @return The relocated file descriptor + */ + private static FileDescriptor processFileDescriptorWithDependencies( + Descriptors.FileDescriptor unrelocatedFileDescriptor, Set processedFiles) + throws Exception { + String fileKey = unrelocatedFileDescriptor.getName(); + + // Check if we've already processed this file + if (fileDescriptorCache.containsKey(fileKey)) { + return fileDescriptorCache.get(fileKey); + } + + // Mark this file as processed to avoid circular dependencies + processedFiles.add(fileKey); + List dependencies = new ArrayList<>(); + + // Process all direct dependencies first for (Descriptors.FileDescriptor dependency : unrelocatedFileDescriptor.getDependencies()) { String depKey = dependency.getName(); - if (!fileDescriptorCache.containsKey(depKey)) { - // Convert the dependency file descriptor - com.google.protobuf.DescriptorProtos.FileDescriptorProto depProto = dependency.toProto(); - byte[] depBytes = depProto.toByteArray(); - FileDescriptorProto relocatedDepProto = FileDescriptorProto.parseFrom(depBytes); - // Build with empty dependencies first (we'll fill them in later) + // Skip if we've already processed this dependency in this call chain + if (processedFiles.contains(depKey)) { + if (fileDescriptorCache.containsKey(depKey)) { + dependencies.add(fileDescriptorCache.get(depKey)); + } + continue; + } + + if (!fileDescriptorCache.containsKey(depKey)) { + // Process this dependency recursively FileDescriptor relocatedDep = - FileDescriptor.buildFrom(relocatedDepProto, new FileDescriptor[] {}); - fileDescriptorCache.put(depKey, relocatedDep); + processFileDescriptorWithDependencies(dependency, processedFiles); + dependencies.add(relocatedDep); + } else { + dependencies.add(fileDescriptorCache.get(depKey)); } - dependencies.add(fileDescriptorCache.get(depKey)); } // Now build the current file descriptor with its dependencies @@ -106,7 +157,7 @@ private static Descriptor getRelocatedDescriptor(Descriptors.Descriptor original FileDescriptor.buildFrom(relocatedFileProto, dependencies.toArray(new FileDescriptor[0])); fileDescriptorCache.put(fileKey, relocatedFileDescriptor); - return relocatedFileDescriptor.findMessageTypeByName(originalDescriptor.getName()); + return relocatedFileDescriptor; } /** @@ -114,28 +165,21 @@ private static Descriptor getRelocatedDescriptor(Descriptors.Descriptor original * the relocated JsonFormat * * @param message The incoming (unrelocated) protobuf message. + * @return JSON string representation of the message + * @throws Exception if conversion fails */ - public static String getMessage(Message message) { + public static String getMessage(Message message) throws Exception { if (message == null) { log.debug("Cannot convert null message to JSON"); return ""; } - try { - // Convert the unrelocated message into a relocated DynamicMessage. - DynamicMessage relocatedMessage = convertToRelocatedDynamicMessage(message); + // Convert the unrelocated message into a relocated DynamicMessage. + DynamicMessage relocatedMessage = convertToRelocatedDynamicMessage(message); - // Use the relocated JsonFormat to print the message as JSON. - JsonFormat.Printer relocatedPrinter = - JsonFormat.printer().includingDefaultValueFields().preservingProtoFieldNames(); - return relocatedPrinter.print(relocatedMessage); - } catch (Exception e) { - log.error("Failed to convert message to JSON: {}", e.getMessage(), e); - if (log.isDebugEnabled()) { - log.debug("Message type: {}", message.getClass().getName()); - log.debug("Message descriptor: {}", message.getDescriptorForType().getFullName()); - } - } - return ""; + // Use the relocated JsonFormat to print the message as JSON. + JsonFormat.Printer relocatedPrinter = + JsonFormat.printer().includingDefaultValueFields().preservingProtoFieldNames(); + return relocatedPrinter.print(relocatedMessage); } }