-
Notifications
You must be signed in to change notification settings - Fork 955
fix(openai): propagate span IDs properly to events #3243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
be07b75
03002d7
71344f8
d308ead
4738519
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry import context as context_api | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry import trace | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.instrumentation.openai.shared import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _set_client_attributes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _set_functions_attributes, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -55,25 +56,27 @@ def completion_wrapper(tracer, wrapped, instance, args, kwargs): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| attributes={SpanAttributes.LLM_REQUEST_TYPE: LLM_REQUEST_TYPE.value}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_request(span, kwargs, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Use the span as current context to ensure events get proper trace context | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with trace.use_span(span, end_on_exit=False): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_request(span, kwargs, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = wrapped(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_attribute(ERROR_TYPE, e.__class__.__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.record_exception(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.end() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if is_streaming_response(response): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # span will be closed after the generator is done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _build_from_streaming_response(span, kwargs, response) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_response(response, span, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+72
to
77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainStreaming path loses current-context for downstream events; wrap generator bodies with trace.use_span When you return the streaming generator, the with trace.use_span context is exited. emit_event calls inside _build_from_streaming_response/_abuild_from_streaming_response will then run without a current span, reintroducing the missing span_id problem for streaming events. Wrap the generator bodies with trace.use_span(span, end_on_exit=False) to keep the span current throughout iteration and finalization. Apply: def _build_from_streaming_response(span, request_kwargs, response):
- complete_response = {"choices": [], "model": "", "id": ""}
- for item in response:
- yield item
- _accumulate_streaming_response(complete_response, item)
- _set_response_attributes(span, complete_response)
- _set_token_usage(span, request_kwargs, complete_response)
- if should_emit_events():
- _emit_streaming_response_events(complete_response)
- else:
- if should_send_prompts():
- _set_completions(span, complete_response.get("choices"))
- span.set_status(Status(StatusCode.OK))
- span.end()
+ # Keep span as current during the whole streaming lifecycle
+ with trace.use_span(span, end_on_exit=False):
+ complete_response = {"choices": [], "model": "", "id": ""}
+ for item in response:
+ yield item
+ _accumulate_streaming_response(complete_response, item)
+ _set_response_attributes(span, complete_response)
+ _set_token_usage(span, request_kwargs, complete_response)
+ if should_emit_events():
+ _emit_streaming_response_events(complete_response)
+ else:
+ if should_send_prompts():
+ _set_completions(span, complete_response.get("choices"))
+ span.set_status(Status(StatusCode.OK))
+ span.end()
async def _abuild_from_streaming_response(span, request_kwargs, response):
- complete_response = {"choices": [], "model": "", "id": ""}
- async for item in response:
- yield item
- _accumulate_streaming_response(complete_response, item)
- _set_response_attributes(span, complete_response)
- _set_token_usage(span, request_kwargs, complete_response)
- if should_emit_events():
- _emit_streaming_response_events(complete_response)
- else:
- if should_send_prompts():
- _set_completions(span, complete_response.get("choices"))
- span.set_status(Status(StatusCode.OK))
- span.end()
+ # Keep span as current during the whole streaming lifecycle
+ with trace.use_span(span, end_on_exit=False):
+ complete_response = {"choices": [], "model": "", "id": ""}
+ async for item in response:
+ yield item
+ _accumulate_streaming_response(complete_response, item)
+ _set_response_attributes(span, complete_response)
+ _set_token_usage(span, request_kwargs, complete_response)
+ if should_emit_events():
+ _emit_streaming_response_events(complete_response)
+ else:
+ if should_send_prompts():
+ _set_completions(span, complete_response.get("choices"))
+ span.set_status(Status(StatusCode.OK))
+ span.end()Run this quick grep to spot any remaining emit_event sites in streaming code that run outside a use_span context: Also applies to: 108-115, 183-201, 204-222 🏁 Script executed: #!/bin/bash
rg -n "emit_event\(" -A 4 -B 6 packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/completion_wrappers.pyLength of output: 1359 I’d like to verify whether any 🏁 Script executed: rg -n "use_span" -A 3 -B 3 packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/completion_wrappers.pyLength of output: 582 Wrap streaming generators in Returning the raw generator exits the outer Affected locations in
Suggested patch: def _build_from_streaming_response(span, request_kwargs, response):
- complete_response = {"choices": [], "model": "", "id": ""}
- for item in response:
- yield item
- _accumulate_streaming_response(complete_response, item)
- _set_response_attributes(span, complete_response)
- _set_token_usage(span, request_kwargs, complete_response)
- if should_emit_events():
- _emit_streaming_response_events(complete_response)
- else:
- if should_send_prompts():
- _set_completions(span, complete_response.get("choices"))
- span.set_status(Status(StatusCode.OK))
- span.end()
+ # Keep span current for events and finalization
+ with trace.use_span(span, end_on_exit=False):
+ complete_response = {"choices": [], "model": "", "id": ""}
+ for item in response:
+ yield item
+ _accumulate_streaming_response(complete_response, item)
+ _set_response_attributes(span, complete_response)
+ _set_token_usage(span, request_kwargs, complete_response)
+ if should_emit_events():
+ _emit_streaming_response_events(complete_response)
+ elif should_send_prompts():
+ _set_completions(span, complete_response.get("choices"))
+ span.set_status(Status(StatusCode.OK))
+ span.end()
async def _abuild_from_streaming_response(span, request_kwargs, response):
- complete_response = {"choices": [], "model": "", "id": ""}
- async for item in response:
- yield item
- _accumulate_streaming_response(complete_response, item)
- _set_response_attributes(span, complete_response)
- _set_token_usage(span, request_kwargs, complete_response)
- if should_emit_events():
- _emit_streaming_response_events(complete_response)
- else:
- if should_send_prompts():
- _set_completions(span, complete_response.get("choices"))
- span.set_status(Status(StatusCode.OK))
- span.end()
+ # Keep span current for events and finalization
+ with trace.use_span(span, end_on_exit=False):
+ complete_response = {"choices": [], "model": "", "id": ""}
+ async for item in response:
+ yield item
+ _accumulate_streaming_response(complete_response, item)
+ _set_response_attributes(span, complete_response)
+ _set_token_usage(span, request_kwargs, complete_response)
+ if should_emit_events():
+ _emit_streaming_response_events(complete_response)
+ elif should_send_prompts():
+ _set_completions(span, complete_response.get("choices"))
+ span.set_status(Status(StatusCode.OK))
+ span.end()📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = wrapped(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_attribute(ERROR_TYPE, e.__class__.__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.record_exception(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.end() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if is_streaming_response(response): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # span will be closed after the generator is done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _build_from_streaming_response(span, kwargs, response) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_response(response, span, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.end() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @_with_tracer_wrapper | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -89,25 +92,27 @@ async def acompletion_wrapper(tracer, wrapped, instance, args, kwargs): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| attributes={SpanAttributes.LLM_REQUEST_TYPE: LLM_REQUEST_TYPE.value}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_request(span, kwargs, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Use the span as current context to ensure events get proper trace context | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with trace.use_span(span, end_on_exit=False): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_request(span, kwargs, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = await wrapped(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_attribute(ERROR_TYPE, e.__class__.__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.record_exception(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.end() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if is_streaming_response(response): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # span will be closed after the generator is done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _abuild_from_streaming_response(span, kwargs, response) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_response(response, span, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = await wrapped(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_attribute(ERROR_TYPE, e.__class__.__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.record_exception(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.end() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if is_streaming_response(response): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # span will be closed after the generator is done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _abuild_from_streaming_response(span, kwargs, response) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _handle_response(response, span, instance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.end() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @dont_throw | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Context not propagated into new thread used by run_async; emit_event may still miss span_id
run_async executes the coroutine in a new thread when a loop is running. The current runtime context (and thus the current span set by trace.use_span) is not automatically propagated to that thread. emit_event inside _handle_request will then lack a current span.
Fix either by propagating the context into the thread here, or by updating utils.run_async to attach a captured context.
Minimal in-place fix:
Alternatively, enhance utils.run_async to accept and attach a context (preferred, single place to maintain).