From 8b7e39560502935a4c6c8587a9f0bebabbcd2429 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 26 May 2025 08:37:56 +0800 Subject: [PATCH 1/2] confighttp: make server span naming spec-compliant Update the confighttp server span names to use the low-cardnality request pattern, rather than the full client-specified path, as described by the specification. --- .../confighttp-server-pattern-spanname.yaml | 27 +++++++++ config/confighttp/confighttp.go | 12 +++- config/confighttp/confighttp_test.go | 58 +++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 .chloggen/confighttp-server-pattern-spanname.yaml diff --git a/.chloggen/confighttp-server-pattern-spanname.yaml b/.chloggen/confighttp-server-pattern-spanname.yaml new file mode 100644 index 00000000000..faf27f58e2e --- /dev/null +++ b/.chloggen/confighttp-server-pattern-spanname.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: confighttp + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Update the HTTP server span naming to use the HTTP method nad route pattern instead of the path. + +# One or more tracking issues or pull requests related to the change +issues: [12468] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + The HTTP server span name will now be formatted as ` `. + If a route pattern is not available, it will fall back to ``. + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 9b91eb68be4..bd2b3f011ae 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -517,7 +517,17 @@ func (hss *ServerConfig) ToServer(ctx context.Context, host component.Host, sett otelhttp.WithTracerProvider(settings.TracerProvider), otelhttp.WithPropagators(otel.GetTextMapPropagator()), otelhttp.WithSpanNameFormatter(func(_ string, r *http.Request) string { - return r.URL.Path + // https://opentelemetry.io/docs/specs/semconv/http/http-spans/#name: + // + // "HTTP span names SHOULD be {method} {target} if there is a (low-cardinality) target available. + // If there is no (low-cardinality) {target} available, HTTP span names SHOULD be {method}. + // ... + // Instrumentation MUST NOT default to using URI path as a {target}." + // + if r.Pattern != "" { + return r.Method + " " + r.Pattern + } + return r.Method }), otelhttp.WithMeterProvider(settings.MeterProvider), }, diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 651defda429..6b393190125 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -1575,3 +1575,61 @@ func TestDefaultHTTPServerSettings(t *testing.T) { assert.Equal(t, time.Duration(0), httpServerSettings.ReadTimeout) assert.Equal(t, 1*time.Minute, httpServerSettings.ReadHeaderTimeout) } + +func TestHTTPServerTelemetry_Tracing(t *testing.T) { + // Create a pattern route. The server name the span after the + // pattern rather than the client-specified path. + mux := http.NewServeMux() + mux.HandleFunc("/b/{bucket}/o/{objectname...}", func(http.ResponseWriter, *http.Request) {}) + + type testcase struct { + handler http.Handler + expectedSpanName string + } + + for name, testcase := range map[string]testcase{ + "pattern": { + handler: mux, + expectedSpanName: "GET /b/{bucket}/o/{objectname...}", + }, + "no_pattern": { + handler: http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), + expectedSpanName: "GET", + }, + } { + t.Run(name, func(t *testing.T) { + telemetry := componenttest.NewTelemetry() + config := NewDefaultServerConfig() + config.Endpoint = "localhost:0" + config.TLSSetting = nil + srv, err := config.ToServer( + context.Background(), + componenttest.NewNopHost(), + telemetry.NewTelemetrySettings(), + testcase.handler, + ) + require.NoError(t, err) + + done := make(chan struct{}) + lis, err := config.ToListener(context.Background()) + require.NoError(t, err) + go func() { + defer close(done) + _ = srv.Serve(lis) + }() + defer func() { + assert.NoError(t, srv.Close()) + <-done + }() + + resp, err := http.Get(fmt.Sprintf("http://%s/b/bucket123/o/object456/segment", lis.Addr())) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + resp.Body.Close() + + spans := telemetry.SpanRecorder.Ended() + require.Len(t, spans, 1) + assert.Equal(t, testcase.expectedSpanName, spans[0].Name()) + }) + } +} From 3e1c30d6e8d13c4bd0bacb62223bd070f5df2844 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Wed, 28 May 2025 08:38:50 -0700 Subject: [PATCH 2/2] Update .chloggen/confighttp-server-pattern-spanname.yaml --- .chloggen/confighttp-server-pattern-spanname.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/confighttp-server-pattern-spanname.yaml b/.chloggen/confighttp-server-pattern-spanname.yaml index faf27f58e2e..ad10abc0f25 100644 --- a/.chloggen/confighttp-server-pattern-spanname.yaml +++ b/.chloggen/confighttp-server-pattern-spanname.yaml @@ -7,7 +7,7 @@ change_type: enhancement component: confighttp # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Update the HTTP server span naming to use the HTTP method nad route pattern instead of the path. +note: Update the HTTP server span naming to use the HTTP method and route pattern instead of the path. # One or more tracking issues or pull requests related to the change issues: [12468]