Add in LLM Observability Span Processor API - #12112
Conversation
…ner/llmobs-span-processor
sabrenner
added
type: feature
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
🎯 Code Coverage (details) 🔗 Commit SHA: 113074b | Docs | Datadog PR Page | Give us feedback! |
| private static final String LLMOBS_TAG_PREFIX = "_ml_obs_tag."; | ||
| private static final String INPUT_TAG = LLMOBS_TAG_PREFIX + "input"; | ||
| private static final String OUTPUT_TAG = LLMOBS_TAG_PREFIX + "output"; | ||
| private static final String SPAN_KIND_TAG = LLMOBS_TAG_PREFIX + Tags.SPAN_KIND; |
There was a problem hiding this comment.
can we use constants from other files? or are they not defined anywhere?
| List<LLMObs.LLMMessage> messages = new ArrayList<>(values.size()); | ||
| for (Object value : values) { | ||
| if (!(value instanceof LLMObs.LLMMessage)) { | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
why do we need this check? don't we always call this function with a list of LLMObs.LLMMessage objects?
| List<? extends CoreSpan<?>> llmobsSpans = | ||
| trace.stream().filter(LLMObsSpanMapper::isLLMObsSpan).collect(Collectors.toList()); | ||
|
|
||
| boolean retry = trace == pendingTrace; |
There was a problem hiding this comment.
it seems this logic was added to avoid retry issues with mapping. however, since this is currently possible in master, why do we need to add new handling? we might be introducing unnecessary overhead on extra checks and objects when not needed
| // 3 | ||
| writable.writeUTF8(PARENT_ID); | ||
| writable.writeString(span.getTag(PARENT_ID_TAG_INTERNAL_FULL), null); | ||
| span.removeTag(PARENT_ID_TAG_INTERNAL_FULL); |
There was a problem hiding this comment.
i could be misreading the diff, but it looks like this tag is no longer removed in the new code?
| LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID; | ||
|
|
||
| private final MetaWriter metaWriter = new MetaWriter(); | ||
| private final Set<CoreSpan<?>> droppedSpans = Collections.newSetFromMap(new IdentityHashMap<>()); |
There was a problem hiding this comment.
does this need to be an instance variable? or can we keep it scoped to the map function?
| } else { | ||
| ++tagsSize; | ||
| } | ||
| } else if (key.startsWith(LLMOBS_TAG_PREFIX) && !key.equals(PARENT_ID_TAG_INTERNAL_FULL)) { |
There was a problem hiding this comment.
is the logic changed here necessary? seems we don't need to have this changed for the task at hand
| String key = tag.getKey(); | ||
| Object value = tag.getValue(); | ||
| if (!tagsToRemapToMeta.containsKey(key) && key.startsWith(LLMOBS_TAG_PREFIX)) { | ||
| if (!tagsToRemapToMeta.containsKey(key) |
There was a problem hiding this comment.
likewise this seems like a non-scoped change?
1 participant
What Does This Do
Adds in a user span processor API to process LLM Observability / Agent Observability spans before they are flushed to Datadog.
Motivation
Being able to redact input / output on certain spans, by analyzing tags on the LLM Observability span.
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: MLOB-7902