-
Notifications
You must be signed in to change notification settings - Fork 268
feat(span-processor): mark app root spans #1651
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
|
|
||
| import backoff | ||
| import httpx | ||
| from opentelemetry import context as otel_context_api | ||
| from opentelemetry import trace as otel_trace_api | ||
| from opentelemetry.sdk.trace import ReadableSpan, TracerProvider | ||
| from opentelemetry.sdk.trace.export import SpanExporter | ||
|
|
@@ -66,7 +67,9 @@ | |
| ) | ||
| from langfuse._client.propagation import ( | ||
| PropagatedExperimentAttributes, | ||
| _detach_context_token_safely, | ||
| _propagate_attributes, | ||
| _set_langfuse_trace_id_in_baggage, | ||
| ) | ||
| from langfuse._client.resource_manager import LangfuseResourceManager | ||
| from langfuse._client.span import ( | ||
|
|
@@ -1178,39 +1181,54 @@ | |
| name=name, | ||
| end_on_exit=end_on_exit if end_on_exit is not None else True, | ||
| ) as otel_span: | ||
| baggage_token = None | ||
|
|
||
| if otel_span.is_recording(): | ||
| context_with_app_root_claim = _set_langfuse_trace_id_in_baggage( | ||
| trace_id=self._get_otel_trace_id(otel_span), | ||
| context=otel_context_api.get_current(), | ||
| ) | ||
| baggage_token = otel_context_api.attach(context_with_app_root_claim) | ||
|
Check failure on line 1191 in langfuse/_client/client.py
|
||
|
Comment on lines
+1184
to
+1191
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. 🔴 The baggage claim ( Extended reasoning...What the bug is
How it manifests in a distributed traceConfigure Service A with a custom
Net effect: Why existing code doesn't prevent itThe local case is handled correctly: when Service A starts The cross-process baggage path has no equivalent. Impact
FixGate the baggage attachment at |
||
|
|
||
| span_class = self._get_span_class( | ||
| as_type or "generation" | ||
| ) # default was "generation" | ||
| common_args = { | ||
| "otel_span": otel_span, | ||
| "langfuse_client": self, | ||
| "environment": self._environment, | ||
| "release": self._release, | ||
| "input": input, | ||
| "output": output, | ||
| "metadata": metadata, | ||
| "version": version, | ||
| "level": level, | ||
| "status_message": status_message, | ||
| } | ||
|
|
||
| if span_class in [ | ||
| LangfuseGeneration, | ||
| LangfuseEmbedding, | ||
| ]: | ||
| common_args.update( | ||
| { | ||
| "completion_start_time": completion_start_time, | ||
| "model": model, | ||
| "model_parameters": model_parameters, | ||
| "usage_details": usage_details, | ||
| "cost_details": cost_details, | ||
| "prompt": prompt, | ||
| } | ||
| ) | ||
| # For span-like types (span, agent, tool, chain, retriever, evaluator, guardrail), no generation properties needed | ||
| try: | ||
| common_args = { | ||
| "otel_span": otel_span, | ||
| "langfuse_client": self, | ||
| "environment": self._environment, | ||
| "release": self._release, | ||
| "input": input, | ||
| "output": output, | ||
| "metadata": metadata, | ||
| "version": version, | ||
| "level": level, | ||
| "status_message": status_message, | ||
| } | ||
|
|
||
| if span_class in [ | ||
| LangfuseGeneration, | ||
| LangfuseEmbedding, | ||
| ]: | ||
| common_args.update( | ||
| { | ||
| "completion_start_time": completion_start_time, | ||
| "model": model, | ||
| "model_parameters": model_parameters, | ||
| "usage_details": usage_details, | ||
| "cost_details": cost_details, | ||
| "prompt": prompt, | ||
| } | ||
| ) | ||
| # For span-like types (span, agent, tool, chain, retriever, evaluator, guardrail), no generation properties needed | ||
|
|
||
| yield span_class(**common_args) # type: ignore[arg-type] | ||
|
|
||
| yield span_class(**common_args) # type: ignore[arg-type] | ||
| finally: | ||
| if baggage_token is not None: | ||
| _detach_context_token_safely(baggage_token) | ||
|
|
||
| def _get_current_otel_span(self) -> Optional[otel_trace_api.Span]: | ||
| current_span = otel_trace_api.get_current_span() | ||
|
|
||
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.
_start_as_current_otel_span_with_processed_medianow addslangfuse_trace_idbaggage for every recording Langfuse span before any export filtering runs, so downstream services can suppressIS_APP_ROOTeven when the upstream parent is later dropped byshould_export_span/scope filters. In that setup (custom filtering + distributed propagation), no exported span may end up marked as app root in downstream traces, which defeats the new root-selection behavior. The claim should only be propagated when the originating span is expected to export (or include a way to distinguish non-exporting parents).Useful? React with 👍 / 👎.