From 6b4d80c3b3ed67858a8c9ae022d3c612d5b4b177 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Tue, 18 Jun 2024 09:03:03 -0500 Subject: [PATCH 1/4] Enable metrics in semconv --- instrumentation/net/http/otelhttp/handler.go | 62 +++------- .../otelhttp/internal/semconv/bench_test.go | 2 +- .../net/http/otelhttp/internal/semconv/env.go | 53 +++++++-- .../otelhttp/internal/semconv/env_test.go | 83 ++++++++++++++ .../http/otelhttp/internal/semconv/util.go | 7 ++ .../http/otelhttp/internal/semconv/v1.20.0.go | 108 ++++++++++++++++++ .../otelhttp/internal/semconv/v1.20.0_test.go | 41 ++++++- .../otelhttp/internal/semconv/v1.24.0_test.go | 2 +- 8 files changed, 301 insertions(+), 57 deletions(-) create mode 100644 instrumentation/net/http/otelhttp/internal/semconv/env_test.go diff --git a/instrumentation/net/http/otelhttp/handler.go b/instrumentation/net/http/otelhttp/handler.go index d01bdccf40d..157145e77af 100644 --- a/instrumentation/net/http/otelhttp/handler.go +++ b/instrumentation/net/http/otelhttp/handler.go @@ -10,9 +10,7 @@ import ( "github.com/felixge/httpsnoop" "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" @@ -24,7 +22,6 @@ type middleware struct { server string tracer trace.Tracer - meter metric.Meter propagators propagation.TextMapPropagator spanStartOptions []trace.SpanStartOption readEvent bool @@ -34,7 +31,7 @@ type middleware struct { publicEndpoint bool publicEndpointFn func(*http.Request) bool - traceSemconv semconv.HTTPServer + semconv semconv.HTTPServer requestBytesCounter metric.Int64Counter responseBytesCounter metric.Int64Counter serverLatencyMeasure metric.Float64Histogram @@ -56,8 +53,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{ @@ -67,7 +62,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) { @@ -78,7 +72,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 @@ -88,6 +81,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) { @@ -96,30 +90,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) { @@ -134,7 +104,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...) @@ -217,8 +187,8 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http next.ServeHTTP(w, r.WithContext(ctx)) - span.SetStatus(semconv.ServerStatus(rww.statusCode)) - span.SetAttributes(h.traceSemconv.ResponseTraceAttrs(semconv.ResponseTelemetry{ + span.SetStatus(h.semconv.Status(rww.statusCode)) + span.SetAttributes(h.semconv.ResponseTraceAttrs(semconv.ResponseTelemetry{ StatusCode: rww.statusCode, ReadBytes: bw.read.Load(), ReadError: bw.err, @@ -226,26 +196,24 @@ func (h *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http WriteError: rww.err, })...) - // Add metrics - attributes := append(labeler.Get(), semconvutil.HTTPServerRequestMetrics(h.server, r)...) - if rww.statusCode > 0 { - attributes = append(attributes, semconv.HTTPStatusCode(rww.statusCode)) - } - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - addOpts := []metric.AddOption{o} // Allocate vararg slice once. - h.requestBytesCounter.Add(ctx, bw.read.Load(), addOpts...) - h.responseBytesCounter.Add(ctx, rww.written, addOpts...) - // 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: rww.statusCode, + AdditionalAttributes: labeler.Get(), + RequestSize: bw.read.Load(), + ResponseSize: rww.written, + 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 3ec0ad00c81..3318b116b91 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) } @@ -80,3 +82,40 @@ 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 { + // 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 +} 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/util.go b/instrumentation/net/http/otelhttp/internal/semconv/util.go index e7f293761bd..aaca7603eb2 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.24.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 c3e838aaa54..42473535265 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" ) @@ -72,3 +76,107 @@ func (o oldHTTPServer) Route(route string) attribute.KeyValue { 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 +} 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 3a84697ea4e..2b5553104e0 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,7 +4,9 @@ package semconv import ( + "context" "fmt" + "net/http" "testing" "github.com/stretchr/testify/assert" @@ -15,7 +17,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"), @@ -83,3 +85,40 @@ 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) +} diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go index 8b0f781597e..4252446a62c 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0_test.go @@ -15,7 +15,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"), From de9ff50a41f1dd008176c2ef3646aec70c7853c9 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Mon, 8 Jul 2024 10:01:11 -0400 Subject: [PATCH 2/4] Fixed lint issues --- instrumentation/net/http/otelhttp/common.go | 7 ------- instrumentation/net/http/otelhttp/handler.go | 6 +----- 2 files changed, 1 insertion(+), 12 deletions(-) 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 157145e77af..50d1d34ebf9 100644 --- a/instrumentation/net/http/otelhttp/handler.go +++ b/instrumentation/net/http/otelhttp/handler.go @@ -11,7 +11,6 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv" "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" ) @@ -31,10 +30,7 @@ type middleware struct { publicEndpoint bool publicEndpointFn func(*http.Request) bool - semconv semconv.HTTPServer - requestBytesCounter metric.Int64Counter - responseBytesCounter metric.Int64Counter - serverLatencyMeasure metric.Float64Histogram + semconv semconv.HTTPServer } func defaultHandlerFormatter(operation string, _ *http.Request) string { From dcdc4c2e2d880e849737665937b8eb30432a9cff Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Thu, 1 Aug 2024 19:13:57 +0200 Subject: [PATCH 3/4] Update instrumentation/net/http/otelhttp/internal/semconv/env.go Co-authored-by: Tyler Yahn --- instrumentation/net/http/otelhttp/internal/semconv/env.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env.go b/instrumentation/net/http/otelhttp/internal/semconv/env.go index 3318b116b91..aedf7dfbfa5 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env.go @@ -95,7 +95,7 @@ type MetricData struct { } func (s HTTPServer) RecordMetrics(ctx context.Context, md MetricData) { - if s.requestBytesCounter == nil { + if s.requestBytesCounter == nil || s.responseBytesCounter == nil || s.serverLatencyMeasure == nil { // This will happen if an HTTPServer{} is used insted of NewHTTPServer. return } From 3f304d0bfd353d49c83f76f83e114dfac5275a61 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Thu, 1 Aug 2024 19:29:17 +0200 Subject: [PATCH 4/4] fix missing nil --- .../net/http/otelhttp/internal/semconv/httpconv_test.go | 2 +- .../net/http/otelhttp/internal/semconv/v1.20.0_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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/v1.20.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.20.0_test.go index c6ae833f40d..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 @@ -123,6 +123,7 @@ func TestV120RecordMetrics(t *testing.T) { 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"