Skip to content

Commit 9a04d90

Browse files
authored
feat: fix support of transitive dependencies in conversion of protobuf messages to json (#419)
* recursive handling of dependencies * removing unnecessary lines
1 parent 3696821 commit 9a04d90

File tree

2 files changed

+76
-32
lines changed

2 files changed

+76
-32
lines changed

instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcSpanDecorator.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import io.grpc.Metadata.Key;
2222
import io.opentelemetry.api.common.AttributeKey;
2323
import io.opentelemetry.api.trace.Span;
24-
import io.opentelemetry.javaagent.instrumentation.hypertrace.com.google.protobuf.util.JsonFormat;
2524
import io.opentelemetry.javaagent.instrumentation.hypertrace.grpc.GrpcSemanticAttributes;
2625
import java.util.LinkedHashMap;
2726
import java.util.Map;
@@ -35,16 +34,17 @@ public class GrpcSpanDecorator {
3534
private GrpcSpanDecorator() {}
3635

3736
private static final Logger log = LoggerFactory.getLogger(GrpcSpanDecorator.class);
38-
private static final JsonFormat.Printer PRINTER = JsonFormat.printer();
3937

4038
public static void addMessageAttribute(Object message, Span span, AttributeKey<String> key) {
4139
if (message instanceof Message) {
4240
Message mb = (Message) message;
4341
try {
4442
String jsonOutput = ProtobufMessageConverter.getMessage(mb);
45-
span.setAttribute(key, jsonOutput);
43+
if (jsonOutput != null && !jsonOutput.isEmpty()) {
44+
span.setAttribute(key, jsonOutput);
45+
}
4646
} catch (Exception e) {
47-
log.error("Failed to decode message as JSON", e);
47+
log.debug("Failed to decode message as JSON: {}", e.getMessage(), e);
4848
}
4949
} else {
5050
log.debug("message is not an instance of com.google.protobuf.Message");

instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/ProtobufMessageConverter.java

+72-28
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,14 @@
2525
import io.opentelemetry.javaagent.instrumentation.hypertrace.com.google.protobuf.util.JsonFormat;
2626
import java.util.ArrayList;
2727
import java.util.HashMap;
28+
import java.util.HashSet;
2829
import java.util.List;
2930
import java.util.Map;
31+
import java.util.Set;
3032
import org.slf4j.Logger;
3133
import org.slf4j.LoggerFactory;
3234

35+
/** Utility class to convert protobuf messages to JSON. */
3336
public class ProtobufMessageConverter {
3437
private static final Logger log = LoggerFactory.getLogger(ProtobufMessageConverter.class);
3538
private static final Map<String, FileDescriptor> fileDescriptorCache = new HashMap<>();
@@ -75,25 +78,73 @@ private static Descriptor getRelocatedDescriptor(Descriptors.Descriptor original
7578
String fileKey = unrelocatedFileDescriptor.getName();
7679
if (fileDescriptorCache.containsKey(fileKey)) {
7780
FileDescriptor relocatedFileDescriptor = fileDescriptorCache.get(fileKey);
78-
return relocatedFileDescriptor.findMessageTypeByName(originalDescriptor.getName());
81+
82+
// Check if the message type exists in the relocated descriptor
83+
Descriptor messageType =
84+
relocatedFileDescriptor.findMessageTypeByName(originalDescriptor.getName());
85+
if (messageType == null) {
86+
log.debug("Message type not found in cached descriptor: {}", originalDescriptor.getName());
87+
}
88+
89+
return messageType;
7990
}
8091

81-
// Process all dependencies first
92+
// Process all dependencies recursively, including transitive ones
93+
FileDescriptor fileDescriptor =
94+
processFileDescriptorWithDependencies(unrelocatedFileDescriptor, new HashSet<>());
95+
96+
// Find the message type in the relocated descriptor
97+
Descriptor result = fileDescriptor.findMessageTypeByName(originalDescriptor.getName());
98+
if (result == null) {
99+
log.debug("Message type not found in new descriptor: {}", originalDescriptor.getName());
100+
}
101+
102+
return result;
103+
}
104+
105+
/**
106+
* Process a file descriptor and all its dependencies recursively.
107+
*
108+
* @param unrelocatedFileDescriptor The file descriptor to process
109+
* @param processedFiles Set of file names that have already been processed to avoid circular
110+
* dependencies
111+
* @return The relocated file descriptor
112+
*/
113+
private static FileDescriptor processFileDescriptorWithDependencies(
114+
Descriptors.FileDescriptor unrelocatedFileDescriptor, Set<String> processedFiles)
115+
throws Exception {
116+
String fileKey = unrelocatedFileDescriptor.getName();
117+
118+
// Check if we've already processed this file
119+
if (fileDescriptorCache.containsKey(fileKey)) {
120+
return fileDescriptorCache.get(fileKey);
121+
}
122+
123+
// Mark this file as processed to avoid circular dependencies
124+
processedFiles.add(fileKey);
125+
82126
List<FileDescriptor> dependencies = new ArrayList<>();
127+
128+
// Process all direct dependencies first
83129
for (Descriptors.FileDescriptor dependency : unrelocatedFileDescriptor.getDependencies()) {
84130
String depKey = dependency.getName();
85-
if (!fileDescriptorCache.containsKey(depKey)) {
86-
// Convert the dependency file descriptor
87-
com.google.protobuf.DescriptorProtos.FileDescriptorProto depProto = dependency.toProto();
88-
byte[] depBytes = depProto.toByteArray();
89-
FileDescriptorProto relocatedDepProto = FileDescriptorProto.parseFrom(depBytes);
90131

91-
// Build with empty dependencies first (we'll fill them in later)
132+
// Skip if we've already processed this dependency in this call chain
133+
if (processedFiles.contains(depKey)) {
134+
if (fileDescriptorCache.containsKey(depKey)) {
135+
dependencies.add(fileDescriptorCache.get(depKey));
136+
}
137+
continue;
138+
}
139+
140+
if (!fileDescriptorCache.containsKey(depKey)) {
141+
// Process this dependency recursively
92142
FileDescriptor relocatedDep =
93-
FileDescriptor.buildFrom(relocatedDepProto, new FileDescriptor[] {});
94-
fileDescriptorCache.put(depKey, relocatedDep);
143+
processFileDescriptorWithDependencies(dependency, processedFiles);
144+
dependencies.add(relocatedDep);
145+
} else {
146+
dependencies.add(fileDescriptorCache.get(depKey));
95147
}
96-
dependencies.add(fileDescriptorCache.get(depKey));
97148
}
98149

99150
// Now build the current file descriptor with its dependencies
@@ -106,36 +157,29 @@ private static Descriptor getRelocatedDescriptor(Descriptors.Descriptor original
106157
FileDescriptor.buildFrom(relocatedFileProto, dependencies.toArray(new FileDescriptor[0]));
107158
fileDescriptorCache.put(fileKey, relocatedFileDescriptor);
108159

109-
return relocatedFileDescriptor.findMessageTypeByName(originalDescriptor.getName());
160+
return relocatedFileDescriptor;
110161
}
111162

112163
/**
113164
* Method that takes an incoming message, converts it to a relocated one, prints it as JSON using
114165
* the relocated JsonFormat
115166
*
116167
* @param message The incoming (unrelocated) protobuf message.
168+
* @return JSON string representation of the message
169+
* @throws Exception if conversion fails
117170
*/
118-
public static String getMessage(Message message) {
171+
public static String getMessage(Message message) throws Exception {
119172
if (message == null) {
120173
log.debug("Cannot convert null message to JSON");
121174
return "";
122175
}
123176

124-
try {
125-
// Convert the unrelocated message into a relocated DynamicMessage.
126-
DynamicMessage relocatedMessage = convertToRelocatedDynamicMessage(message);
177+
// Convert the unrelocated message into a relocated DynamicMessage.
178+
DynamicMessage relocatedMessage = convertToRelocatedDynamicMessage(message);
127179

128-
// Use the relocated JsonFormat to print the message as JSON.
129-
JsonFormat.Printer relocatedPrinter =
130-
JsonFormat.printer().includingDefaultValueFields().preservingProtoFieldNames();
131-
return relocatedPrinter.print(relocatedMessage);
132-
} catch (Exception e) {
133-
log.error("Failed to convert message to JSON: {}", e.getMessage(), e);
134-
if (log.isDebugEnabled()) {
135-
log.debug("Message type: {}", message.getClass().getName());
136-
log.debug("Message descriptor: {}", message.getDescriptorForType().getFullName());
137-
}
138-
}
139-
return "";
180+
// Use the relocated JsonFormat to print the message as JSON.
181+
JsonFormat.Printer relocatedPrinter =
182+
JsonFormat.printer().includingDefaultValueFields().preservingProtoFieldNames();
183+
return relocatedPrinter.print(relocatedMessage);
140184
}
141185
}

0 commit comments

Comments
 (0)