fix(telemetry): use supported OpenTelemetry logging handler - #973
fix(telemetry): use supported OpenTelemetry logging handler#973seonghobae wants to merge 5 commits into
Conversation
|
Warning Review limit reachedNext included review available in 30 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review finding — the handler migration itself is directionally correct, but this PR was Ready while its owned production module coverage is explicitly 95%. The LineageWeave maintainer contract requires owned production Test/Edge Case Coverage 100%; a documented 95% gap is therefore a repair finding, not merge-ready evidence. I returned the PR to Draft. Keep the supported |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review: the handler migration and the newly added _failure_counter() failure boundary are causal and keep telemetry fail-open. The provider is retained early enough for shutdown ownership if handler setup fails, and the application failure classification is no longer replaceable by a metrics-provider exception. I found no new source-semantics defect in this six-file delta. However, the PR's own current evidence still reports lineageweave/observability.py at 96% statement/branch coverage. Under this repository's owned-production 100% Test/Edge Coverage contract, that is a real verification gap, so keeping this head Draft is correct. Do not convert the focused 32-test GREEN or this COMMENT into merge-ready evidence; close the remaining executable branches with realistic tests, then require fresh exact-head full-suite/security/CodeQL and qualifying independent approval.
seonghobae
added
bug
ChatGPT Codex Connector
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
AGENTS.md— repository behaviordocs/product-technical-gap-baseline.md— operator or user guidancelineageweave/observability.py— Python module behaviorpyproject.toml— repository behaviortests/test_observability.py— regression suitetests/test_observability_telemetry.py— regression suiteuv.lock— repository behavior
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: AGENTS.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: AGENTS.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: product-technical-gap-baseline.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: product-technical-gap-baseline.md"]
R2 --> V2["docs review"]
Evidence --> S3["Python: observability.py"]
S3 --> I3["Python module behavior"]
I3 --> R3["Review risk: Python: observability.py"]
R3 --> V3["pytest plus coverage"]
Evidence --> S4["Repository file: pyproject.toml"]
S4 --> I4["repository behavior"]
I4 --> R4["Review risk: Repository file: pyproject.toml"]
R4 --> V4["required checks"]
Evidence --> S5["Test: test_observability.py (2 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test: test_observability.py (2 files)"]
R5 --> V5["targeted test run"]
Evidence --> S6["Repository file: uv.lock"]
S6 --> I6["repository behavior"]
I6 --> R6["Review risk: Repository file: uv.lock"]
R6 --> V6["required checks"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
182d3c9d4c5f2a8ab2d63e77b8a9ced663a183f6 - Workflow run: 34113287611
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: AGENTS.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: AGENTS.md"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: product-technical-gap-baseline.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: product-technical-gap-baseline.md"]
R2 --> V2["docs review"]
Evidence --> S3["Python: observability.py"]
S3 --> I3["Python module behavior"]
I3 --> R3["Review risk: Python: observability.py"]
R3 --> V3["pytest plus coverage"]
Evidence --> S4["Repository file: pyproject.toml"]
S4 --> I4["repository behavior"]
I4 --> R4["Review risk: Repository file: pyproject.toml"]
R4 --> V4["required checks"]
Evidence --> S5["Test: test_observability.py (2 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test: test_observability.py (2 files)"]
R5 --> V5["targeted test run"]
Evidence --> S6["Repository file: uv.lock"]
S6 --> I6["repository behavior"]
I6 --> R6["Review risk: Repository file: uv.lock"]
R6 --> V6["required checks"]
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
1 participant
The hosted backend suite emitted the deprecated OpenTelemetry SDK
LoggingHandlerwarning. Telemetry setup could also orphan a log provider after handler construction failed, and metric initialization could replace the application's original error.Use the supported logging-instrumentation handler with the existing explicit provider, WARNING threshold, and dedicated bounded logger. Register provider ownership immediately after allocation for normal shutdown cleanup. Include counter acquisition in the existing fail-open metric boundary. Do not introduce global instrumentation, root-handler mutation, or a LogRecord-factory replacement. ADR 0122 remains the governing boundary.
RED → causal repair → exact coverage GREEN
Causal RED assertions reproduced the deprecated handler warning, orphaned provider shutdown, counter-initialization error escape, missing span/propagator branches, unsupported outcome rejection, and partial SDK status support. Historical hosted run
34087122183measured only 96% after 32 tests; an exact-current RED lane34088114733then ran all owned observability test modules and failed--fail-under=100at 97%, identifying the remaining executable branches. Earlier measurement harness failures ran no valid coverage measurement and are not product evidence.Current product head
182d3c9d4c5f2a8ab2d63e77b8a9ced663a183f6adds only causal degraded-SDK/privacy regressions for those branches. Exact-head verification run34088301425, job101636507846, is terminal GREEN: 44 tests passed,lineageweave/observability.pyreports 197 statements / 0 missed and 68 branches / 0 partial at 100%,coverage report --fail-under=100passed without exclusions or denominator reduction, and compileall passed.The two
github-code-qualityinline findings on the newpytest.raisesregression were verified as false positives: the explicit exception is caught by the context manager, execution continues to the span assertions, and theexportervariable is consumed there. The exact 44-test run executes those statements and branch coverage is 100%. Both threads were answered with that evidence and resolved without rewriting working tests merely for the analyzer.Current hosted boundary
Exact-head standard Tests
34088155091, PROV-O34088154928, Ontology Pages34088154989, and SAST34088128228are terminal GREEN on182d3c9d4c5f2a8ab2d63e77b8a9ced663a183f6.Security
34088128243is terminal failure only at Dependency Review availability. Exact-head checkout was verified; OSV, Trivy, and Scorecard are GREEN. Dependency Review's support probe called the authoritative GitHub dependency comparison formain@83eba56149eb802cd63642c507c324c9976ec78e...182d3c9d4c5f2a8ab2d63e77b8a9ced663a183f6and received HTTP 403, so the gate correctly failed closed instead of fabricating dependency evidence.CodeQL
34088128190is also terminal fail-closed: changed-language detection succeeded and javascript-typescript/actions/python compatibility jobs dispatched the current-head scan, but each compatibility job lacks the required authenticated terminal central receipt. These are central evidence-path failures, not a basis for a LineageWeave-local shim or synthetic success.No qualifying independent current-head APPROVE exists. Therefore this is Ready for Review but not merge-ready. Collector/runtime acceptance, protected merge, deployment, and release remain unproven. Do not substitute the 100% owned-module proof, repository Tests GREEN, resolved COMMENT threads, predecessor evidence, synthetic status, or no-op head churn for those gates.
Official guidance: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/logging/logging.html