dd-trace-py icon indicating copy to clipboard operation
dd-trace-py copied to clipboard

feat(llmobs): allow span processor to return None to omit spans

Open amir-mic-lin opened this issue 5 months ago • 1 comments

Allow users to omit any traces they want by returning none in the user_span_processor method.

Motivation: I am trying to prevent some auto traces from cluttering my Datadog observability dashboards. These traces create noise and make it harder to focus on the more critical traces. My LLM observability overview is filled with what is clustered as empty input which is incorrect. e.g. all embedding traces are are just a spammer.

To do that, I am using the new span_processor, and the new way to omit specific spans, will be to return null by the relevant span processor.

I have added a test for that.

The only risk is that there is no indication that span was omitted but I was afraid that a debug log would be too spammy. But please let me know if that will help. Also there are some telemetric collected about sent span which might also need to be changed, will be great to hear your thoughts

Checklist

  • [x] PR author has checked that all the criteria below are met
  • The PR description includes an overview of the change
  • The PR description articulates the motivation for the change
  • The change includes tests OR the PR description describes a testing strategy
  • The PR description notes risks associated with the change, if any
  • Newly-added code is easy to change
  • The change follows the library release note guidelines
  • The change includes or references documentation updates if necessary
  • Backport labels are set (if applicable)

Reviewer Checklist

  • [ ] Reviewer has checked that all the criteria below are met
  • Title is accurate
  • All changes are related to the pull request's stated goal
  • Avoids breaking API changes
  • Testing strategy adequately addresses listed risks
  • Newly-added code is easy to change
  • Release note makes sense to a user of the library
  • If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
  • Backport labels are set in a manner that is consistent with the release branch maintenance policy

amir-mic-lin avatar Jun 22 '25 04:06 amir-mic-lin

I am also making another change where instead of having LLMObsSpan - I will use the original Span - that way the user can have more inputs on the span and filter only relevant

amir-mic-lin avatar Jun 22 '25 06:06 amir-mic-lin

hey @amir-mic-lin thanks for the contribution and sorry for the late reply! This is definitely something on our radar to introduce. I was debating introducing this originally with the span processor functionality but held off pending a concrete use-case. Also, I was internally conflicted about this approach vs using a contextual API instead to flag the span upfront at creation rather than at transmission time... something like:

with LLMObs.with_annotation_context(drop=True):
     # code block where the specific span may be generated
    ...

Is this something that would suit your use-case or is there more value in the SpanProcessor approach for you?

Kyle-Verhoog avatar Jul 08 '25 03:07 Kyle-Verhoog

@Kyle-Verhoog thanks for getting back! The reason I wanted to go with the "span processor functionality" concept, is because sometimes we would like to drop/omit spans based on a specific condition in the span (metadata or tags field). E.g., we have a customer that wants to avoid from logging any LLMs communications. That is why I also added to the span processor function the whole span and not only the input and output.

Going with the contextual API will not allow that (unless I am missing something and then please keep me honest). WDYT?

amir-mic-lin avatar Jul 09 '25 09:07 amir-mic-lin

Thanks for sharing the use-case, totally agreed.

I think the contextual API would still allow it, you'd just inline with LLMObs.with_annotation_context(drop=True) where you would normally tag a request. I think the easy of the spanprocessor is a bit better though and I can see cases for both.

I'd like to refrain from exposing the underlying raw span to the spanprocessor as it leaks the data model which will make it harder for us to change or optimize in the future.

Kyle-Verhoog avatar Jul 15 '25 13:07 Kyle-Verhoog

@Kyle-Verhoog I understand, so you are saying that all my use cases could be answered with the current LLMObsSpan representation. I just should add the relevant tags. How would I be able to omit all azure open ai auto spans?

Anyway, I have reverted the span context addition and kept only the input/output/tags approach. How can we continue with this?

amir-mic-lin avatar Jul 15 '25 17:07 amir-mic-lin

How would I be able to omit all azure open ai auto spans?

I think we can extend the LLMObsSpan with additional info (like name, model, etc) - I just don't want to leak the underlying data model 😄

Anyway, I have reverted the span context addition and kept only the input/output/tags approach. 👌 perf

How can we continue with this?

I think this is great! We'll see to getting it reviewed and merged. We have some high priority stuff so ETA is to get it merged later next week

Kyle-Verhoog avatar Jul 17 '25 05:07 Kyle-Verhoog

I think this is great! We'll see to getting it reviewed and merged. We have some high priority stuff so ETA is to get it merged later next week

Thanks! waiting for further instructions of what I should do to be able to merge it

amir-mic-lin avatar Jul 20 '25 06:07 amir-mic-lin

@Kyle-Verhoog Hi! any updates regarding this PR?

amir-mic-lin avatar Jul 27 '25 08:07 amir-mic-lin

@amir-mic-lin I opened https://github.com/DataDog/dd-trace-py/pull/14260 with your work cherry-picked over since i think the CI is blocked on not having the commits signed. We'll take it from there and it'll go out next week in the 3.12 release 😄. Thank you for driving this!!

Kyle-Verhoog avatar Aug 08 '25 14:08 Kyle-Verhoog