fix(http): stop using raw URL path as http.route for net/http

pkg/inst-api-semconv/instrumenter/http/http_attrs_extractor.go

@@ -209,13 +209,18 @@ func (h *HttpServerAttrsExtractor[REQUEST, RESPONSE, GETTER1, GETTER2, GETTER3])
 	localRootSpan, ok := span.(sdktrace.ReadOnlySpan)
 	if ok && span.IsRecording() {
 		route := h.Base.HttpGetter.GetHttpRoute(request)
-		if !strings.Contains(localRootSpan.Name(), route) {
-			route = localRootSpan.Name()
+		spanName := localRootSpan.Name()
+		if route == "" || (route != "" && !strings.Contains(spanName, route)) {
+			if tmpl := httpRouteFromSpanName(spanName); tmpl != "" {
+				route = tmpl
+			}
+		}
+		if route != "" {
+			attributes = append(attributes, attribute.KeyValue{
+				Key:   semconv.HTTPRouteKey,
+				Value: attribute.StringValue(route),
+			})
 		}
-		attributes = append(attributes, attribute.KeyValue{
-			Key:   semconv.HTTPRouteKey,
-			Value: attribute.StringValue(route),
-		})
 	}
 	if h.Base.AttributesFilter != nil {
 		attributes = h.Base.AttributesFilter(attributes)

pkg/inst-api-semconv/instrumenter/http/http_route.go

@@ -0,0 +1,56 @@
+// Copyright (c) 2024 Alibaba Group Holding Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package http
+
+import "strings"
+
+// httpRouteFromSpanName returns a low-cardinality route template encoded in the
+// span name by framework hooks (gin FullPath, mux template, etc.). Raw request
+// paths such as "/users/123" must not be inferred from span names.
+func httpRouteFromSpanName(spanName string) string {
+	if spanName == "" {
+		return ""
+	}
+	path := spanName
+	if idx := strings.Index(spanName, " "); idx >= 0 {
+		path = spanName[idx+1:]
+	}
+	if path == "" {
+		return ""
+	}
+	if strings.Contains(path, "{") && strings.Contains(path, "}") {
+		return path
+	}
+	for _, segment := range strings.Split(path, "/") {
+		if strings.HasPrefix(segment, ":") || strings.HasPrefix(segment, "*") {
+			return path
+		}
+	}
+	return ""
+}
+
+// NetHttpRouteFromPattern normalizes net/http Request.Pattern (Go 1.22+).
+func NetHttpRouteFromPattern(pattern string) string {
+	if pattern == "" {
+		return ""
+	}
+	if _, rest, ok := strings.Cut(pattern, " "); ok {
+		pattern = rest
+	}
+	if idx := strings.IndexByte(pattern, '/'); idx >= 0 {
+		return pattern[idx:]
+	}
+	return ""
+}

pkg/inst-api-semconv/instrumenter/http/http_route_test.go

@@ -0,0 +1,56 @@
+// Copyright (c) 2024 Alibaba Group Holding Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package http
+
+import "testing"
+
+func TestHttpRouteFromSpanName(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		want string
+	}{
+		{"GET /users/123", ""},
+		{"/user/:name", "/user/:name"},
+		{"/test/{key}", "/test/{key}"},
+		{"/hertz/:version/*action", "/hertz/:version/*action"},
+		{"GET /a", ""},
+		{"GET /time/12:30", ""},
+		{"GET /files/*path", "/files/*path"},
+		{"GET /literal*value", ""},
+	} {
+		if got := httpRouteFromSpanName(tc.name); got != tc.want {
+			t.Fatalf("%q: got %q want %q", tc.name, got, tc.want)
+		}
+	}
+}
+
+func TestNetHttpRouteFromPattern(t *testing.T) {
+	for _, tc := range []struct {
+		pattern string
+		want    string
+	}{
+		{"GET /users/{id}", "/users/{id}"},
+		{"/health", "/health"},
+		{"example.com/x", "/x"},
+		{"GET example.com/x/{id}", "/x/{id}"},
+		{"/files/{path...}", "/files/{path...}"},
+		{"/{$}", "/{$}"},
+		{"", ""},
+	} {
+		if got := NetHttpRouteFromPattern(tc.pattern); got != tc.want {
+			t.Fatalf("%q: got %q want %q", tc.pattern, got, tc.want)
+		}
+	}
+}

pkg/rules/echo/echo_server_setup.go

@@ -40,7 +40,7 @@ func otelTraceMiddleware() echo.MiddlewareFunc {
 				c.Error(err)
 			}
 			lcs := trace.LocalRootSpanFromGLS()
-			if lcs != nil && c.Path() != "" && c.Request() != nil && c.Request().URL != nil && (c.Request().URL.Path != c.Path()) {
+			if lcs != nil && c.Path() != "" {
 				lcs.SetName(c.Path())
 			}
 			return

pkg/rules/gin/gin_html_setup.go

@@ -32,7 +32,7 @@ func htmlOnEnter(call api.CallContext, c *gin.Context, code int, name string, ob
 		return
 	}
 	lcs := trace.LocalRootSpanFromGLS()
-	if lcs != nil && c.FullPath() != "" && c.Request != nil && c.Request.URL != nil && (c.FullPath() != c.Request.URL.Path) {
+	if lcs != nil && c.FullPath() != "" {
 		lcs.SetName(c.FullPath())
 	}
 }

pkg/rules/gin/gin_server_setup.go

@@ -32,7 +32,7 @@ func nextOnEnter(call api.CallContext, c *gin.Context) {
 		return
 	}
 	lcs := trace.LocalRootSpanFromGLS()
-	if lcs != nil && c.FullPath() != "" && c.Request != nil && c.Request.URL != nil && (c.FullPath() != c.Request.URL.Path) {
+	if lcs != nil && c.FullPath() != "" {
 		lcs.SetName(c.FullPath())
 	}
 }

pkg/rules/gorestful/gorestful_server_setup.go

@@ -75,7 +75,7 @@ var filterRest = func(req *restful.Request, resp *restful.Response, chain *restf
 		return
 	}
 	lcs := trace.LocalRootSpanFromGLS()
-	if lcs != nil && req.SelectedRoutePath() != "" && req.Request != nil && req.Request.URL != nil && (req.SelectedRoutePath() != req.Request.URL.Path) {
+	if lcs != nil && req.SelectedRoutePath() != "" {
 		lcs.SetName(req.SelectedRoutePath())
 	}
 }

pkg/rules/http/net_http_data_type.go

@@ -27,6 +27,7 @@ type netHttpRequest struct {
 	isTls          bool
 	header         http.Header
 	version        string
+	routePattern   string
 	requestHeaders string
 	requestBody    string
 }

pkg/rules/http/net_http_otel_instrumenter.go

@@ -210,7 +210,7 @@ func (n netHttpServerAttrsGetter) GetNetworkPeerPort(request *netHttpRequest, re
 }
 
 func (n netHttpServerAttrsGetter) GetHttpRoute(request *netHttpRequest) string {
-	return request.url.Path
+	return http.NetHttpRouteFromPattern(request.routePattern)
 }
 
 func BuildNetHttpClientOtelInstrumenter() *instrumenter.PropagatingToDownstreamInstrumenter[*netHttpRequest, *netHttpResponse] {

pkg/rules/http/server_setup.go

@@ -23,6 +23,8 @@ import (
 	_ "unsafe"
 
 	"github.com/alibaba/loongsuite-go/pkg/api"
+	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/trace"
 )
 
 var netHttpServerInstrumenter = BuildNetHttpServerOtelInstrumenter()
@@ -39,12 +41,12 @@ func serverOnEnter(call api.CallContext, _ interface{}, w http.ResponseWriter, r
 		return
 	}
 	request := &netHttpRequest{
-		method:  r.Method,
-		url:     r.URL,
-		header:  r.Header,
-		version: getProtocolVersion(r.ProtoMajor, r.ProtoMinor),
-		host:    r.Host,
-		isTls:   r.TLS != nil,
+		method:       r.Method,
+		url:          r.URL,
+		header:       r.Header,
+		version:      getProtocolVersion(r.ProtoMajor, r.ProtoMinor),
+		host:         r.Host,
+		isTls:        r.TLS != nil,
 	}
 	request.requestHeaders = netHttpCaptureConfig.captureHeaders(r.Header)
 	request.requestBody = captureHTTPRequestBody(r, netHttpCaptureConfig)
@@ -58,9 +60,10 @@ func serverOnEnter(call api.CallContext, _ interface{}, w http.ResponseWriter, r
 		}
 		call.SetParam(1, x1)
 	}
-	data := make(map[string]interface{}, 2)
+	data := make(map[string]interface{}, 3)
 	data["ctx"] = ctx
 	data["request"] = request
+	data["httpRequest"] = r
 	call.SetData(data)
 	return
 }
@@ -79,6 +82,35 @@ func serverOnExit(call api.CallContext) {
 	if !ok {
 		return
 	}
+	if rawRequest, ok := data["httpRequest"].(*http.Request); ok {
+		// ServeMux fills Pattern after this hook's entry. Middleware that passes
+		// r.WithContext(...) to the mux replaces the request pointer, so the
+		// outer hook cannot observe that copy's pattern.
+		request.routePattern = rawRequest.Pattern
+		span := trace.SpanFromContext(ctx)
+		// Framework hooks set a validated static route as the span name. When
+		// that route equals the raw path, retain it without inferring arbitrary
+		// request paths as route templates.
+		if request.routePattern == "" && rawRequest.URL != nil {
+			if readable, ok := span.(sdktrace.ReadOnlySpan); ok &&
+				readable.Name() == rawRequest.URL.Path {
+				request.routePattern = rawRequest.URL.Path
+			}
+		}
+		// A framework may already have published a more specific route template
+		// (for example go-restful's /users/{id}). Do not replace it with the
+		// outer ServeMux catch-all pattern. A framework static route equals the
+		// raw path, so it is still safe to add the HTTP method in that case.
+		spanName := ""
+		if readable, ok := span.(sdktrace.ReadOnlySpan); ok {
+			spanName = readable.Name()
+		}
+		if route := (netHttpServerAttrsGetter{}).GetHttpRoute(request); route != "" &&
+			(spanName == request.method ||
+				(rawRequest.URL != nil && spanName == rawRequest.URL.Path)) {
+			span.SetName(request.method + " " + route)
+		}
+	}
 	if p, ok := call.GetParam(1).(http.ResponseWriter); ok {
 		if w1, ok := p.(*writerWrapper); ok {
 			netHttpServerInstrumenter.End(ctx, request, &netHttpResponse{

pkg/rules/iris/router_setup.go

@@ -32,7 +32,7 @@ func irisHttpOnEnter(call api.CallContext, _ interface{}, iCtx *iContext.Context
 	}
 	r := iCtx.Request()
 	lcs := trace.LocalRootSpanFromGLS()
-	if lcs != nil && r != nil && iCtx.Path() != "" && r.URL != nil && (iCtx.Path() != r.URL.Path) {
+	if lcs != nil && r != nil && iCtx.Path() != "" {
 		lcs.SetName(iCtx.Path())
 	}
 }

pkg/rules/mux/mux_server_setup.go

@@ -46,7 +46,7 @@ func muxRoute130OnEnter(call api.CallContext, req *http.Request, route interface
 			r, ok := route.(*mux.Route)
 			if ok {
 				tmpl, err := r.GetPathTemplate()
-				if err == nil && req.URL != nil && tmpl != req.URL.Path {
+				if err == nil && tmpl != "" {
 					lcs.SetName(tmpl)
 				}
 			}
@@ -65,7 +65,7 @@ func muxRoute174OnEnter(call api.CallContext, req *http.Request, route *mux.Rout
 		lcs := trace.LocalRootSpanFromGLS()
 		if lcs != nil && route != nil {
 			tmpl, err := route.GetPathTemplate()
-			if err == nil && req.URL != nil && tmpl != req.URL.Path {
+			if err == nil && tmpl != "" {
 				lcs.SetName(tmpl)
 			}
 		}

test/otel_tests.go

@@ -26,5 +26,7 @@ func TestSpanFromContext(t *testing.T, env ...string) {
 	UseApp("otel")
 	RunGoBuild(t, "go", "build", "--", "test_span_from_context.go")
 	stdout, _ := RunApp(t, "test_span_from_context", env...)
-	ExpectContains(t, stdout, "GET /otel")
+	// The ServeMux route is applied when the server hook exits, after this
+	// application prints the in-flight span from inside the handler.
+	ExpectContains(t, stdout, "GET")
 }

test/verifier/verifier.go

@@ -85,7 +85,15 @@ func VerifyHttpServerAttributes(span tracetest.SpanStub, name, method, protocolN
 	Assert(GetAttribute(span.Attributes, "url.scheme").AsString() == scheme, "Except url scheme to be %s, got %s", scheme, GetAttribute(span.Attributes, "url.scheme").AsString())
 	Assert(GetAttribute(span.Attributes, "url.path").AsString() == path, "Except url path to be %s, got %s", path, GetAttribute(span.Attributes, "url.path").AsString())
 	Assert(GetAttribute(span.Attributes, "url.query").AsString() == query, "Except url query to be %s, got %s", query, GetAttribute(span.Attributes, "url.query").AsString())
-	Assert(GetAttribute(span.Attributes, "http.route").AsString() == route, "Except http route to be %s, got %s", route, GetAttribute(span.Attributes, "http.route").AsString())
+	if route == "" {
+		for _, attr := range span.Attributes {
+			if attr.Key == semconv.HTTPRouteKey {
+				Assert(false, "http.route should be omitted, got %q", attr.Value.AsString())
+			}
+		}
+	} else {
+		Assert(GetAttribute(span.Attributes, "http.route").AsString() == route, "Except http route to be %s, got %s", route, GetAttribute(span.Attributes, "http.route").AsString())
+	}
 	Assert(GetAttribute(span.Attributes, "http.response.status_code").AsInt64() == statusCode, "Except status code to be %d, got %d", statusCode, GetAttribute(span.Attributes, "http.response.status_code").AsInt64())
 }
 

test/zap/test_zap_with_field.go

@@ -50,7 +50,7 @@ func main() {
 	port, err := strconv.Atoi(Url.Port())
 	verifier.WaitAndAssertTraces(func(stubs []tracetest.SpanStubs) {
 		verifier.VerifyHttpClientAttributes(stubs[0][0], "GET", "GET", ts_a.URL, "http", "1.1", "tcp", "ipv4", "", Url.Host, 200, 0, int64(port))
-		verifier.VerifyHttpServerAttributes(stubs[0][1], "GET /", "GET", "http", "tcp", "ipv4", "", Url.Host, "Go-http-client/1.1", "http", "/", "", "/", 200)
+		verifier.VerifyHttpServerAttributes(stubs[0][1], "GET", "GET", "http", "tcp", "ipv4", "", Url.Host, "Go-http-client/1.1", "http", "/", "", "", 200)
 		if stubs[0][1].Parent.TraceID().String() != stubs[0][0].SpanContext.TraceID().String() {
 			log.Fatal("span 1 should be child of span 0")
 		}