tracing: name HTTP spans as "{method} {route}" per OTel semconv#8632
Open
ChrisJr404 wants to merge 1 commit into
Open
tracing: name HTTP spans as "{method} {route}" per OTel semconv#8632ChrisJr404 wants to merge 1 commit into
ChrisJr404 wants to merge 1 commit into
Conversation
OPA emits HTTP spans named after the Prometheus handler label (e.g.
"v1/data") on the server side and "HTTP {METHOD}" on the client side.
The OpenTelemetry HTTP semantic conventions specify span names of the
form "{method} {route}" / "{method} {target}", which makes traces easier
to read in tools like Jaeger.
Install default span-name formatters on both the otelhttp Handler and
Transport produced by features/tracing:
- server: "{METHOD} {operation}" where operation is the route label
OPA already supplies to NewHandler (e.g. "POST v1/data", "GET health").
- client: "{METHOD} {url.path}" with a "{METHOD}" fallback when no
path is available, replacing otelhttp's legacy "HTTP {METHOD}".
The defaults are prepended to the converted otelhttp options so that any
user-supplied WithSpanNameFormatter passed via tracing.Options still
takes precedence (otelhttp applies options in order, last write wins).
Existing assertions in the distributed-tracing e2e test are updated to
match the new names. New unit tests in v1/features/tracing cover the
formatter logic directly and end-to-end through otelhttp, including the
user-override case.
Closes open-policy-agent#8612
Signed-off-by: ChrisJr404 <chris@hacknow.com>
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
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.
Closes #8612.
OPA's tracing emits HTTP spans named after the Prometheus handler label on the server side (e.g.
v1/data,health) andHTTP {METHOD}on the client side. The OpenTelemetry HTTP semantic conventions prescribe span names of the form{method} {route}(server) and{method} {target}(client), which makes traces in tools like Jaeger far easier to scan.Change
In
v1/features/tracing/tracing.go, install defaultWithSpanNameFormatteroptions on both the otelhttpHandlerandTransport:"{METHOD} {operation}", whereoperationis the route label OPA already passes toNewHandler. SoPOST v1/data,GET v1/data,DELETE v1/policies,GET health, etc."{METHOD} {url.path}", falling back to"{METHOD}"when no path is available, replacing otelhttp's legacy"HTTP {METHOD}".The defaults are prepended to the converted otelhttp options. Because otelhttp applies options in order with last-writer-wins semantics on
SpanNameFormatter, anyone passing their ownWithSpanNameFormatterthroughtracing.Optionsstill wins. A dedicated unit test covers that override path.Notes on naming
The reporter's example used
POST /v1/data/:path(the registered Go 1.22 mux pattern with the{path...}capture). I used the existingoperationlabel (v1/data) rather than the full mux pattern for two reasons:{path...}would collapse the same way at the visualization layer but adds noise to the span name.NewHandleralready receives, so the formatter is route-stable without any plumbing changes through the server.If the maintainers would prefer the full registered pattern (e.g.
POST /v1/data/{path...}), I'm happy to wireinstrumentHandlerto pass the pattern alongside the Prom label and switch the formatter to use it.Tests
v1/features/tracing/tracing_test.gocover:serverSpanNametable tests (method, empty operation, empty method, nil request).clientSpanNametable tests (with/without path, empty method, nil request).otelhttp.NewHandler+httptest.Server: span name isPOST v1/data.otelhttp.NewTransport: client span name isPOST /v1/logs.WithSpanNameFormatteroverrides the default.v1/test/e2e/distributedtracing/distributedtracing_test.goto match the new server (POST v0/data,GET v1/data,POST authz,POST v1/data) and client (POST /logs) names.Local runs that pass on this branch:
Compat
Anyone parsing span names from Jaeger/Tempo/etc. will see a string change (
v1/datatoPOST v1/data,HTTP POSTtoPOST /v1/logs). That seems acceptable: the existing names didn't conform to the spec, the OTel attributes (http.request.method,url.path, etc.) on the span are unchanged, and the new names are what the OTel docs say tracing UIs should be grouping on.Signed-off-by: ChrisJr404 chris@hacknow.com