diff --git a/instrumentation/net/http/otelhttp/common.go b/instrumentation/net/http/otelhttp/common.go index 214acaf581e..5d6e6156b7b 100644 --- a/instrumentation/net/http/otelhttp/common.go +++ b/instrumentation/net/http/otelhttp/common.go @@ -18,13 +18,6 @@ const ( WriteErrorKey = attribute.Key("http.write_error") // if an error occurred while writing a reply, the string of the error (io.EOF is not recorded) ) -// Server HTTP metrics. -const ( - serverRequestSize = "http.server.request.size" // Incoming request bytes total - serverResponseSize = "http.server.response.size" // Incoming response bytes total - serverDuration = "http.server.duration" // Incoming end to end duration, milliseconds -) - // Client HTTP metrics. const ( clientRequestSize = "http.client.request.size" // Outgoing request bytes total diff --git a/instrumentation/net/http/otelhttp/handler.go b/instrumentation/net/http/otelhttp/handler.go index 5169ebf02eb..33580a35b77 100644 --- a/instrumentation/net/http/otelhttp/handler.go +++ b/instrumentation/net/http/otelhttp/handler.go @@ -11,10 +11,7 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/request" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv" - "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconvutil" "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" ) @@ -25,7 +22,6 @@ type middleware struct { server string tracer trace.Tracer - meter metric.Meter propagators propagation.TextMapPropagator spanStartOptions []trace.SpanStartOption readEvent bool @@ -35,10 +31,7 @@ type middleware struct { publicEndpoint bool publicEndpointFn func(*http.Request) bool - traceSemconv semconv.HTTPServer - requestBytesCounter metric.Int64Counter - responseBytesCounter metric.Int64Counter - serverLatencyMeasure metric.Float64Histogram + semconv semconv.HTTPServer } func defaultHandlerFormatter(operation string, _ *http.Request) string { @@ -57,8 +50,6 @@ func NewHandler(handler http.Handler, operation string, opts ...Option) http.Han func NewMiddleware(operation string, opts ...Option) func(http.Handler) http.Handler { h := middleware{ operation: operation, - - traceSemconv: semconv.NewHTTPServer(), } defaultOpts := []Option{ @@ -68,7 +59,6 @@ func NewMiddleware(operation string, opts ...Option) func(http.Handler) http.Han c := newConfig(append(defaultOpts, opts...)...) h.configure(c) - h.createMeasures() return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -79,7 +69,6 @@ func NewMiddleware(operation string, opts ...Option) func(http.Handler) http.Han func (h *middleware) configure(c *config) { h.tracer = c.Tracer - h.meter = c.Meter h.propagators = c.Propagators h.spanStartOptions = c.SpanStartOptions h.readEvent = c.ReadEvent @@ -89,6 +78,7 @@ func (h *middleware) configure(c *config) { h.publicEndpoint = c.PublicEndpoint h.publicEndpointFn = c.PublicEndpointFn h.server = c.ServerName + h.semconv = semconv.NewHTTPServer(c.Meter) } func handleErr(err error) { @@ -97,30 +87,6 @@ func handleErr(err error) { } } -func (h *middleware) createMeasures() { - var err error - h.requestBytesCounter, err = h.meter.Int64Counter( - serverRequestSize, - metric.WithUnit("By"), - metric.WithDescription("Measures the size of HTTP request messages."), - ) - handleErr(err) - - h.responseBytesCounter, err = h.meter.Int64Counter( - serverResponseSize, - metric.WithUnit("By"), - metric.WithDescription("Measures the size of HTTP response messages."), - ) - handleErr(err) - - h.serverLatencyMeasure, err = h.meter.Float64Histogram( - serverDuration, - metric.WithUnit("ms"), - metric.WithDescription("Measures the duration of inbound HTTP requests."), - ) - handleErr(err) -} - // serveHTTP sets up tracing and calls the given next http.Handler with the span // context injected into the request context. func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http.Handler) { @@ -135,7 +101,7 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http ctx := h.propagators.Extract(r.Context(), propagation.HeaderCarrier(r.Header)) opts := []trace.SpanStartOption{ - trace.WithAttributes(h.traceSemconv.RequestTraceAttrs(h.server, r)...), + trace.WithAttributes(h.semconv.RequestTraceAttrs(h.server, r)...), } opts = append(opts, h.spanStartOptions...) @@ -212,8 +178,8 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http statusCode := rww.StatusCode() bytesWritten := rww.BytesWritten() - span.SetStatus(semconv.ServerStatus(statusCode)) - span.SetAttributes(h.traceSemconv.ResponseTraceAttrs(semconv.ResponseTelemetry{ + span.SetStatus(h.semconv.Status(statusCode)) + span.SetAttributes(h.semconv.ResponseTraceAttrs(semconv.ResponseTelemetry{ StatusCode: statusCode, ReadBytes: bw.BytesRead(), ReadError: bw.Error(), @@ -221,26 +187,24 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http WriteError: rww.Error(), })...) - // Add metrics - attributes := append(labeler.Get(), semconvutil.HTTPServerRequestMetrics(h.server, r)...) - if statusCode > 0 { - attributes = append(attributes, semconv.HTTPStatusCode(statusCode)) - } - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - - h.requestBytesCounter.Add(ctx, bw.BytesRead(), o) - h.responseBytesCounter.Add(ctx, bytesWritten, o) - // Use floating point division here for higher precision (instead of Millisecond method). elapsedTime := float64(time.Since(requestStartTime)) / float64(time.Millisecond) - h.serverLatencyMeasure.Record(ctx, elapsedTime, o) + h.semconv.RecordMetrics(ctx, semconv.MetricData{ + ServerName: h.server, + Req: r, + StatusCode: statusCode, + AdditionalAttributes: labeler.Get(), + RequestSize: bw.BytesRead(), + ResponseSize: bytesWritten, + ElapsedTime: elapsedTime, + }) } // WithRouteTag annotates spans and metrics with the provided route name // with HTTP route attribute. func WithRouteTag(route string, h http.Handler) http.Handler { - attr := semconv.NewHTTPServer().Route(route) + attr := semconv.NewHTTPServer(nil).Route(route) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { span := trace.SpanFromContext(r.Context()) span.SetAttributes(attr) diff --git a/instrumentation/net/http/otelhttp/internal/semconv/bench_test.go b/instrumentation/net/http/otelhttp/internal/semconv/bench_test.go index ec2d2794154..b24abba4cbc 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/bench_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/bench_test.go @@ -35,7 +35,7 @@ func BenchmarkHTTPServerRequest(b *testing.B) { RemoteAddr: "127.0.0.1:38738", RequestURI: "/", } - serv := NewHTTPServer() + serv := NewHTTPServer(nil) b.ReportAllocs() b.ResetTimer() diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env.go b/instrumentation/net/http/otelhttp/internal/semconv/env.go index 25970e3e9eb..d95391becec 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env.go @@ -4,6 +4,7 @@ package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv" import ( + "context" "fmt" "net/http" "os" @@ -11,6 +12,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/metric" ) type ResponseTelemetry struct { @@ -23,6 +25,11 @@ type ResponseTelemetry struct { type HTTPServer struct { duplicate bool + + // Old metrics + requestBytesCounter metric.Int64Counter + responseBytesCounter metric.Int64Counter + serverLatencyMeasure metric.Float64Histogram } // RequestTraceAttrs returns trace attributes for an HTTP request received by a @@ -63,15 +70,10 @@ func (s HTTPServer) Route(route string) attribute.KeyValue { return oldHTTPServer{}.Route(route) } -func NewHTTPServer() HTTPServer { - env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE")) - return HTTPServer{duplicate: env == "http/dup"} -} - -// ServerStatus returns a span status code and message for an HTTP status code +// Status returns a span status code and message for an HTTP status code // value returned by a server. Status codes in the 400-499 range are not // returned as errors. -func ServerStatus(code int) (codes.Code, string) { +func (s HTTPServer) Status(code int) (codes.Code, string) { if code < 100 || code >= 600 { return codes.Error, fmt.Sprintf("Invalid HTTP status code %d", code) } @@ -81,6 +83,43 @@ func ServerStatus(code int) (codes.Code, string) { return codes.Unset, "" } +type MetricData struct { + ServerName string + Req *http.Request + StatusCode int + AdditionalAttributes []attribute.KeyValue + + RequestSize int64 + ResponseSize int64 + ElapsedTime float64 +} + +func (s HTTPServer) RecordMetrics(ctx context.Context, md MetricData) { + if s.requestBytesCounter == nil || s.responseBytesCounter == nil || s.serverLatencyMeasure == nil { + // This will happen if an HTTPServer{} is used insted of NewHTTPServer. + return + } + + attributes := oldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) + o := metric.WithAttributeSet(attribute.NewSet(attributes...)) + addOpts := []metric.AddOption{o} // Allocate vararg slice once. + s.requestBytesCounter.Add(ctx, md.RequestSize, addOpts...) + s.responseBytesCounter.Add(ctx, md.ResponseSize, addOpts...) + s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) + + // TODO: Duplicate Metrics +} + +func NewHTTPServer(meter metric.Meter) HTTPServer { + env := strings.ToLower(os.Getenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE")) + duplicate := env == "http/dup" + server := HTTPServer{ + duplicate: duplicate, + } + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = oldHTTPServer{}.createMeasures(meter) + return server +} + type HTTPClient struct { duplicate bool } diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env_test.go b/instrumentation/net/http/otelhttp/internal/semconv/env_test.go new file mode 100644 index 00000000000..5f4a3e391d6 --- /dev/null +++ b/instrumentation/net/http/otelhttp/internal/semconv/env_test.go @@ -0,0 +1,83 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package semconv + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/embedded" + "go.opentelemetry.io/otel/metric/noop" +) + +func TestHTTPServerDoesNotPanic(t *testing.T) { + testCases := []struct { + name string + server HTTPServer + }{ + { + name: "empty", + server: HTTPServer{}, + }, + { + name: "nil meter", + server: NewHTTPServer(nil), + }, + { + name: "with Meter", + server: NewHTTPServer(noop.Meter{}), + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + require.NotPanics(t, func() { + req, err := http.NewRequest("GET", "http://example.com", nil) + require.NoError(t, err) + + _ = tt.server.RequestTraceAttrs("stuff", req) + _ = tt.server.ResponseTraceAttrs(ResponseTelemetry{StatusCode: 200}) + tt.server.RecordMetrics(context.Background(), MetricData{ + ServerName: "stuff", + Req: req, + }) + }) + }) + } +} + +type testInst struct { + embedded.Int64Counter + embedded.Float64Histogram + + intValue int64 + floatValue float64 + attributes []attribute.KeyValue +} + +func (t *testInst) Add(ctx context.Context, incr int64, options ...metric.AddOption) { + t.intValue = incr + cfg := metric.NewAddConfig(options) + attr := cfg.Attributes() + t.attributes = attr.ToSlice() +} + +func (t *testInst) Record(ctx context.Context, value float64, options ...metric.RecordOption) { + t.floatValue = value + cfg := metric.NewRecordConfig(options) + attr := cfg.Attributes() + t.attributes = attr.ToSlice() +} + +func NewTestHTTPServer() HTTPServer { + return HTTPServer{ + requestBytesCounter: &testInst{}, + responseBytesCounter: &testInst{}, + serverLatencyMeasure: &testInst{}, + } +} diff --git a/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go b/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go index 49b70a04535..82bb0133168 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go @@ -18,7 +18,7 @@ import ( func TestNewTraceRequest(t *testing.T) { t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http/dup") - serv := NewHTTPServer() + serv := NewHTTPServer(nil) want := func(req testServerReq) []attribute.KeyValue { return []attribute.KeyValue{ attribute.String("http.request.method", "GET"), diff --git a/instrumentation/net/http/otelhttp/internal/semconv/util.go b/instrumentation/net/http/otelhttp/internal/semconv/util.go index 6cede5a5f99..af862d1a7f0 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/util.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/util.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" semconvNew "go.opentelemetry.io/otel/semconv/v1.26.0" ) @@ -89,3 +90,9 @@ var methodLookup = map[string]attribute.KeyValue{ http.MethodPut: semconvNew.HTTPRequestMethodPut, http.MethodTrace: semconvNew.HTTPRequestMethodTrace, } + +func handleErr(err error) { + if err != nil { + otel.Handle(err) + } +} diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go index c2bc041ac50..c999b05e675 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0.go @@ -7,9 +7,13 @@ import ( "errors" "io" "net/http" + "slices" + "strings" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconvutil" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/noop" semconv "go.opentelemetry.io/otel/semconv/v1.20.0" ) @@ -73,6 +77,110 @@ func HTTPStatusCode(status int) attribute.KeyValue { return semconv.HTTPStatusCode(status) } +// Server HTTP metrics. +const ( + serverRequestSize = "http.server.request.size" // Incoming request bytes total + serverResponseSize = "http.server.response.size" // Incoming response bytes total + serverDuration = "http.server.duration" // Incoming end to end duration, milliseconds +) + +func (h oldHTTPServer) createMeasures(meter metric.Meter) (metric.Int64Counter, metric.Int64Counter, metric.Float64Histogram) { + if meter == nil { + return noop.Int64Counter{}, noop.Int64Counter{}, noop.Float64Histogram{} + } + var err error + requestBytesCounter, err := meter.Int64Counter( + serverRequestSize, + metric.WithUnit("By"), + metric.WithDescription("Measures the size of HTTP request messages."), + ) + handleErr(err) + + responseBytesCounter, err := meter.Int64Counter( + serverResponseSize, + metric.WithUnit("By"), + metric.WithDescription("Measures the size of HTTP response messages."), + ) + handleErr(err) + + serverLatencyMeasure, err := meter.Float64Histogram( + serverDuration, + metric.WithUnit("ms"), + metric.WithDescription("Measures the duration of inbound HTTP requests."), + ) + handleErr(err) + + return requestBytesCounter, responseBytesCounter, serverLatencyMeasure +} + +func (o oldHTTPServer) MetricAttributes(server string, req *http.Request, statusCode int, additionalAttributes []attribute.KeyValue) []attribute.KeyValue { + n := len(additionalAttributes) + 3 + var host string + var p int + if server == "" { + host, p = splitHostPort(req.Host) + } else { + // Prioritize the primary server name. + host, p = splitHostPort(server) + if p < 0 { + _, p = splitHostPort(req.Host) + } + } + hostPort := requiredHTTPPort(req.TLS != nil, p) + if hostPort > 0 { + n++ + } + protoName, protoVersion := netProtocol(req.Proto) + if protoName != "" { + n++ + } + if protoVersion != "" { + n++ + } + + if statusCode > 0 { + n++ + } + + attributes := slices.Grow(additionalAttributes, n) + attributes = append(attributes, + o.methodMetric(req.Method), + o.scheme(req.TLS != nil), + semconv.NetHostName(host)) + + if hostPort > 0 { + attributes = append(attributes, semconv.NetHostPort(hostPort)) + } + if protoName != "" { + attributes = append(attributes, semconv.NetProtocolName(protoName)) + } + if protoVersion != "" { + attributes = append(attributes, semconv.NetProtocolVersion(protoVersion)) + } + + if statusCode > 0 { + attributes = append(attributes, semconv.HTTPStatusCode(statusCode)) + } + return attributes +} + +func (o oldHTTPServer) methodMetric(method string) attribute.KeyValue { + method = strings.ToUpper(method) + switch method { + case http.MethodConnect, http.MethodDelete, http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodPatch, http.MethodPost, http.MethodPut, http.MethodTrace: + default: + method = "_OTHER" + } + return semconv.HTTPMethod(method) +} + +func (o oldHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:revive + if https { + return semconv.HTTPSchemeHTTPS + } + return semconv.HTTPSchemeHTTP +} + type oldHTTPClient struct{} func (o oldHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0_test.go index fcf116f94f5..7fc0e90eb45 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0_test.go @@ -4,6 +4,7 @@ package semconv import ( + "context" "fmt" "net/http" "strings" @@ -17,7 +18,7 @@ import ( func TestV120TraceRequest(t *testing.T) { // Anything but "http" or "http/dup" works. t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "old") - serv := NewHTTPServer() + serv := NewHTTPServer(nil) want := func(req testServerReq) []attribute.KeyValue { return []attribute.KeyValue{ attribute.String("http.method", "GET"), @@ -86,6 +87,43 @@ func TestV120TraceResponse(t *testing.T) { } } +func TestV120RecordMetrics(t *testing.T) { + server := NewTestHTTPServer() + req, err := http.NewRequest("POST", "http://example.com", nil) + assert.NoError(t, err) + + server.RecordMetrics(context.Background(), MetricData{ + ServerName: "stuff", + Req: req, + StatusCode: 301, + AdditionalAttributes: []attribute.KeyValue{ + attribute.String("key", "value"), + }, + + RequestSize: 100, + ResponseSize: 200, + ElapsedTime: 300, + }) + + assert.Equal(t, int64(100), server.requestBytesCounter.(*testInst).intValue) + assert.Equal(t, int64(200), server.responseBytesCounter.(*testInst).intValue) + assert.Equal(t, float64(300), server.serverLatencyMeasure.(*testInst).floatValue) + + want := []attribute.KeyValue{ + attribute.String("http.scheme", "http"), + attribute.String("http.method", "POST"), + attribute.Int64("http.status_code", 301), + attribute.String("key", "value"), + attribute.String("net.host.name", "stuff"), + attribute.String("net.protocol.name", "http"), + attribute.String("net.protocol.version", "1.1"), + } + + assert.ElementsMatch(t, want, server.requestBytesCounter.(*testInst).attributes) + assert.ElementsMatch(t, want, server.responseBytesCounter.(*testInst).attributes) + assert.ElementsMatch(t, want, server.serverLatencyMeasure.(*testInst).attributes) +} + func TestV120ClientRequest(t *testing.T) { body := strings.NewReader("Hello, world!") url := "https://example.com:8888/foo/bar?stuff=morestuff"