refactor(otelgin): draft version of default semantic standard compliance

CHANGELOG.md

@@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Change the span name to be `GET /path` so it complies with the OTel HTTP semantic conventions in `go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho`. (#6365)
 - Record errors instead of setting the `gin.errors` attribute in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#6346)
+- Change the default span name to be `GET /path` so it complies with the OTel HTTP semantic conventions, and set `spanNameFormatter` to a non-public property in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#6381)
 
 ### Fixed
 

instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go

@@ -6,7 +6,9 @@
 package otelgin // import "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin"
 
 import (
-	"fmt"
+	"net/http"
+	"slices"
+	"strings"
 
 	"github.com/gin-gonic/gin"
 
@@ -71,15 +73,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
 			oteltrace.WithAttributes(semconv.HTTPRoute(c.FullPath())),
 			oteltrace.WithSpanKind(oteltrace.SpanKindServer),
 		}
-		var spanName string
-		if cfg.SpanNameFormatter == nil {
-			spanName = c.FullPath()
-		} else {
-			spanName = cfg.SpanNameFormatter(c.Request)
-		}
-		if spanName == "" {
-			spanName = fmt.Sprintf("HTTP %s route not found", c.Request.Method)
-		}
+		spanName := spanNameFormatter(c.FullPath(), c.Request)
 		ctx, span := tracer.Start(ctx, spanName, opts...)
 		defer span.End()
 
@@ -128,3 +122,22 @@ func HTML(c *gin.Context, code int, name string, obj interface{}) {
 	defer span.End()
 	c.HTML(code, name, obj)
 }
+
+func spanNameFormatter(path string, r *http.Request) string {
+	method := strings.ToUpper(r.Method)
+	if !slices.Contains([]string{
+		http.MethodGet, http.MethodHead,
+		http.MethodPost, http.MethodPut,
+		http.MethodPatch, http.MethodDelete,
+		http.MethodConnect, http.MethodOptions,
+		http.MethodTrace,
+	}, method) {
+		method = "HTTP"
+	}
+
+	if path != "" {
+		return method + " " + path
+	}
+
+	return method
+}

instrumentation/github.com/gin-gonic/gin/otelgin/option.go

@@ -15,11 +15,10 @@ import (
 )
 
 type config struct {
-	TracerProvider    oteltrace.TracerProvider
-	Propagators       propagation.TextMapPropagator
-	Filters           []Filter
-	GinFilters        []GinFilter
-	SpanNameFormatter SpanNameFormatter
+	TracerProvider oteltrace.TracerProvider
+	Propagators    propagation.TextMapPropagator
+	Filters        []Filter
+	GinFilters     []GinFilter
 }
 
 // Filter is a predicate used to determine whether a given http.request should
@@ -30,9 +29,6 @@ type Filter func(*http.Request) bool
 // gin.Context has FullPath() method, which returns a matched route full path.
 type GinFilter func(*gin.Context) bool
 
-// SpanNameFormatter is used to set span name by http.request.
-type SpanNameFormatter func(r *http.Request) string
-
 // Option specifies instrumentation configuration options.
 type Option interface {
 	apply(*config)
@@ -83,11 +79,3 @@ func WithGinFilter(f ...GinFilter) Option {
 		c.GinFilters = append(c.GinFilters, f...)
 	})
 }
-
-// WithSpanNameFormatter takes a function that will be called on every
-// request and the returned string will become the Span Name.
-func WithSpanNameFormatter(f func(r *http.Request) string) Option {
-	return optionFunc(func(c *config) {
-		c.SpanNameFormatter = f
-	})
-}

instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go

@@ -7,6 +7,7 @@ package test
 
 import (
 	"errors"
+	"fmt"
 	"html/template"
 	"net/http"
 	"net/http/httptest"
@@ -157,7 +158,7 @@ func TestTrace200(t *testing.T) {
 	spans := sr.Ended()
 	require.Len(t, spans, 1)
 	span := spans[0]
-	assert.Equal(t, "/user/:id", span.Name())
+	assert.Equal(t, "GET /user/:id", span.Name())
 	assert.Equal(t, trace.SpanKindServer, span.SpanKind())
 	attr := span.Attributes()
 	assert.Contains(t, attr, attribute.String("net.host.name", "foobar"))
@@ -193,7 +194,7 @@ func TestError(t *testing.T) {
 	spans := sr.Ended()
 	require.Len(t, spans, 1)
 	span := spans[0]
-	assert.Equal(t, "/server_err", span.Name())
+	assert.Equal(t, "GET /server_err", span.Name())
 	attr := span.Attributes()
 	assert.Contains(t, attr, attribute.String("net.host.name", "foobar"))
 	assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusInternalServerError))
@@ -263,27 +264,44 @@ func TestSpanStatus(t *testing.T) {
 }
 
 func TestSpanName(t *testing.T) {
+	imsb := tracetest.NewInMemoryExporter()
+	provider := sdktrace.NewTracerProvider(
+		sdktrace.WithSyncer(imsb),
+	)
+
 	testCases := []struct {
-		requestPath       string
-		spanNameFormatter otelgin.SpanNameFormatter
-		wantSpanName      string
+		method       string
+		route        string
+		requestPath  string
+		wantSpanName string
 	}{
-		{"/user/1", nil, "/user/:id"},
-		{"/user/1", func(r *http.Request) string { return r.URL.Path }, "/user/1"},
+		// Test for standard methods
+		{http.MethodGet, "/user/:id", "/user/1", "GET /user/:id"},
+		{http.MethodPost, "/user/:id", "/user/1", "POST /user/:id"},
+		{http.MethodPut, "/user/:id", "/user/1", "PUT /user/:id"},
+		{http.MethodPatch, "/user/:id", "/user/1", "PATCH /user/:id"},
+		{http.MethodDelete, "/user/:id", "/user/1", "DELETE /user/:id"},
+		{http.MethodConnect, "/user/:id", "/user/1", "CONNECT /user/:id"},
+		{http.MethodOptions, "/user/:id", "/user/1", "OPTIONS /user/:id"},
+		{http.MethodTrace, "/user/:id", "/user/1", "TRACE /user/:id"},
+		// Test for no route
+		{http.MethodGet, "", "/user/1", "GET"},
+		// Test for invalid method
+		{"INVALID", "/user/:id", "/user/1", "HTTP /user/:id"},
 	}
+
 	for _, tc := range testCases {
-		t.Run(tc.requestPath, func(t *testing.T) {
-			sr := tracetest.NewSpanRecorder()
-			provider := sdktrace.NewTracerProvider()
-			provider.RegisterSpanProcessor(sr)
+		t.Run(fmt.Sprintf("method: %s, route: %s, requestPath: %s", tc.method, tc.route, tc.requestPath), func(t *testing.T) {
+			defer imsb.Reset()
+
 			router := gin.New()
-			router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider), otelgin.WithSpanNameFormatter(tc.spanNameFormatter)))
-			router.GET("/user/:id", func(c *gin.Context) {})
+			router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider)))
+			router.Handle(tc.method, tc.route, func(c *gin.Context) {})
 
-			router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", tc.requestPath, nil))
+			router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(tc.method, tc.requestPath, nil))
 
-			require.Len(t, sr.Ended(), 1, "should emit a span")
-			assert.Equal(t, tc.wantSpanName, sr.Ended()[0].Name(), "span name not correct")
+			require.Len(t, imsb.GetSpans(), 1, "should emit a span")
+			assert.Equal(t, tc.wantSpanName, imsb.GetSpans()[0].Name, "span name not correct")
 		})
 	}
 }
@@ -295,9 +313,6 @@ func TestHTTPRouteWithSpanNameFormatter(t *testing.T) {
 	router := gin.New()
 	router.Use(otelgin.Middleware("foobar",
 		otelgin.WithTracerProvider(provider),
-		otelgin.WithSpanNameFormatter(func(r *http.Request) string {
-			return r.URL.Path
-		}),
 	),
 	)
 	router.GET("/user/:id", func(c *gin.Context) {