feat(otelfiber): align trace attributes with semantic convention `1.1…

…2.0`

Includes:
- making `http.server_name` optional - this attribute is only ever mentioned as an example in the "[Observability Primer](https://opentelemetry.io/docs/concepts/observability-primer/)" (see https://opentelemetry.io/search/?q=http.server_name) and seems to have been replaced-by usage of `[net.host.name](https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/span-general/#nethostname)` instead.
- addition of recommended attributes `http.flavor` and `http.response_content_length`
- addition of conditionally required attribute `net.host.port`
- removed optional/unmentioned attribute `net.host.ip`; this was being incorrectly set to the client IP instead of the the host. Client IP is already available via `http.client_ip`.

See https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/ for more

otelfiber/README.md

@@ -22,7 +22,7 @@ go get -u github.com/gofiber/contrib/otelfiber
 ### Signature
 
 ```
-otelfiber.Middleware(service string, opts ...Option) fiber.Handler
+otelfiber.Middleware(opts ...Option) fiber.Handler
 ```
 
 ### Usage

otelfiber/fiber.go

@@ -2,9 +2,7 @@ package otelfiber
 
 import (
 	"context"
-	"encoding/base64"
 	"net/http"
-	"strings"
 	"time"
 
 	"github.com/gofiber/fiber/v2"
@@ -24,7 +22,6 @@ import (
 const (
 	tracerKey           = "gofiber-contrib-tracer-fiber"
 	instrumentationName = "github.com/gofiber/contrib/otelfiber"
-	defaultServiceName  = "fiber-server"
 
 	metricNameHttpServerDuration       = "http.server.duration"
 	metricNameHttpServerRequestSize    = "http.server.request.size"
@@ -33,28 +30,28 @@ const (
 )
 
 // Middleware returns fiber handler which will trace incoming requests.
-func Middleware(service string, opts ...Option) fiber.Handler {
-	if service == "" {
-		service = defaultServiceName
-	}
+func Middleware(opts ...Option) fiber.Handler {
 	cfg := config{}
 	for _, opt := range opts {
 		opt.apply(&cfg)
 	}
+
 	if cfg.TracerProvider == nil {
 		cfg.TracerProvider = otel.GetTracerProvider()
 	}
 	tracer := cfg.TracerProvider.Tracer(
 		instrumentationName,
 		oteltrace.WithInstrumentationVersion(otelcontrib.SemVersion()),
 	)
+
 	if cfg.MeterProvider == nil {
 		cfg.MeterProvider = global.MeterProvider()
 	}
 	meter := cfg.MeterProvider.Meter(
 		instrumentationName,
 		metric.WithInstrumentationVersion(otelcontrib.SemVersion()),
 	)
+
 	httpServerDuration, err := meter.SyncFloat64().Histogram(metricNameHttpServerDuration, instrument.WithUnit(unit.Milliseconds), instrument.WithDescription("measures the duration inbound HTTP requests"))
 	if err != nil {
 		otel.Handle(err)
@@ -71,6 +68,7 @@ func Middleware(service string, opts ...Option) fiber.Handler {
 	if err != nil {
 		otel.Handle(err)
 	}
+
 	if cfg.Propagators == nil {
 		cfg.Propagators = otel.GetTextMapPropagator()
 	}
@@ -96,29 +94,12 @@ func Middleware(service string, opts ...Option) fiber.Handler {
 		})
 
 		ctx := cfg.Propagators.Extract(savedCtx, propagation.HeaderCarrier(reqHeader))
+
 		opts := []oteltrace.SpanStartOption{
-			oteltrace.WithAttributes(
-				// utils.CopyString: we need to copy the string as fasthttp strings are by default
-				// mutable so it will be unsafe to use in this middleware as it might be used after
-				// the handler returns.
-				semconv.HTTPServerNameKey.String(service),
-				semconv.HTTPMethodKey.String(utils.CopyString(c.Method())),
-				semconv.HTTPTargetKey.String(string(utils.CopyBytes(c.Request().RequestURI()))),
-				semconv.HTTPURLKey.String(utils.CopyString(c.OriginalURL())),
-				semconv.NetHostIPKey.String(utils.CopyString(c.IP())),
-				semconv.NetHostNameKey.String(utils.CopyString(c.Hostname())),
-				semconv.HTTPUserAgentKey.String(string(utils.CopyBytes(c.Request().Header.UserAgent()))),
-				semconv.HTTPRequestContentLengthKey.Int(c.Request().Header.ContentLength()),
-				semconv.HTTPSchemeKey.String(utils.CopyString(c.Protocol())),
-				semconv.NetTransportTCP),
+			oteltrace.WithAttributes(httpServerTraceAttributesFromRequest(c, cfg)...),
 			oteltrace.WithSpanKind(oteltrace.SpanKindServer),
 		}
-		if username, ok := hasBasicAuth(c.Get(fiber.HeaderAuthorization)); ok {
-			opts = append(opts, oteltrace.WithAttributes(semconv.EnduserIDKey.String(utils.CopyString(username))))
-		}
-		if len(c.IPs()) > 0 {
-			opts = append(opts, oteltrace.WithAttributes(semconv.HTTPClientIPKey.String(utils.CopyString(c.IPs()[0]))))
-		}
+
 		// temporary set to c.Path() first
 		// update with c.Route().Path after c.Next() is called
 		// to get pathRaw
@@ -137,27 +118,34 @@ func Middleware(service string, opts ...Option) fiber.Handler {
 			_ = c.App().Config().ErrorHandler(c, err)
 		}
 
+		// extract common attributes from response
 		responseAttrs := append(
 			semconv.HTTPAttributesFromHTTPStatusCode(c.Response().StatusCode()),
 			semconv.HTTPRouteKey.String(c.Route().Path), // no need to copy c.Route().Path: route strings should be immutable across app lifecycle
 		)
 
-		responseMetricAttrs = append(
-			responseMetricAttrs,
-			responseAttrs...)
 		requestSize := int64(len(c.Request().Body()))
 		responseSize := int64(len(c.Response().Body()))
 
 		defer func() {
+			responseMetricAttrs = append(
+				responseMetricAttrs,
+				responseAttrs...)
+
+			httpServerActiveRequests.Add(savedCtx, -1, requestMetricsAttrs...)
 			httpServerDuration.Record(savedCtx, float64(time.Since(start).Microseconds())/1000, responseMetricAttrs...)
 			httpServerRequestSize.Record(savedCtx, requestSize, responseMetricAttrs...)
 			httpServerResponseSize.Record(savedCtx, responseSize, responseMetricAttrs...)
-			httpServerActiveRequests.Add(savedCtx, -1, requestMetricsAttrs...)
+
 			c.SetUserContext(savedCtx)
 			cancel()
 		}()
 
-		span.SetAttributes(responseAttrs...)
+		span.SetAttributes(
+			append(
+				responseAttrs,
+				semconv.HTTPResponseContentLengthKey.Int64(responseSize),
+			)...)
 		span.SetName(cfg.SpanNameFormatter(c))
 
 		spanStatus, spanMessage := semconv.SpanStatusFromHTTPStatusCodeAndSpanKind(c.Response().StatusCode(), oteltrace.SpanKindServer)
@@ -172,33 +160,3 @@ func Middleware(service string, opts ...Option) fiber.Handler {
 func defaultSpanNameFormatter(ctx *fiber.Ctx) string {
 	return ctx.Route().Path
 }
-
-func hasBasicAuth(auth string) (string, bool) {
-	if auth == "" {
-		return "", false
-	}
-
-	// Check if the Authorization header is Basic
-	if !strings.HasPrefix(auth, "Basic ") {
-		return "", false
-	}
-
-	// Decode the header contents
-	raw, err := base64.StdEncoding.DecodeString(auth[6:])
-	if err != nil {
-		return "", false
-	}
-
-	// Get the credentials
-	creds := utils.UnsafeString(raw)
-
-	// Check if the credentials are in the correct form
-	// which is "username:password".
-	index := strings.Index(creds, ":")
-	if index == -1 {
-		return "", false
-	}
-
-	// Get the username
-	return creds[:index], true
-}

otelfiber/fiber_test.go

@@ -33,7 +33,7 @@ func TestChildSpanFromGlobalTracer(t *testing.T) {
 	var gotSpan oteltrace.Span
 
 	app := fiber.New()
-	app.Use(Middleware("foobar"))
+	app.Use(Middleware())
 	app.Get("/user/:id", func(ctx *fiber.Ctx) error {
 		gotSpan = oteltrace.SpanFromContext(ctx.UserContext())
 		return ctx.SendStatus(http.StatusNoContent)
@@ -50,7 +50,7 @@ func TestChildSpanFromCustomTracer(t *testing.T) {
 	var gotSpan oteltrace.Span
 
 	app := fiber.New()
-	app.Use(Middleware("foobar", WithTracerProvider(provider)))
+	app.Use(Middleware(WithTracerProvider(provider)))
 	app.Get("/user/:id", func(ctx *fiber.Ctx) error {
 		gotSpan = oteltrace.SpanFromContext(ctx.UserContext())
 		return ctx.SendStatus(http.StatusNoContent)
@@ -65,11 +65,17 @@ func TestChildSpanFromCustomTracer(t *testing.T) {
 func TestTrace200(t *testing.T) {
 	sr := new(oteltest.SpanRecorder)
 	provider := oteltest.NewTracerProvider(oteltest.WithSpanRecorder(sr))
+	serverName := "foobar"
 
 	var gotSpan oteltrace.Span
 
 	app := fiber.New()
-	app.Use(Middleware("foobar", WithTracerProvider(provider)))
+	app.Use(
+		Middleware(
+			WithTracerProvider(provider),
+			WithServerName(serverName),
+		),
+	)
 	app.Get("/user/:id", func(ctx *fiber.Ctx) error {
 		gotSpan = oteltrace.SpanFromContext(ctx.UserContext())
 		id := ctx.Params("id")
@@ -83,7 +89,7 @@ func TestTrace200(t *testing.T) {
 
 	mspan, ok := gotSpan.(*oteltest.Span)
 	require.True(t, ok)
-	assert.Equal(t, attribute.StringValue("foobar"), mspan.Attributes()[semconv.HTTPServerNameKey])
+	assert.Equal(t, attribute.StringValue(serverName), mspan.Attributes()[semconv.HTTPServerNameKey])
 
 	// verify traces look good
 	spans := sr.Completed()
@@ -104,7 +110,7 @@ func TestError(t *testing.T) {
 
 	// setup
 	app := fiber.New()
-	app.Use(Middleware("foobar", WithTracerProvider(provider)))
+	app.Use(Middleware(WithTracerProvider(provider)))
 	// configure a handler that returns an error and 5xx status
 	// code
 	app.Get("/server_err", func(ctx *fiber.Ctx) error {
@@ -118,7 +124,6 @@ func TestError(t *testing.T) {
 	require.Len(t, spans, 1)
 	span := spans[0]
 	assert.Equal(t, "/server_err", span.Name())
-	assert.Equal(t, attribute.StringValue("foobar"), span.Attributes()["http.server_name"])
 	assert.Equal(t, attribute.IntValue(http.StatusInternalServerError), span.Attributes()["http.status_code"])
 	assert.Equal(t, attribute.StringValue("oh no"), span.Events()[0].Attributes[semconv.ExceptionMessageKey])
 	// server errors set the status
@@ -133,7 +138,7 @@ func TestErrorOnlyHandledOnce(t *testing.T) {
 			return fiber.NewError(http.StatusInternalServerError, err.Error())
 		},
 	})
-	app.Use(Middleware("test-service"))
+	app.Use(Middleware())
 	app.Get("/", func(ctx *fiber.Ctx) error {
 		return errors.New("mock error")
 	})
@@ -171,7 +176,7 @@ func TestPropagationWithGlobalPropagators(t *testing.T) {
 	otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(r.Header))
 
 	app := fiber.New()
-	app.Use(Middleware("foobar", WithTracerProvider(provider)))
+	app.Use(Middleware(WithTracerProvider(provider)))
 	app.Get("/user/:id", func(ctx *fiber.Ctx) error {
 		gotSpan = oteltrace.SpanFromContext(ctx.UserContext())
 		return ctx.SendStatus(http.StatusNoContent)
@@ -198,7 +203,7 @@ func TestPropagationWithCustomPropagators(t *testing.T) {
 	b3.Inject(ctx, propagation.HeaderCarrier(r.Header))
 
 	app := fiber.New()
-	app.Use(Middleware("foobar", WithTracerProvider(provider), WithPropagators(b3)))
+	app.Use(Middleware(WithTracerProvider(provider), WithPropagators(b3)))
 	app.Get("/user/:id", func(ctx *fiber.Ctx) error {
 		gotSpan = oteltrace.SpanFromContext(ctx.UserContext())
 		return ctx.SendStatus(http.StatusNoContent)
@@ -259,7 +264,6 @@ func TestMetric(t *testing.T) {
 	app := fiber.New()
 	app.Use(
 		Middleware(
-			serverName,
 			WithMeterProvider(provider),
 			WithPort(port),
 			WithServerName(serverName),

otelfiber/semconv.go

@@ -1,6 +1,9 @@
 package otelfiber
 
 import (
+	"encoding/base64"
+	"strings"
+
 	"github.com/gofiber/fiber/v2"
 	"github.com/gofiber/fiber/v2/utils"
 	"go.opentelemetry.io/otel/attribute"
@@ -26,11 +29,75 @@ func httpServerMetricAttributesFromRequest(c *fiber.Ctx, cfg config) []attribute
 	return attrs
 }
 
+func httpServerTraceAttributesFromRequest(c *fiber.Ctx, cfg config) []attribute.KeyValue {
+	attrs := []attribute.KeyValue{
+		httpFlavorAttribute(c),
+		// utils.CopyString: we need to copy the string as fasthttp strings are by default
+		// mutable so it will be unsafe to use in this middleware as it might be used after
+		// the handler returns.
+		semconv.HTTPMethodKey.String(utils.CopyString(c.Method())),
+		semconv.HTTPRequestContentLengthKey.Int(c.Request().Header.ContentLength()),
+		semconv.HTTPSchemeKey.String(utils.CopyString(c.Protocol())),
+		semconv.HTTPTargetKey.String(string(utils.CopyBytes(c.Request().RequestURI()))),
+		semconv.HTTPURLKey.String(utils.CopyString(c.OriginalURL())),
+		semconv.HTTPUserAgentKey.String(string(utils.CopyBytes(c.Request().Header.UserAgent()))),
+		semconv.NetHostNameKey.String(utils.CopyString(c.Hostname())),
+		semconv.NetTransportTCP,
+	}
+
+	if cfg.Port != nil {
+		attrs = append(attrs, semconv.NetHostPortKey.Int(*cfg.Port))
+	}
+
+	if cfg.ServerName != nil {
+		attrs = append(attrs, semconv.HTTPServerNameKey.String(*cfg.ServerName))
+	}
+
+	if username, ok := hasBasicAuth(c.Get(fiber.HeaderAuthorization)); ok {
+		attrs = append(attrs, semconv.EnduserIDKey.String(utils.CopyString(username)))
+	}
+	clientIP := c.IP()
+	if len(clientIP) > 0 {
+		attrs = append(attrs, semconv.HTTPClientIPKey.String(utils.CopyString(clientIP)))
+	}
+
+	return attrs
+}
 
 func httpFlavorAttribute(c *fiber.Ctx) attribute.KeyValue {
 	if c.Request().Header.IsHTTP11() {
 		return semconv.HTTPFlavorHTTP11
 	}
 
 	return semconv.HTTPFlavorHTTP10
-}
+}
+
+func hasBasicAuth(auth string) (string, bool) {
+	if auth == "" {
+		return "", false
+	}
+
+	// Check if the Authorization header is Basic
+	if !strings.HasPrefix(auth, "Basic ") {
+		return "", false
+	}
+
+	// Decode the header contents
+	raw, err := base64.StdEncoding.DecodeString(auth[6:])
+	if err != nil {
+		return "", false
+	}
+
+	// Get the credentials
+	creds := utils.UnsafeString(raw)
+
+	// Check if the credentials are in the correct form
+	// which is "username:password".
+	index := strings.Index(creds, ":")
+	if index == -1 {
+		return "", false
+	}
+
+	// Get the username
+	return creds[:index], true
+}