Skip to content

Commit

Permalink
fix(llmobs): don't set span type to llm if llmobs is disabled (#9421)
Browse files Browse the repository at this point in the history
NOTE to apm-python reviewers: The only files this PR touches outside of
LLM Observability ownership are snapshot files.

This PR expands on #9417 by only set span.span_type = "llm" if LLMObs is
enabled. This means that we will not even attempt to process the given
span (i.e. set temporary LLMObs tags), which should minimize when the
affect LLMObs code has outside of LLMObs contexts. The majority of this
PR's LOC involves modifying all openai/langchain/bedrock snapshot files
to remove "type":"llm".

Existing tests should cover the reverted functionality.

## Checklist

- [x] Change(s) are motivated and described in the PR description
- [x] Testing strategy is described if automated tests are not included
in the PR
- [x] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
  • Loading branch information
Yun-Kim authored May 30, 2024
1 parent c8ac79c commit 8b27834
Show file tree
Hide file tree
Showing 109 changed files with 181 additions and 179 deletions.
6 changes: 4 additions & 2 deletions ddtrace/contrib/botocore/services/bedrock.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,16 +317,18 @@ def patched_bedrock_api_call(original_func, instance, args, kwargs, function_var
params = function_vars.get("params")
pin = function_vars.get("pin")
model_provider, model_name = params.get("modelId").split(".")
integration = function_vars.get("integration")
submit_to_llmobs = integration.llmobs_enabled and "embed" not in model_name
with core.context_with_data(
"botocore.patched_bedrock_api_call",
pin=pin,
span_name=function_vars.get("trace_operation"),
service=schematize_service_name("{}.{}".format(pin.service, function_vars.get("endpoint_name"))),
resource=function_vars.get("operation"),
span_type=SpanTypes.LLM if "embed" not in model_name else None,
span_type=SpanTypes.LLM if submit_to_llmobs else None,
call_key="instrumented_bedrock_call",
call_trace=True,
bedrock_integration=function_vars.get("integration"),
bedrock_integration=integration,
params=params,
model_provider=model_provider,
model_name=model_name,
Expand Down
4 changes: 2 additions & 2 deletions ddtrace/llmobs/_integrations/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ def trace(self, pin: Pin, operation_id: str, submit_to_llmobs: bool = False, **k
# Enable trace metrics for these spans so users can see per-service openai usage in APM.
span.set_tag(SPAN_MEASURED_KEY)
self._set_base_span_tags(span, **kwargs)
if submit_to_llmobs:
if submit_to_llmobs and self.llmobs_enabled:
span.span_type = SpanTypes.LLM
if self.llmobs_enabled and span.get_tag(PROPAGATED_PARENT_ID_KEY) is None:
if span.get_tag(PROPAGATED_PARENT_ID_KEY) is None:
# For non-distributed traces or spans in the first service of a distributed trace,
# The LLMObs parent ID tag is not set at span start time. We need to manually set the parent ID tag now
# in these cases to avoid conflicting with the later propagated tags.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 1,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 1,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 1,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 1,
"meta": {
"_dd.base_service": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down Expand Up @@ -36,7 +36,7 @@
"trace_id": 0,
"span_id": 2,
"parent_id": 1,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"langchain.request.inputs.question": "what is thirteen raised to the .3432 power?",
Expand All @@ -60,7 +60,7 @@
"trace_id": 0,
"span_id": 3,
"parent_id": 2,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"langchain.request.api_key": "...key>",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down Expand Up @@ -51,7 +51,7 @@
"trace_id": 0,
"span_id": 2,
"parent_id": 1,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"component": "openai",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 1,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"trace_id": 0,
"span_id": 1,
"parent_id": 0,
"type": "llm",
"type": "",
"error": 0,
"meta": {
"_dd.p.dm": "-0",
Expand Down
Loading

0 comments on commit 8b27834

Please sign in to comment.