refactor(otelecho): comply with OTEL span naming specs

CHANGELOG.md

@@ -12,7 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Added support for providing `endpoint`, `pollingIntervalMs` and `initialSamplingRate` using environment variable `OTEL_TRACES_SAMPLER_ARG` in `go.opentelemetry.io/contrib/samples/jaegerremote`. (#6310)
 - Added support exporting logs via OTLP over gRPC in `go.opentelemetry.io/contrib/config`. (#6340)
-- Added support custom span name and refactored the default span name in `go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho`. (#6365)
+
+### Changed
+
+- Change the logic of span to make it comply with OTEL specifications in `go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho`. (#6365)
 
 ### Fixed
 

instrumentation/github.com/labstack/echo/otelecho/config.go

@@ -4,29 +4,17 @@
 package otelecho // import "go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho"
 
 import (
-	"net/http"
-	"strings"
-
 	"github.com/labstack/echo/v4/middleware"
 
 	"go.opentelemetry.io/otel/propagation"
 	oteltrace "go.opentelemetry.io/otel/trace"
 )
 
-// defaultSpanNameFormatter is the default function used for formatting span names.
-var defaultSpanNameFormatter = func(path string, req *http.Request) string {
-	return strings.ToUpper(req.Method) + " " + path
-}
-
-// SpanNameFormatter is a function that takes a path and an HTTP request and returns a span name.
-type SpanNameFormatter func(string, *http.Request) string
-
 // config is used to configure the mux middleware.
 type config struct {
-	TracerProvider    oteltrace.TracerProvider
-	Propagators       propagation.TextMapPropagator
-	Skipper           middleware.Skipper
-	spanNameFormatter SpanNameFormatter
+	TracerProvider oteltrace.TracerProvider
+	Propagators    propagation.TextMapPropagator
+	Skipper        middleware.Skipper
 }
 
 // Option specifies instrumentation configuration options.
@@ -67,12 +55,3 @@ func WithSkipper(skipper middleware.Skipper) Option {
 		cfg.Skipper = skipper
 	})
 }
-
-// WithSpanNameFormatter specifies a function to use for formatting span names.
-func WithSpanNameFormatter(formatter SpanNameFormatter) Option {
-	return optionFunc(func(cfg *config) {
-		if formatter != nil {
-			cfg.spanNameFormatter = formatter
-		}
-	})
-}

instrumentation/github.com/labstack/echo/otelecho/echo.go

@@ -4,7 +4,9 @@
 package otelecho // import "go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho"
 
 import (
-	"fmt"
+	"net/http"
+	"slices"
+	"strings"
 
 	"github.com/labstack/echo/v4"
 	"github.com/labstack/echo/v4/middleware"
@@ -44,10 +46,6 @@ func Middleware(service string, opts ...Option) echo.MiddlewareFunc {
 		cfg.Skipper = middleware.DefaultSkipper
 	}
 
-	if cfg.spanNameFormatter == nil {
-		cfg.spanNameFormatter = defaultSpanNameFormatter
-	}
-
 	return func(next echo.HandlerFunc) echo.HandlerFunc {
 		return func(c echo.Context) error {
 			if cfg.Skipper(c) {
@@ -70,10 +68,7 @@ func Middleware(service string, opts ...Option) echo.MiddlewareFunc {
 				rAttr := semconv.HTTPRoute(path)
 				opts = append(opts, oteltrace.WithAttributes(rAttr))
 			}
-			spanName := cfg.spanNameFormatter(c.Path(), c.Request())
-			if spanName == "" {
-				spanName = fmt.Sprintf("HTTP %s route not found", request.Method)
-			}
+			spanName := spanNameFormatter(c)
 
 			ctx, span := tracer.Start(ctx, spanName, opts...)
 			defer span.End()
@@ -99,3 +94,22 @@ func Middleware(service string, opts ...Option) echo.MiddlewareFunc {
 		}
 	}
 }
+
+func spanNameFormatter(c echo.Context) string {
+	method, path := strings.ToUpper(c.Request().Method), c.Path()
+	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/labstack/echo/otelecho/test/echo_test.go

@@ -206,45 +206,47 @@ func TestSpanNameFormatter(t *testing.T) {
 	provider := trace.NewTracerProvider(trace.WithSyncer(imsb))
 
 	tests := []struct {
-		name      string
-		formatter otelecho.SpanNameFormatter
-		expected  string
+		name     string
+		method   string
+		path     string
+		url      string
+		expected string
 	}{
-		{
-			name:      "default",
-			formatter: nil,
-			expected:  "GET /user/:id",
-		},
-		{
-			name: "custom",
-			formatter: func(path string, _ *http.Request) string {
-				return "custom " + path
-			},
-			expected: "custom /user/:id",
-		},
-		{
-			name: "use request",
-			formatter: func(path string, req *http.Request) string {
-				return req.Method + " " + req.URL.String()
-			},
-			expected: "GET /user/123",
-		},
+		// Test for standard methods
+		{"", http.MethodGet, "/user/:id", "/user/123", "GET /user/:id"},
+		{"", http.MethodHead, "/user/:id", "/user/123", "HEAD /user/:id"},
+		{"", http.MethodPost, "/user/:id", "/user/123", "POST /user/:id"},
+		{"", http.MethodPut, "/user/:id", "/user/123", "PUT /user/:id"},
+		{"", http.MethodPatch, "/user/:id", "/user/123", "PATCH /user/:id"},
+		{"", http.MethodDelete, "/user/:id", "/user/123", "DELETE /user/:id"},
+		{"", http.MethodConnect, "/user/:id", "/user/123", "CONNECT /user/:id"},
+		{"", http.MethodOptions, "/user/:id", "/user/123", "OPTIONS /user/:id"},
+		{"", http.MethodTrace, "/user/:id", "/user/123", "TRACE /user/:id"},
+		{"", http.MethodGet, "/", "/", "GET /"},
+
+		// Test for no route
+		{"", http.MethodGet, "/", "/user/id", "GET"},
+
+		// Test for case-insensitive method
+		{"", "get", "/user/123", "/user/123", "GET /user/123"},
+		{"", "Get", "/user/123", "/user/123", "GET /user/123"},
+		{"", "GET", "/user/:id", "/user/123", "GET /user/:id"},
+
+		// Test for invalid method
+		{"", "INVALID", "/user/123", "/user/123", "HTTP /user/123"},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			defer imsb.Reset()
 
 			router := echo.New()
-			router.Use(otelecho.Middleware("foobar",
-				otelecho.WithSpanNameFormatter(test.formatter),
-				otelecho.WithTracerProvider(provider)),
-			)
-			router.GET("/user/:id", func(c echo.Context) error {
+			router.Use(otelecho.Middleware("foobar", otelecho.WithTracerProvider(provider)))
+			router.Add(test.method, test.path, func(c echo.Context) error {
 				return c.NoContent(http.StatusOK)
 			})
 
-			r := httptest.NewRequest(http.MethodGet, "/user/123", nil)
+			r := httptest.NewRequest(test.method, test.url, nil)
 			w := httptest.NewRecorder()
 			router.ServeHTTP(w, r)
 
@@ -253,26 +255,4 @@ func TestSpanNameFormatter(t *testing.T) {
 			assert.Equal(t, test.expected, spans[0].Name)
 		})
 	}
-
-	t.Run("test default span name formatter with lowercase method", func(t *testing.T) {
-		router := echo.New()
-		router.Use(otelecho.Middleware("foobar",
-			otelecho.WithTracerProvider(provider)),
-		)
-		router.GET("/user/:id", func(c echo.Context) error {
-			return c.NoContent(http.StatusOK)
-		})
-
-		for _, method := range []string{"Get", "GET", "get"} {
-			r := httptest.NewRequest(method, "/user/123", nil)
-			w := httptest.NewRecorder()
-			router.ServeHTTP(w, r)
-
-			spans := imsb.GetSpans()
-			require.Len(t, spans, 1)
-			assert.Equal(t, "GET /user/:id", spans[0].Name)
-
-			imsb.Reset()
-		}
-	})
 }