From 70fbdaba39d98ff78281ee00a1f9bd2406ba478a Mon Sep 17 00:00:00 2001 From: keisku Date: Fri, 23 May 2025 11:11:25 +0900 Subject: [PATCH 1/3] otelhttp: Add server-side test for OTEL_SEMCONV_STABILITY_OPT_IN=http/dup with new/old metrics --- .../net/http/otelhttp/handler_test.go | 145 ++++++++++++++---- 1 file changed, 118 insertions(+), 27 deletions(-) diff --git a/instrumentation/net/http/otelhttp/handler_test.go b/instrumentation/net/http/otelhttp/handler_test.go index fdf2aee877c..bbf538f7341 100644 --- a/instrumentation/net/http/otelhttp/handler_test.go +++ b/instrumentation/net/http/otelhttp/handler_test.go @@ -796,63 +796,154 @@ func TestHandlerWithMetricAttributesFn(t *testing.T) { } func TestHandlerWithSemConvStabilityOptIn(t *testing.T) { + newSpanAttrs := []attribute.KeyValue{ + attribute.String("http.request.method", "GET"), + attribute.String("url.scheme", "http"), + attribute.String("server.address", "localhost"), + attribute.String("network.protocol.version", "1.1"), + attribute.String("url.path", "/"), + attribute.Int("http.response.status_code", 200), + } + newMetricAttrs := attribute.NewSet( + attribute.String("http.request.method", "GET"), + attribute.Int("http.response.status_code", 200), + attribute.String("network.protocol.name", "http"), + attribute.String("network.protocol.version", "1.1"), + attribute.String("server.address", "localhost"), + attribute.String("url.scheme", "http"), + ) + newMetrics := []metricdata.Metrics{ + { + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", + Unit: "By", + Data: metricdata.Histogram[int64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[int64]{{Attributes: newMetricAttrs}}, + }, + }, + { + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", + Unit: "By", + Data: metricdata.Histogram[int64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[int64]{{Attributes: newMetricAttrs}}, + }, + }, + { + Name: "http.server.request.duration", + Description: "Duration of HTTP server requests.", + Unit: "s", + Data: metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: newMetricAttrs}}, + }, + }, + } + oldSpanAttrs := []attribute.KeyValue{ + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "http"), + attribute.String("net.host.name", "localhost"), + attribute.String("net.protocol.version", "1.1"), + attribute.String("http.target", "/"), + attribute.Int("http.status_code", 200), + } + oldMetricAttrs := attribute.NewSet( + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "http"), + attribute.Int("http.status_code", 200), + attribute.String("net.host.name", "localhost"), + attribute.String("net.protocol.name", "http"), + attribute.String("net.protocol.version", "1.1"), + ) + oldMetrics := []metricdata.Metrics{ + { + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", + Unit: "By", + Data: metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{Attributes: oldMetricAttrs}}, + }, + }, + { + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", + Unit: "By", + Data: metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{Attributes: oldMetricAttrs}}, + }, + }, + { + Name: "http.server.duration", + Description: "Measures the duration of inbound HTTP requests.", + Unit: "ms", + Data: metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: oldMetricAttrs}}, + }, + }, + } tests := []struct { name string semConvStabilityOptInValue string - expected []attribute.KeyValue + expectedSpanAttributes []attribute.KeyValue + expectedMetrics metricdata.ScopeMetrics }{ { name: "without opt-in", semConvStabilityOptInValue: "", - expected: []attribute.KeyValue{ - attribute.String("http.request.method", "GET"), - attribute.String("url.scheme", "http"), - attribute.String("server.address", "localhost"), - attribute.String("network.protocol.version", "1.1"), - attribute.String("url.path", "/"), - attribute.Int("http.response.status_code", 200), + expectedSpanAttributes: newSpanAttrs, + expectedMetrics: metricdata.ScopeMetrics{ + Scope: instrumentation.Scope{ + Name: ScopeName, + Version: Version(), + }, + Metrics: newMetrics, }, }, { name: "with http/dup opt-in", semConvStabilityOptInValue: "http/dup", - expected: []attribute.KeyValue{ - // New semantic conventions - attribute.String("http.request.method", "GET"), - attribute.String("url.scheme", "http"), - attribute.String("server.address", "localhost"), - attribute.String("network.protocol.version", "1.1"), - attribute.String("url.path", "/"), - attribute.Int("http.response.status_code", 200), - // Old semantic conventions - attribute.String("http.method", "GET"), - attribute.String("http.scheme", "http"), - attribute.String("net.host.name", "localhost"), - attribute.String("net.protocol.version", "1.1"), - attribute.String("http.target", "/"), - attribute.Int("http.status_code", 200), + expectedSpanAttributes: append(newSpanAttrs, oldSpanAttrs...), + expectedMetrics: metricdata.ScopeMetrics{ + Scope: instrumentation.Scope{ + Name: ScopeName, + Version: Version(), + }, + Metrics: append(newMetrics, oldMetrics...), }, }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Setenv("OTEL_SEMCONV_STABILITY_OPT_IN", tt.semConvStabilityOptInValue) + metricRecorder := sdkmetric.NewManualReader() spanRecorder := tracetest.NewSpanRecorder() - provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(spanRecorder)) + meterProvider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(metricRecorder)) + traceProvider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(spanRecorder)) h := NewHandler( http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }), "test_handler", - WithTracerProvider(provider), + WithTracerProvider(traceProvider), + WithMeterProvider(meterProvider), ) r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil) require.NoError(t, err) h.ServeHTTP(httptest.NewRecorder(), r) spans := spanRecorder.Ended() require.Len(t, spans, 1) - assert.ElementsMatch(t, spans[0].Attributes(), tt.expected) + assert.ElementsMatch(t, spans[0].Attributes(), tt.expectedSpanAttributes) + rm := metricdata.ResourceMetrics{} + metricRecorder.Collect(context.Background(), &rm) + require.Len(t, rm.ScopeMetrics, 1) + metricdatatest.AssertEqual(t, tt.expectedMetrics, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) }) } } From 1be82c4721d5fbbc9df95cda0d1007b3a3c25fac Mon Sep 17 00:00:00 2001 From: keisku Date: Fri, 23 May 2025 13:44:07 +0900 Subject: [PATCH 2/3] fix errcheck --- instrumentation/net/http/otelhttp/handler_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/net/http/otelhttp/handler_test.go b/instrumentation/net/http/otelhttp/handler_test.go index bbf538f7341..d46cecab89d 100644 --- a/instrumentation/net/http/otelhttp/handler_test.go +++ b/instrumentation/net/http/otelhttp/handler_test.go @@ -941,7 +941,8 @@ func TestHandlerWithSemConvStabilityOptIn(t *testing.T) { require.Len(t, spans, 1) assert.ElementsMatch(t, spans[0].Attributes(), tt.expectedSpanAttributes) rm := metricdata.ResourceMetrics{} - metricRecorder.Collect(context.Background(), &rm) + err = metricRecorder.Collect(context.Background(), &rm) + require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) metricdatatest.AssertEqual(t, tt.expectedMetrics, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) }) From c02b278d68e6888e9f5581163762943c6b96f6a2 Mon Sep 17 00:00:00 2001 From: keisku Date: Sat, 31 May 2025 08:16:39 +0900 Subject: [PATCH 3/3] replace expect with want --- .../net/http/otelhttp/handler_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/instrumentation/net/http/otelhttp/handler_test.go b/instrumentation/net/http/otelhttp/handler_test.go index d46cecab89d..33f0016e004 100644 --- a/instrumentation/net/http/otelhttp/handler_test.go +++ b/instrumentation/net/http/otelhttp/handler_test.go @@ -891,14 +891,14 @@ func TestHandlerWithSemConvStabilityOptIn(t *testing.T) { tests := []struct { name string semConvStabilityOptInValue string - expectedSpanAttributes []attribute.KeyValue - expectedMetrics metricdata.ScopeMetrics + wantSpanAttributes []attribute.KeyValue + wantMetrics metricdata.ScopeMetrics }{ { name: "without opt-in", semConvStabilityOptInValue: "", - expectedSpanAttributes: newSpanAttrs, - expectedMetrics: metricdata.ScopeMetrics{ + wantSpanAttributes: newSpanAttrs, + wantMetrics: metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: ScopeName, Version: Version(), @@ -909,8 +909,8 @@ func TestHandlerWithSemConvStabilityOptIn(t *testing.T) { { name: "with http/dup opt-in", semConvStabilityOptInValue: "http/dup", - expectedSpanAttributes: append(newSpanAttrs, oldSpanAttrs...), - expectedMetrics: metricdata.ScopeMetrics{ + wantSpanAttributes: append(newSpanAttrs, oldSpanAttrs...), + wantMetrics: metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: ScopeName, Version: Version(), @@ -939,12 +939,12 @@ func TestHandlerWithSemConvStabilityOptIn(t *testing.T) { h.ServeHTTP(httptest.NewRecorder(), r) spans := spanRecorder.Ended() require.Len(t, spans, 1) - assert.ElementsMatch(t, spans[0].Attributes(), tt.expectedSpanAttributes) + assert.ElementsMatch(t, spans[0].Attributes(), tt.wantSpanAttributes) rm := metricdata.ResourceMetrics{} err = metricRecorder.Collect(context.Background(), &rm) require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) - metricdatatest.AssertEqual(t, tt.expectedMetrics, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) + metricdatatest.AssertEqual(t, tt.wantMetrics, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) }) } }