feat(span-processor): mark app root spans#1651
Conversation
|
@claude review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fffa395845
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba553a13db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
chore: add codeowners for github config Co-authored-by: Codex Opus 4.6 (1M context) <noreply@anthropic.com>
…updates (#1645) * chore(deps): bump the github-actions group across 1 directory with 5 updates Bumps the github-actions group with 5 updates in the / directory: | Package | From | To | | --- | --- | --- | | [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) | `8.0.0` | `8.1.0` | | [actions/cache](https://github.com/actions/cache) | `5.0.4` | `5.0.5` | | [github/codeql-action](https://github.com/github/codeql-action) | `4.35.1` | `4.35.4` | | [dependabot/fetch-metadata](https://github.com/dependabot/fetch-metadata) | `3.0.0` | `3.1.0` | | [slackapi/slack-github-action](https://github.com/slackapi/slack-github-action) | `3.0.1` | `3.0.3` | Updates `astral-sh/setup-uv` from 8.0.0 to 8.1.0 - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@cec2083...0880764) Updates `actions/cache` from 5.0.4 to 5.0.5 - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@6682284...27d5ce7) Updates `github/codeql-action` from 4.35.1 to 4.35.4 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@c10b806...68bde55) Updates `dependabot/fetch-metadata` from 3.0.0 to 3.1.0 - [Release notes](https://github.com/dependabot/fetch-metadata/releases) - [Commits](dependabot/fetch-metadata@ffa630c...25dd0e3) Updates `slackapi/slack-github-action` from 3.0.1 to 3.0.3 - [Release notes](https://github.com/slackapi/slack-github-action/releases) - [Changelog](https://github.com/slackapi/slack-github-action/blob/main/CHANGELOG.md) - [Commits](slackapi/slack-github-action@af78098...45a88b9) --- updated-dependencies: - dependency-name: actions/cache dependency-version: 5.0.5 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: astral-sh/setup-uv dependency-version: 8.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: dependabot/fetch-metadata dependency-version: 3.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: github/codeql-action dependency-version: 4.35.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: slackapi/slack-github-action dependency-version: 3.0.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] <support@github.com> * ci: remove stale cache action version comment --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Tobias Wochinger <tobias.wochinger@clickhouse.com>
Bumps [langsmith](https://github.com/langchain-ai/langsmith-sdk) from 0.7.22 to 0.8.0. - [Release notes](https://github.com/langchain-ai/langsmith-sdk/releases) - [Commits](langchain-ai/langsmith-sdk@v0.7.22...v0.8.0) --- updated-dependencies: - dependency-name: langsmith dependency-version: 0.8.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment has been minimized.
This comment has been minimized.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
What
Adds SDK-side app-root marking for exported spans so Langfuse V4 can default observation tables to the top-most exported observations without relying on the OpenTelemetry root span being exported.
Why
Default and custom span filtering can remove infrastructure or instrumentation spans before export. When that happens, the backend may never receive the original OTEL root span, so filtering the UI by
parent_observation_id IS NULLcan miss the useful root row. The SDK now marks the best-effort app root on the span before export.Changes
langfuse.internal.is_app_rootas the internal app-root marker attribute.LangfuseSpanProcessor.langfuse_trace_idbaggage for best-effort distributed suppression after active Langfuse spans start.langfuse_trace_idbaggage out of propagated user metadata.Limitations
should_export_spanchanges its decision by span end, the SDK does not repair already-started descendants.Verification
uv run --frozen ruff check langfuse/_client/span_processor.py langfuse/_client/client.py langfuse/_client/propagation.py langfuse/_client/attributes.py tests/unit/test_app_root_detection.py tests/unit/test_propagate_attributes.pyuv run --frozen ruff format --check langfuse/_client/span_processor.py langfuse/_client/client.py langfuse/_client/propagation.py langfuse/_client/attributes.py tests/unit/test_app_root_detection.py tests/unit/test_propagate_attributes.pyuv run --frozen mypy langfuse --no-error-summaryuv run --frozen pytest tests/unit/test_app_root_detection.py tests/unit/test_propagate_attributes.py tests/unit/test_otel.py -qGreptile Summary
This PR adds best-effort "app root" span detection to the Langfuse SDK so that Langfuse V4 can identify the topmost exported observation per trace even when OTEL infrastructure spans are filtered out before export.
LangfuseSpanProcessornow tracks per-trace, per-span export-expectation state in a thread-safe dict, marking a span aslangfuse.internal.is_app_rootaton_starttime when its immediate parent is not expected to export. Cleanup runs inon_end'sfinallyblock to ensure the state is always released.client.pyinjects alangfuse_trace_idbaggage entry into the child context after eachstart_as_current_observationspan starts, enabling distributed suppression for downstream services while avoiding self-suppression._get_propagated_attributes_from_contextto prevent it from surfacing as user metadata.Confidence Score: 3/5
The feature is logically sound and the threading model is correct, but _get_readable_span relies on a private OTel SDK method that, if ever removed or renamed, will silently disable app-root marking across the entire SDK with only a DEBUG-level log.
The core detection and cleanup logic is well-designed and the new test coverage is thorough. The risk is concentrated in _get_readable_span, which bridges from the mutable Span seen at on_start to the ReadableSpan interface expected by _should_export_span. Using span._readable_span() achieves this, but if the OTel SDK drops the private method the feature degrades invisibly with no warning and no fallback.
langfuse/_client/span_processor.py, specifically the _get_readable_span static method around line 311.
Sequence Diagram
sequenceDiagram participant C as Client (start_as_current_observation) participant SP as LangfuseSpanProcessor participant BS as BatchSpanProcessor C->>SP: on_start(span, parent_context) SP->>SP: _mark_app_root_candidate(span, parent_context) note over SP: Reads langfuse_trace_id from parent_context baggage SP->>BS: super().on_start(span, parent_context) SP-->>C: returns C->>C: attach baggage token (langfuse_trace_id) C->>C: yield Langfuse span wrapper C->>C: finally: detach baggage token C->>SP: on_end(span) SP->>SP: project/scope/filter checks alt should export SP->>BS: super().on_end(span) end SP->>SP: finally: _cleanup_app_root_state(span)Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Merge branch 'main' into add-app-root-sp..." | Re-trigger Greptile
Context used:
Learned From
langfuse/langfuse-python#1387