Skip to content

Commit 4dc6b0c

Browse files
albe2669andrescrz
andauthored
[OPIK-7496] [BE] fix: strip otel attribute rule prefix from saved key (#7499)
* [OPIK-7496] [BE] fix: strip otel attribute rule prefix from saved key * fix: require boundary before stripping prefix in storageKey * refactor: remove storageKeyExactRuleNeverStrips test uu * fix: parse JSON before branching on isObject u * fix: filter reserved metadata keys from metadata * fix: narrow exception catch and log parse failures * fix: remove PII from debug log and parameterize reserved key test * fix: guard storageKey substring and add @nonnull validation * test: update testRuleMapping for prefix-stripped storage keys * fix: log as warn and eextract block * fix: only parse json looking strings --------- Co-authored-by: Andres Cruz <andresc@comet.com>
1 parent ed2921e commit 4dc6b0c

5 files changed

Lines changed: 298 additions & 25 deletions

File tree

apps/opik-backend/src/main/java/com/comet/opik/domain/OpenTelemetryMapper.java

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.comet.opik.domain.retention.RetentionUtils;
1111
import com.comet.opik.utils.JsonUtils;
1212
import com.fasterxml.jackson.databind.node.ObjectNode;
13+
import io.opentelemetry.proto.common.v1.AnyValue;
1314
import io.opentelemetry.proto.common.v1.KeyValue;
1415
import io.opentelemetry.proto.trace.v1.Span;
1516
import io.opentelemetry.proto.trace.v1.Status;
@@ -19,6 +20,7 @@
1920
import org.apache.commons.collections4.CollectionUtils;
2021
import org.apache.commons.lang3.StringUtils;
2122

23+
import java.io.UncheckedIOException;
2224
import java.nio.ByteBuffer;
2325
import java.time.Duration;
2426
import java.time.Instant;
@@ -35,6 +37,7 @@
3537
import static com.comet.opik.domain.mapping.OpenTelemetryMappingUtils.extractTags;
3638
import static com.comet.opik.domain.mapping.OpenTelemetryMappingUtils.extractToJsonColumn;
3739
import static com.comet.opik.domain.mapping.OpenTelemetryMappingUtils.extractUsageField;
40+
import static com.comet.opik.domain.mapping.OpenTelemetryMappingUtils.storageKey;
3841

3942
@UtilityClass
4043
@Slf4j
@@ -133,6 +136,9 @@ public static void enrichSpanWithAttributes(SpanBuilder spanBuilder, List<KeyVal
133136
private static final String CLAUDE_CODE_LLM_SPAN = "claude_code.llm_request";
134137
private static final String NEW_CONTEXT_ATTR = "new_context";
135138

139+
// Reserved metadata keys that must not be overwritten by user-supplied JSON merged from opik.metadata.
140+
private static final Set<String> RESERVED_METADATA_KEYS = Set.of("thread_id", "integration", "server.address");
141+
136142
/**
137143
* Same as {@link #enrichSpanWithAttributes(SpanBuilder, List, String, List)} but with the OTEL
138144
* span name, used for span-name-aware routing (e.g. Claude Code's {@code new_context} maps to
@@ -216,7 +222,16 @@ public static void enrichSpanWithAttributes(SpanBuilder spanBuilder, List<KeyVal
216222
default -> metadata;
217223
};
218224

219-
extractToJsonColumn(node, key, value);
225+
String jsonKey = storageKey(rule, key);
226+
// If the suffix is empty then try merging as a JSON object,
227+
// otherwise nest under the stripped key or the rule key.
228+
if (jsonKey.isEmpty() && value.getValueCase() == AnyValue.ValueCase.STRING_VALUE) {
229+
mergeJsonObjectOrFallback(node, rule.getRule(), key, value);
230+
} else if (jsonKey.isEmpty()) {
231+
extractToJsonColumn(node, rule.getRule(), value);
232+
} else {
233+
extractToJsonColumn(node, jsonKey, value);
234+
}
220235
break;
221236

222237
case TAGS :
@@ -298,6 +313,39 @@ public static void enrichSpanWithAttributes(SpanBuilder spanBuilder, List<KeyVal
298313
}
299314
}
300315

316+
/**
317+
* When the storage key is empty and the value is a string, try parsing it as a JSON object
318+
* and merge its fields into {@code node}, skipping {@link #RESERVED_METADATA_KEYS}. On a
319+
* non-object JSON value or a parse failure, fall back to storing the raw value under the
320+
* rule's key via {@link #extractToJsonColumn}.
321+
*/
322+
private static void mergeJsonObjectOrFallback(ObjectNode node, String ruleKey, String key, AnyValue value) {
323+
String stringValue = value.getStringValue();
324+
325+
// Only try to parse JSON-looking strings
326+
String trimmed = StringUtils.trimToEmpty(stringValue);
327+
if (!trimmed.startsWith("{") || !trimmed.endsWith("}")) {
328+
extractToJsonColumn(node, ruleKey, value);
329+
return;
330+
}
331+
try {
332+
var jsonNode = JsonUtils.getJsonNodeFromString(stringValue);
333+
if (jsonNode.isObject()) {
334+
jsonNode.fields()
335+
.forEachRemaining(entry -> {
336+
if (!RESERVED_METADATA_KEYS.contains(entry.getKey())) {
337+
node.set(entry.getKey(), entry.getValue());
338+
}
339+
});
340+
} else {
341+
extractToJsonColumn(node, ruleKey, value);
342+
}
343+
} catch (UncheckedIOException e) {
344+
log.warn("Failed to parse JSON, falling back to text for key '{}'", key, e);
345+
extractToJsonColumn(node, ruleKey, value);
346+
}
347+
}
348+
301349
private static final String TOOL_OUTPUT_EVENT_NAME = "tool.output";
302350
private static final Set<String> TOOL_OUTPUT_CONTENT_KEYS = Set.of("output", "content");
303351

apps/opik-backend/src/main/java/com/comet/opik/domain/mapping/OpenTelemetryMappingUtils.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,43 @@ public static void extractToJsonColumn(ObjectNode node, String key, @NonNull Any
8282
}
8383
}
8484

85+
/**
86+
* Computes the storage key for an attribute under a mapping rule.
87+
* <p>
88+
* For exact-match rules the original key is returned unchanged. For prefix rules the
89+
* prefix is stripped and any leading dot on the suffix is removed. The suffix may be
90+
* empty, in which case the caller should typically merge the value into the enclosing object.
91+
* If a prefix rule does not actually match the key (key shorter than the prefix, or the
92+
* stripped suffix does not start with a dot when the prefix lacks one), the original
93+
* key is returned unchanged.
94+
*
95+
* @param rule the mapping rule to apply; must not be {@code null}
96+
* @param key the attribute key to transform; must not be {@code null}
97+
* @return the storage key to use, never {@code null}
98+
*/
99+
public static String storageKey(@NonNull OpenTelemetryMappingRule rule, @NonNull String key) {
100+
if (!rule.isPrefix()) {
101+
return key;
102+
}
103+
104+
String prefix = rule.getRule();
105+
if (key.length() < prefix.length()) {
106+
return key;
107+
}
108+
String suffix = key.substring(prefix.length());
109+
110+
// Strip when suffix is empty, suffix starts with '.', or the rule itself ends with '.' (e.g. "gen_ai.input.").
111+
// A prefix match like "input" on "input_tokens" must not strip.
112+
if (suffix.isEmpty() || suffix.startsWith(".") || prefix.endsWith(".")) {
113+
if (suffix.startsWith(".")) {
114+
suffix = suffix.substring(1);
115+
}
116+
return suffix;
117+
}
118+
119+
return key;
120+
}
121+
85122
// Prefix rules (e.g. `gen_ai.usage.`) carry the usage name in the suffix; exact-match rules
86123
// (e.g. Claude Code's `input_tokens`) use the full key.
87124
private static String usageKey(OpenTelemetryMappingRule rule, String key) {

apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/OpenTelemetryResourceTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -406,12 +406,12 @@ void testRuleMapping(String modelKey) {
406406
assertThat(span.provider()).isEqualTo("openai");
407407
assertThat(span.type()).isEqualTo(SpanType.llm);
408408

409-
assertThat(span.metadata().get("code.line").asInt()).isEqualTo(11);
410-
assertThat(span.metadata().get("smolagents.single").asText()).isEqualTo("value");
411-
assertThat(span.metadata().get("smolagents.node").get("key").asText()).isEqualTo("value");
412-
assertThat(span.metadata().get("smolagents.array").isArray()).isTrue();
409+
assertThat(span.metadata().get("line").asInt()).isEqualTo(11);
410+
assertThat(span.metadata().get("single").asText()).isEqualTo("value");
411+
assertThat(span.metadata().get("node").get("key").asText()).isEqualTo("value");
412+
assertThat(span.metadata().get("array").isArray()).isTrue();
413413

414-
assertThat(span.input().get("input").get("key").asText()).isEqualTo("value");
414+
assertThat(span.input().get("key").asText()).isEqualTo("value");
415415
assertThat(span.input().get("tools").isArray()).isEqualTo(Boolean.TRUE);
416416
assertThat(span.input().get("all_messages").isArray()).isEqualTo(Boolean.TRUE);
417417

@@ -423,8 +423,8 @@ void testRuleMapping(String modelKey) {
423423

424424
// check metadata
425425
assertThat(span.metadata()).isNotEmpty();
426-
assertThat(span.metadata().get("opik.metadata").get("foo").asText()).isEqualTo("bar");
427-
assertThat(span.metadata().get("opik.metadata.inline").asText()).isEqualTo("inline_value");
426+
assertThat(span.metadata().get("foo").asText()).isEqualTo("bar");
427+
assertThat(span.metadata().get("inline").asText()).isEqualTo("inline_value");
428428
}
429429

430430
@Test

0 commit comments

Comments
 (0)