fix: give each OTLP signal its own path - #4
Merged
Merged
Conversation
0.5.0 handed one endpoint to both OTLPSpanExporter and OTLPMetricExporter. Passing endpoint= overrides the SDK and is used VERBATIM; only when the SDK reads OTEL_EXPORTER_OTLP_ENDPOINT itself does it append /v1/traces or /v1/metrics. So a service configured with http://collector:4318 POSTed both signals to the collector's root and got 404 for every batch. The service served perfectly and the only evidence was an export error in its own logs, which is why this reached a running cluster before anyone noticed. Found by deploying the template to DOKS, pointing it at a real Collector, and observing that no metric or trace ever arrived. Both endpoint forms are accepted now, because both exist: a base URL from an operator who read the OTel docs, and a full signal URL from one who read this kit's own tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
kit v0.5.1.
The bug
0.5.0 handed one
endpointto bothOTLPSpanExporterandOTLPMetricExporter. Passingendpoint=overrides the SDK and is used verbatim — only when the SDK readsOTEL_EXPORTER_OTLP_ENDPOINTitself does it append/v1/tracesor/v1/metrics.So a service configured with
http://collector:4318posted both signals to the collector's root:The service served perfectly. The only evidence was
Failed to export span batch code: 404in its own logs. That is why it reached a running cluster before anyone noticed — a monitoring pipeline that silently carries nothing looks identical to one nobody has sent data to yet.Particularly ironic given
_tracing.pysays everything except the endpoint is read by the SDK from its standard variables. The endpoint was the one thing it overrode, and it overrode it wrongly.The fix
signal_endpoint()appends the signal path unless the caller already supplied one. Both forms are accepted because both exist in the wild: a base URL from someone who read the OTel docs, and a full signal URL from someone who read this kit's own tests.How it was found
By deploying the template to DOKS, pointing it at a real Collector on the monitoring VM, and observing that no metric or trace ever arrived. No unit test would have caught it: it needs a real collector answering 404 on the wrong path.
205 tests, 100% coverage, ruff and pyright clean.