diff --git a/CHANGELOG.md b/CHANGELOG.md index d43a76baf67..332f4965ac4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed - Jaeger remote sampler's probabilistic strategy now uses the same sampling algorithm as `trace.TraceIDRatioBased` in `go.opentelemetry.io/contrib/samplers/jaegerremote`. (#6892) +- Switched the default for `OTEL_SEMCONV_STABILITY_OPT_IN` to emit the v1.26.0 semantic conventions by default in the following modules. + - `go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful` + - `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin` + - `go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux` + - `go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho` + - `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace` + - `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` + + The `OTEL_SEMCONV_STABILITY_OPT_IN=http/dup` environment variable can be still used to emit both the v1.20.0 and v1.26.0 semantic conventions. + It is however impossible to emit only the 1.20.0 semantic conventions, as the next release will drop support for that environment variable. (#6899) - Update the Jaeger remote sampler to use "github.com/jaegertracing/jaeger-idl/proto-gen/api_v2" in `go.opentelemetry.io/contrib/samplers/jaegerremote`. (#7061) ### Fixed diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env.go index 66050f23d7f..6cc309edee8 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env.go @@ -20,7 +20,7 @@ import ( ) // OTelSemConvStabilityOptIn is an environment variable. -// That can be set to "old" or "http/dup" to opt into the new HTTP semantic conventions. +// That can be set to "http/dup" to keep getting the old HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -65,7 +65,7 @@ func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts Req if s.duplicate { return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts)...) } - return OldHTTPServer{}.RequestTraceAttrs(server, req) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) } func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { @@ -73,7 +73,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { return append([]attribute.KeyValue{OldHTTPServer{}.NetworkTransportAttr(network)}, CurrentHTTPServer{}.NetworkTransportAttr(network)) } return []attribute.KeyValue{ - OldHTTPServer{}.NetworkTransportAttr(network), + CurrentHTTPServer{}.NetworkTransportAttr(network), } } @@ -84,12 +84,12 @@ func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyVa if s.duplicate { return append(OldHTTPServer{}.ResponseTraceAttrs(resp), CurrentHTTPServer{}.ResponseTraceAttrs(resp)...) } - return OldHTTPServer{}.ResponseTraceAttrs(resp) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp) } // Route returns the attribute for the route. func (s HTTPServer) Route(route string) attribute.KeyValue { - return OldHTTPServer{}.Route(route) + return CurrentHTTPServer{}.Route(route) } // Status returns a span status code and message for an HTTP status code @@ -141,19 +141,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - if s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { - attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) - *addOpts = append(*addOpts, o) - s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) - s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) - s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) - *addOpts = (*addOpts)[:0] - metricAddOptionPool.Put(addOpts) - } - - if s.duplicate && s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { + if s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { attributes := CurrentHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) o := metric.WithAttributeSet(attribute.NewSet(attributes...)) recordOpts := metricRecordOptionPool.Get().(*[]metric.RecordOption) @@ -164,6 +152,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *recordOpts = (*recordOpts)[:0] metricRecordOptionPool.Put(recordOpts) } + + if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) + o := metric.WithAttributeSet(attribute.NewSet(attributes...)) + addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) + *addOpts = append(*addOpts, o) + s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) + s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) + s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) + *addOpts = (*addOpts)[:0] + metricAddOptionPool.Put(addOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -172,9 +172,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) } return server } @@ -198,9 +198,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) } return client @@ -211,7 +211,7 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { if c.duplicate { return append(OldHTTPClient{}.RequestTraceAttrs(req), CurrentHTTPClient{}.RequestTraceAttrs(req)...) } - return OldHTTPClient{}.RequestTraceAttrs(req) + return CurrentHTTPClient{}.RequestTraceAttrs(req) } // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. @@ -220,7 +220,7 @@ func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue return append(OldHTTPClient{}.ResponseTraceAttrs(resp), CurrentHTTPClient{}.ResponseTraceAttrs(resp)...) } - return OldHTTPClient{}.ResponseTraceAttrs(resp) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp) } func (c HTTPClient) Status(code int) (codes.Code, string) { @@ -234,11 +234,7 @@ func (c HTTPClient) Status(code int) (codes.Code, string) { } func (c HTTPClient) ErrorType(err error) attribute.KeyValue { - if c.duplicate { - return CurrentHTTPClient{}.ErrorType(err) - } - - return attribute.KeyValue{} + return CurrentHTTPClient{}.ErrorType(err) } type MetricOpts struct { @@ -257,17 +253,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["old"] = MetricOpts{ + opts["new"] = MetricOpts{ measurement: set, addOptions: set, } if c.duplicate { - attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["new"] = MetricOpts{ + opts["old"] = MetricOpts{ measurement: set, addOptions: set, } @@ -277,17 +273,17 @@ func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { } func (s HTTPClient) RecordMetrics(ctx context.Context, md MetricData, opts map[string]MetricOpts) { - if s.requestBytesCounter == nil || s.latencyMeasure == nil { + if s.requestBodySize == nil || s.requestDuration == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) - s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) + s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) + s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) if s.duplicate { - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000.0, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) } } @@ -305,5 +301,5 @@ func (s HTTPClient) TraceAttributes(host string) []attribute.KeyValue { return append(OldHTTPClient{}.TraceAttributes(host), CurrentHTTPClient{}.TraceAttributes(host)...) } - return OldHTTPClient{}.TraceAttributes(host) + return CurrentHTTPClient{}.TraceAttributes(host) } diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env_test.go index 635f171805a..39b243ba915 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env_test.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/env_test.go @@ -68,16 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - network: "tcp", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), + attribute.String("network.transport", "tcp"), }, }, { @@ -152,15 +143,7 @@ func TestHTTPClientTraceAttributes(t *testing.T) { name: "with no optin set", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "with optin set to old only", - optinVal: "old", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { @@ -196,16 +179,7 @@ func TestClientTraceAttributes(t *testing.T) { host: "example.com", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - host: "example.com", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/gen.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/gen.go index 9d5ba6649e1..3eb5616358d 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/gen.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/gen.go @@ -6,4 +6,3 @@ package test // import "go.opentelemetry.io/contrib/instrumentation/github.com/e // Generate semconv/test package: //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/common_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful\" }" --out=common_test.go //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/httpconv_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful\" }" --out=httpconv_test.go -//go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/v1.20.0_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful\" }" --out=v1.20.0_test.go diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/httpconv_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/httpconv_test.go index 61d99125359..316f4a43133 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/httpconv_test.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/httpconv_test.go @@ -78,49 +78,47 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPServer version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", + 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: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -128,44 +126,46 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPServer version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPServer version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.server.request.body.size", - Description: "Size of HTTP server request bodies.", + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.response.body.size", - Description: "Size of HTTP server response bodies.", + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.request.duration", - Description: "Duration of HTTP server requests.", - Unit: "s", + 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: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -221,7 +221,7 @@ func TestNewServerRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 6) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -416,35 +416,34 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPClient version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.client.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.client.request.body.size", + Description: "Size of HTTP client request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", + Name: "http.client.request.duration", + Description: "Duration of HTTP client requests.", + Unit: "s", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -452,31 +451,32 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPClient version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.client.request.body.size", - Description: "Size of HTTP client request bodies.", + Name: "http.client.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.client.request.duration", - Description: "Duration of HTTP client requests.", - Unit: "s", + Name: "http.client.duration", + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -508,9 +508,8 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) - // because of OldHTTPClient require.Len(t, rm.ScopeMetrics[0].Metrics, 2) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -532,7 +531,7 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 4) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -595,9 +594,6 @@ func TestClientRecordResponseSize(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - tests := []struct { name string setEnv bool @@ -621,11 +617,7 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Len(t, rm.ScopeMetrics, 1) - - // because of OldHTTPClient - require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + require.Empty(t, rm.ScopeMetrics) }, }, { @@ -647,7 +639,7 @@ func TestClientRecordResponseSize(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/v1.20.0_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/v1.20.0_test.go deleted file mode 100644 index 10156a001fd..00000000000 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/test/v1.20.0_test.go +++ /dev/null @@ -1,311 +0,0 @@ -// Code created by gotmpl. DO NOT MODIFY. -// source: internal/shared/semconv/test/v1.20.0_test.go.tmpl - -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package test - -import ( - "context" - "fmt" - "net/http" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/instrumentation" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" -) - -func TestV120TraceRequest(t *testing.T) { - // Anything but "http" or "http/dup" works. - t.Setenv("OTEL_SEMCONV_STABILITY_OPT_IN", "old") - serv := semconv.NewHTTPServer(nil) - want := func(req testServerReq) []attribute.KeyValue { - return []attribute.KeyValue{ - attribute.String("http.method", "GET"), - attribute.String("http.scheme", "http"), - attribute.String("net.host.name", req.hostname), - attribute.Int("net.host.port", req.serverPort), - attribute.String("net.sock.peer.addr", req.peerAddr), - attribute.Int("net.sock.peer.port", req.peerPort), - attribute.String("user_agent.original", "Go-http-client/1.1"), - attribute.String("http.client_ip", req.clientIP), - attribute.String("net.protocol.version", "1.1"), - attribute.String("http.target", "/"), - } - } - testTraceRequest(t, serv, want) -} - -func TestV120TraceResponse(t *testing.T) { - testCases := []struct { - name string - resp semconv.ResponseTelemetry - want []attribute.KeyValue - }{ - { - name: "empty", - resp: semconv.ResponseTelemetry{}, - want: nil, - }, - { - name: "no errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - WriteBytes: 802, - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.Int("http.response_content_length", 802), - attribute.Int("http.status_code", 200), - }, - }, - { - name: "with errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - ReadError: fmt.Errorf("read error"), - WriteBytes: 802, - WriteError: fmt.Errorf("write error"), - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.String("http.read_error", "read error"), - attribute.Int("http.response_content_length", 802), - attribute.String("http.write_error", "write error"), - attribute.Int("http.status_code", 200), - }, - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - got := semconv.OldHTTPServer{}.ResponseTraceAttrs(tt.resp) - assert.ElementsMatch(t, tt.want, got) - }) - } -} - -func TestV120RecordMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - server := semconv.NewHTTPServer(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - server.RecordMetrics(context.Background(), semconv.ServerMetricData{ - ServerName: "stuff", - ResponseSize: 200, - MetricAttributes: semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }, - MetricData: semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, - }) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - 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"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - -func TestV120ClientRequest(t *testing.T) { - body := strings.NewReader("Hello, world!") - url := "https://example.com:8888/foo/bar?stuff=morestuff" - req, err := http.NewRequest("POST", url, body) - assert.NoError(t, err) - req.Header.Set("User-Agent", "go-test-agent") - - want := []attribute.KeyValue{ - attribute.String("http.method", "POST"), - attribute.String("http.url", url), - attribute.String("net.peer.name", "example.com"), - attribute.Int("net.peer.port", 8888), - attribute.Int("http.request_content_length", body.Len()), - attribute.String("user_agent.original", "go-test-agent"), - } - got := semconv.OldHTTPClient{}.RequestTraceAttrs(req) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientResponse(t *testing.T) { - resp := http.Response{ - StatusCode: 200, - ContentLength: 123, - } - - want := []attribute.KeyValue{ - attribute.Int("http.response_content_length", 123), - attribute.Int("http.status_code", 200), - } - - got := semconv.OldHTTPClient{}.ResponseTraceAttrs(&resp) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - client := semconv.NewHTTPClient(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - opts := client.MetricOptions(semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }) - - ctx := context.Background() - - client.RecordResponseSize(ctx, 200, opts) - - client.RecordMetrics(ctx, semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, opts) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - attribute.String("http.method", "POST"), - attribute.Int64("http.status_code", 301), - attribute.String("key", "value"), - attribute.String("net.peer.name", "example.com"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []metricdata.Metrics{ - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env.go index dc43813db29..f69cf817449 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env.go @@ -20,7 +20,7 @@ import ( ) // OTelSemConvStabilityOptIn is an environment variable. -// That can be set to "old" or "http/dup" to opt into the new HTTP semantic conventions. +// That can be set to "http/dup" to keep getting the old HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -65,7 +65,7 @@ func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts Req if s.duplicate { return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts)...) } - return OldHTTPServer{}.RequestTraceAttrs(server, req) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) } func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { @@ -73,7 +73,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { return append([]attribute.KeyValue{OldHTTPServer{}.NetworkTransportAttr(network)}, CurrentHTTPServer{}.NetworkTransportAttr(network)) } return []attribute.KeyValue{ - OldHTTPServer{}.NetworkTransportAttr(network), + CurrentHTTPServer{}.NetworkTransportAttr(network), } } @@ -84,12 +84,12 @@ func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyVa if s.duplicate { return append(OldHTTPServer{}.ResponseTraceAttrs(resp), CurrentHTTPServer{}.ResponseTraceAttrs(resp)...) } - return OldHTTPServer{}.ResponseTraceAttrs(resp) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp) } // Route returns the attribute for the route. func (s HTTPServer) Route(route string) attribute.KeyValue { - return OldHTTPServer{}.Route(route) + return CurrentHTTPServer{}.Route(route) } // Status returns a span status code and message for an HTTP status code @@ -141,19 +141,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - if s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { - attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) - *addOpts = append(*addOpts, o) - s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) - s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) - s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) - *addOpts = (*addOpts)[:0] - metricAddOptionPool.Put(addOpts) - } - - if s.duplicate && s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { + if s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { attributes := CurrentHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) o := metric.WithAttributeSet(attribute.NewSet(attributes...)) recordOpts := metricRecordOptionPool.Get().(*[]metric.RecordOption) @@ -164,6 +152,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *recordOpts = (*recordOpts)[:0] metricRecordOptionPool.Put(recordOpts) } + + if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) + o := metric.WithAttributeSet(attribute.NewSet(attributes...)) + addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) + *addOpts = append(*addOpts, o) + s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) + s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) + s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) + *addOpts = (*addOpts)[:0] + metricAddOptionPool.Put(addOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -172,9 +172,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) } return server } @@ -198,9 +198,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) } return client @@ -211,7 +211,7 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { if c.duplicate { return append(OldHTTPClient{}.RequestTraceAttrs(req), CurrentHTTPClient{}.RequestTraceAttrs(req)...) } - return OldHTTPClient{}.RequestTraceAttrs(req) + return CurrentHTTPClient{}.RequestTraceAttrs(req) } // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. @@ -220,7 +220,7 @@ func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue return append(OldHTTPClient{}.ResponseTraceAttrs(resp), CurrentHTTPClient{}.ResponseTraceAttrs(resp)...) } - return OldHTTPClient{}.ResponseTraceAttrs(resp) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp) } func (c HTTPClient) Status(code int) (codes.Code, string) { @@ -234,11 +234,7 @@ func (c HTTPClient) Status(code int) (codes.Code, string) { } func (c HTTPClient) ErrorType(err error) attribute.KeyValue { - if c.duplicate { - return CurrentHTTPClient{}.ErrorType(err) - } - - return attribute.KeyValue{} + return CurrentHTTPClient{}.ErrorType(err) } type MetricOpts struct { @@ -257,17 +253,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["old"] = MetricOpts{ + opts["new"] = MetricOpts{ measurement: set, addOptions: set, } if c.duplicate { - attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["new"] = MetricOpts{ + opts["old"] = MetricOpts{ measurement: set, addOptions: set, } @@ -277,17 +273,17 @@ func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { } func (s HTTPClient) RecordMetrics(ctx context.Context, md MetricData, opts map[string]MetricOpts) { - if s.requestBytesCounter == nil || s.latencyMeasure == nil { + if s.requestBodySize == nil || s.requestDuration == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) - s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) + s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) + s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) if s.duplicate { - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000.0, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) } } @@ -305,5 +301,5 @@ func (s HTTPClient) TraceAttributes(host string) []attribute.KeyValue { return append(OldHTTPClient{}.TraceAttributes(host), CurrentHTTPClient{}.TraceAttributes(host)...) } - return OldHTTPClient{}.TraceAttributes(host) + return CurrentHTTPClient{}.TraceAttributes(host) } diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env_test.go index 635f171805a..39b243ba915 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/env_test.go @@ -68,16 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - network: "tcp", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), + attribute.String("network.transport", "tcp"), }, }, { @@ -152,15 +143,7 @@ func TestHTTPClientTraceAttributes(t *testing.T) { name: "with no optin set", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "with optin set to old only", - optinVal: "old", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { @@ -196,16 +179,7 @@ func TestClientTraceAttributes(t *testing.T) { host: "example.com", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - host: "example.com", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/gen.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/gen.go index 1fae83d3148..e920f52e4cf 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/gen.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/gen.go @@ -6,4 +6,3 @@ package test // import "go.opentelemetry.io/contrib/instrumentation/github.com/g // Generate semconv/test package: //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/common_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin\" }" --out=common_test.go //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/httpconv_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin\" }" --out=httpconv_test.go -//go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/v1.20.0_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin\" }" --out=v1.20.0_test.go diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/httpconv_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/httpconv_test.go index 899ce6fc7c1..b52a2a2dd4c 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/httpconv_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/httpconv_test.go @@ -78,49 +78,47 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPServer version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", + 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: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -128,44 +126,46 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPServer version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPServer version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.server.request.body.size", - Description: "Size of HTTP server request bodies.", + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.response.body.size", - Description: "Size of HTTP server response bodies.", + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.request.duration", - Description: "Duration of HTTP server requests.", - Unit: "s", + 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: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -221,7 +221,7 @@ func TestNewServerRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 6) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -416,35 +416,34 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPClient version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.client.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.client.request.body.size", + Description: "Size of HTTP client request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", + Name: "http.client.request.duration", + Description: "Duration of HTTP client requests.", + Unit: "s", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -452,31 +451,32 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPClient version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.client.request.body.size", - Description: "Size of HTTP client request bodies.", + Name: "http.client.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.client.request.duration", - Description: "Duration of HTTP client requests.", - Unit: "s", + Name: "http.client.duration", + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -508,9 +508,8 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) - // because of OldHTTPClient require.Len(t, rm.ScopeMetrics[0].Metrics, 2) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -532,7 +531,7 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 4) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -595,9 +594,6 @@ func TestClientRecordResponseSize(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - tests := []struct { name string setEnv bool @@ -621,11 +617,7 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Len(t, rm.ScopeMetrics, 1) - - // because of OldHTTPClient - require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + require.Empty(t, rm.ScopeMetrics) }, }, { @@ -647,7 +639,7 @@ func TestClientRecordResponseSize(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/v1.20.0_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/v1.20.0_test.go deleted file mode 100644 index 6577fd95cf7..00000000000 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/test/v1.20.0_test.go +++ /dev/null @@ -1,311 +0,0 @@ -// Code created by gotmpl. DO NOT MODIFY. -// source: internal/shared/semconv/test/v1.20.0_test.go.tmpl - -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package test - -import ( - "context" - "fmt" - "net/http" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/instrumentation" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" -) - -func TestV120TraceRequest(t *testing.T) { - // Anything but "http" or "http/dup" works. - t.Setenv("OTEL_SEMCONV_STABILITY_OPT_IN", "old") - serv := semconv.NewHTTPServer(nil) - want := func(req testServerReq) []attribute.KeyValue { - return []attribute.KeyValue{ - attribute.String("http.method", "GET"), - attribute.String("http.scheme", "http"), - attribute.String("net.host.name", req.hostname), - attribute.Int("net.host.port", req.serverPort), - attribute.String("net.sock.peer.addr", req.peerAddr), - attribute.Int("net.sock.peer.port", req.peerPort), - attribute.String("user_agent.original", "Go-http-client/1.1"), - attribute.String("http.client_ip", req.clientIP), - attribute.String("net.protocol.version", "1.1"), - attribute.String("http.target", "/"), - } - } - testTraceRequest(t, serv, want) -} - -func TestV120TraceResponse(t *testing.T) { - testCases := []struct { - name string - resp semconv.ResponseTelemetry - want []attribute.KeyValue - }{ - { - name: "empty", - resp: semconv.ResponseTelemetry{}, - want: nil, - }, - { - name: "no errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - WriteBytes: 802, - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.Int("http.response_content_length", 802), - attribute.Int("http.status_code", 200), - }, - }, - { - name: "with errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - ReadError: fmt.Errorf("read error"), - WriteBytes: 802, - WriteError: fmt.Errorf("write error"), - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.String("http.read_error", "read error"), - attribute.Int("http.response_content_length", 802), - attribute.String("http.write_error", "write error"), - attribute.Int("http.status_code", 200), - }, - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - got := semconv.OldHTTPServer{}.ResponseTraceAttrs(tt.resp) - assert.ElementsMatch(t, tt.want, got) - }) - } -} - -func TestV120RecordMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - server := semconv.NewHTTPServer(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - server.RecordMetrics(context.Background(), semconv.ServerMetricData{ - ServerName: "stuff", - ResponseSize: 200, - MetricAttributes: semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }, - MetricData: semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, - }) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - 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"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - -func TestV120ClientRequest(t *testing.T) { - body := strings.NewReader("Hello, world!") - url := "https://example.com:8888/foo/bar?stuff=morestuff" - req, err := http.NewRequest("POST", url, body) - assert.NoError(t, err) - req.Header.Set("User-Agent", "go-test-agent") - - want := []attribute.KeyValue{ - attribute.String("http.method", "POST"), - attribute.String("http.url", url), - attribute.String("net.peer.name", "example.com"), - attribute.Int("net.peer.port", 8888), - attribute.Int("http.request_content_length", body.Len()), - attribute.String("user_agent.original", "go-test-agent"), - } - got := semconv.OldHTTPClient{}.RequestTraceAttrs(req) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientResponse(t *testing.T) { - resp := http.Response{ - StatusCode: 200, - ContentLength: 123, - } - - want := []attribute.KeyValue{ - attribute.Int("http.response_content_length", 123), - attribute.Int("http.status_code", 200), - } - - got := semconv.OldHTTPClient{}.ResponseTraceAttrs(&resp) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - client := semconv.NewHTTPClient(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - opts := client.MetricOptions(semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }) - - ctx := context.Background() - - client.RecordResponseSize(ctx, 200, opts) - - client.RecordMetrics(ctx, semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, opts) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - attribute.String("http.method", "POST"), - attribute.Int64("http.status_code", 301), - attribute.String("key", "value"), - attribute.String("net.peer.name", "example.com"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []metricdata.Metrics{ - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gin_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gin_test.go index 0a90143ab99..1b745f5afd4 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gin_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gin_test.go @@ -31,7 +31,6 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" - semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" ) @@ -166,9 +165,9 @@ func TestTrace200(t *testing.T) { assert.Equal(t, "/user/:id", span.Name()) assert.Equal(t, trace.SpanKindServer, span.SpanKind()) attr := span.Attributes() - assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) + assert.Contains(t, attr, attribute.String("server.address", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusOK)) - assert.Contains(t, attr, attribute.String("http.method", "GET")) + assert.Contains(t, attr, attribute.String("http.request.method", "GET")) assert.Contains(t, attr, attribute.String("http.route", "/user/:id")) assert.Empty(t, span.Events()) assert.Equal(t, codes.Unset, span.Status().Code) @@ -201,7 +200,7 @@ func TestError(t *testing.T) { span := spans[0] assert.Equal(t, "/server_err", span.Name()) attr := span.Attributes() - assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) + assert.Contains(t, attr, attribute.String("server.address", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusInternalServerError)) // verify the error events @@ -326,7 +325,7 @@ func TestHTTPRouteWithSpanNameFormatter(t *testing.T) { assert.Equal(t, "/user/123", span.Name()) assert.Equal(t, trace.SpanKindServer, span.SpanKind()) attr := span.Attributes() - assert.Contains(t, attr, attribute.String("http.method", "GET")) + assert.Contains(t, attr, attribute.String("http.request.method", "GET")) assert.Contains(t, attr, attribute.String("http.route", "/user/:id")) } @@ -498,12 +497,12 @@ func TestMetrics(t *testing.T) { assert.Equal(t, otelgin.Version(), sm.Scope.Version) attrs := []attribute.KeyValue{ - semconv.NetHostName("foobar"), - semconv.HTTPSchemeHTTP, - semconv.NetProtocolName("http"), - semconv.NetProtocolVersion(fmt.Sprintf("1.%d", r.ProtoMinor)), - semconv.HTTPMethod(http.MethodGet), - semconv.HTTPStatusCode(200), + attribute.String("http.request.method", "GET"), + attribute.Int64("http.response.status_code", 200), + attribute.String("network.protocol.name", "http"), + attribute.String("network.protocol.version", fmt.Sprintf("1.%d", r.ProtoMinor)), + attribute.String("server.address", "foobar"), + attribute.String("url.scheme", "http"), } if tt.metricAttributeExtractor != nil { @@ -514,38 +513,44 @@ func TestMetrics(t *testing.T) { } metricdatatest.AssertEqual(t, metricdata.Metrics{ - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ - DataPoints: []metricdata.DataPoint[int64]{{ - Attributes: attribute.NewSet(attrs...), Value: 0, - }}, + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, + DataPoints: []metricdata.HistogramDataPoint[int64]{ + { + Attributes: attribute.NewSet(attrs...), + }, + }, }, - }, sm.Metrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars()) + }, sm.Metrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) metricdatatest.AssertEqual(t, metricdata.Metrics{ - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ - DataPoints: []metricdata.DataPoint[int64]{{ - Attributes: attribute.NewSet(attrs...), Value: 3, - }}, + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, + DataPoints: []metricdata.HistogramDataPoint[int64]{ + { + Attributes: attribute.NewSet(attrs...), + }, + }, }, - }, sm.Metrics[1], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars()) + }, sm.Metrics[1], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) metricdatatest.AssertEqual(t, metricdata.Metrics{ - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", + Name: "http.server.request.duration", + Description: "Duration of HTTP server requests.", + Unit: "s", Data: metricdata.Histogram[float64]{ - DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attribute.NewSet(attrs...)}}, Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{ + { + Attributes: attribute.NewSet(attrs...), + }, + }, }, }, sm.Metrics[2], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) }) diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env.go index 9072d98e3cc..c14d86f7a4a 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env.go @@ -20,7 +20,7 @@ import ( ) // OTelSemConvStabilityOptIn is an environment variable. -// That can be set to "old" or "http/dup" to opt into the new HTTP semantic conventions. +// That can be set to "http/dup" to keep getting the old HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -65,7 +65,7 @@ func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts Req if s.duplicate { return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts)...) } - return OldHTTPServer{}.RequestTraceAttrs(server, req) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) } func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { @@ -73,7 +73,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { return append([]attribute.KeyValue{OldHTTPServer{}.NetworkTransportAttr(network)}, CurrentHTTPServer{}.NetworkTransportAttr(network)) } return []attribute.KeyValue{ - OldHTTPServer{}.NetworkTransportAttr(network), + CurrentHTTPServer{}.NetworkTransportAttr(network), } } @@ -84,12 +84,12 @@ func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyVa if s.duplicate { return append(OldHTTPServer{}.ResponseTraceAttrs(resp), CurrentHTTPServer{}.ResponseTraceAttrs(resp)...) } - return OldHTTPServer{}.ResponseTraceAttrs(resp) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp) } // Route returns the attribute for the route. func (s HTTPServer) Route(route string) attribute.KeyValue { - return OldHTTPServer{}.Route(route) + return CurrentHTTPServer{}.Route(route) } // Status returns a span status code and message for an HTTP status code @@ -141,19 +141,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - if s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { - attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) - *addOpts = append(*addOpts, o) - s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) - s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) - s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) - *addOpts = (*addOpts)[:0] - metricAddOptionPool.Put(addOpts) - } - - if s.duplicate && s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { + if s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { attributes := CurrentHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) o := metric.WithAttributeSet(attribute.NewSet(attributes...)) recordOpts := metricRecordOptionPool.Get().(*[]metric.RecordOption) @@ -164,6 +152,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *recordOpts = (*recordOpts)[:0] metricRecordOptionPool.Put(recordOpts) } + + if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) + o := metric.WithAttributeSet(attribute.NewSet(attributes...)) + addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) + *addOpts = append(*addOpts, o) + s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) + s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) + s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) + *addOpts = (*addOpts)[:0] + metricAddOptionPool.Put(addOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -172,9 +172,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) } return server } @@ -198,9 +198,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) } return client @@ -211,7 +211,7 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { if c.duplicate { return append(OldHTTPClient{}.RequestTraceAttrs(req), CurrentHTTPClient{}.RequestTraceAttrs(req)...) } - return OldHTTPClient{}.RequestTraceAttrs(req) + return CurrentHTTPClient{}.RequestTraceAttrs(req) } // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. @@ -220,7 +220,7 @@ func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue return append(OldHTTPClient{}.ResponseTraceAttrs(resp), CurrentHTTPClient{}.ResponseTraceAttrs(resp)...) } - return OldHTTPClient{}.ResponseTraceAttrs(resp) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp) } func (c HTTPClient) Status(code int) (codes.Code, string) { @@ -234,11 +234,7 @@ func (c HTTPClient) Status(code int) (codes.Code, string) { } func (c HTTPClient) ErrorType(err error) attribute.KeyValue { - if c.duplicate { - return CurrentHTTPClient{}.ErrorType(err) - } - - return attribute.KeyValue{} + return CurrentHTTPClient{}.ErrorType(err) } type MetricOpts struct { @@ -257,17 +253,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["old"] = MetricOpts{ + opts["new"] = MetricOpts{ measurement: set, addOptions: set, } if c.duplicate { - attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["new"] = MetricOpts{ + opts["old"] = MetricOpts{ measurement: set, addOptions: set, } @@ -277,17 +273,17 @@ func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { } func (s HTTPClient) RecordMetrics(ctx context.Context, md MetricData, opts map[string]MetricOpts) { - if s.requestBytesCounter == nil || s.latencyMeasure == nil { + if s.requestBodySize == nil || s.requestDuration == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) - s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) + s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) + s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) if s.duplicate { - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000.0, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) } } @@ -305,5 +301,5 @@ func (s HTTPClient) TraceAttributes(host string) []attribute.KeyValue { return append(OldHTTPClient{}.TraceAttributes(host), CurrentHTTPClient{}.TraceAttributes(host)...) } - return OldHTTPClient{}.TraceAttributes(host) + return CurrentHTTPClient{}.TraceAttributes(host) } diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env_test.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env_test.go index 635f171805a..39b243ba915 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env_test.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/env_test.go @@ -68,16 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - network: "tcp", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), + attribute.String("network.transport", "tcp"), }, }, { @@ -152,15 +143,7 @@ func TestHTTPClientTraceAttributes(t *testing.T) { name: "with no optin set", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "with optin set to old only", - optinVal: "old", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { @@ -196,16 +179,7 @@ func TestClientTraceAttributes(t *testing.T) { host: "example.com", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - host: "example.com", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/gen.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/gen.go index 09cb5c9963b..f43c6baf29b 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/gen.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/gen.go @@ -6,4 +6,3 @@ package test // import "go.opentelemetry.io/contrib/instrumentation/github.com/g // Generate semconv/test package: //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/common_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux\" }" --out=common_test.go //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/httpconv_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux\" }" --out=httpconv_test.go -//go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/v1.20.0_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux\" }" --out=v1.20.0_test.go diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/httpconv_test.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/httpconv_test.go index ba847dd6ca5..47f6013ca37 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/httpconv_test.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/httpconv_test.go @@ -78,49 +78,47 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPServer version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", + 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: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -128,44 +126,46 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPServer version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPServer version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.server.request.body.size", - Description: "Size of HTTP server request bodies.", + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.response.body.size", - Description: "Size of HTTP server response bodies.", + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.request.duration", - Description: "Duration of HTTP server requests.", - Unit: "s", + 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: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -221,7 +221,7 @@ func TestNewServerRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 6) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -416,35 +416,34 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPClient version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.client.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.client.request.body.size", + Description: "Size of HTTP client request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", + Name: "http.client.request.duration", + Description: "Duration of HTTP client requests.", + Unit: "s", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -452,31 +451,32 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPClient version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.client.request.body.size", - Description: "Size of HTTP client request bodies.", + Name: "http.client.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.client.request.duration", - Description: "Duration of HTTP client requests.", - Unit: "s", + Name: "http.client.duration", + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -508,9 +508,8 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) - // because of OldHTTPClient require.Len(t, rm.ScopeMetrics[0].Metrics, 2) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -532,7 +531,7 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 4) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -595,9 +594,6 @@ func TestClientRecordResponseSize(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - tests := []struct { name string setEnv bool @@ -621,11 +617,7 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Len(t, rm.ScopeMetrics, 1) - - // because of OldHTTPClient - require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + require.Empty(t, rm.ScopeMetrics) }, }, { @@ -647,7 +639,7 @@ func TestClientRecordResponseSize(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/v1.20.0_test.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/v1.20.0_test.go deleted file mode 100644 index 9e0b9d75086..00000000000 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/test/v1.20.0_test.go +++ /dev/null @@ -1,311 +0,0 @@ -// Code created by gotmpl. DO NOT MODIFY. -// source: internal/shared/semconv/test/v1.20.0_test.go.tmpl - -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package test - -import ( - "context" - "fmt" - "net/http" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/instrumentation" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" -) - -func TestV120TraceRequest(t *testing.T) { - // Anything but "http" or "http/dup" works. - t.Setenv("OTEL_SEMCONV_STABILITY_OPT_IN", "old") - serv := semconv.NewHTTPServer(nil) - want := func(req testServerReq) []attribute.KeyValue { - return []attribute.KeyValue{ - attribute.String("http.method", "GET"), - attribute.String("http.scheme", "http"), - attribute.String("net.host.name", req.hostname), - attribute.Int("net.host.port", req.serverPort), - attribute.String("net.sock.peer.addr", req.peerAddr), - attribute.Int("net.sock.peer.port", req.peerPort), - attribute.String("user_agent.original", "Go-http-client/1.1"), - attribute.String("http.client_ip", req.clientIP), - attribute.String("net.protocol.version", "1.1"), - attribute.String("http.target", "/"), - } - } - testTraceRequest(t, serv, want) -} - -func TestV120TraceResponse(t *testing.T) { - testCases := []struct { - name string - resp semconv.ResponseTelemetry - want []attribute.KeyValue - }{ - { - name: "empty", - resp: semconv.ResponseTelemetry{}, - want: nil, - }, - { - name: "no errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - WriteBytes: 802, - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.Int("http.response_content_length", 802), - attribute.Int("http.status_code", 200), - }, - }, - { - name: "with errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - ReadError: fmt.Errorf("read error"), - WriteBytes: 802, - WriteError: fmt.Errorf("write error"), - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.String("http.read_error", "read error"), - attribute.Int("http.response_content_length", 802), - attribute.String("http.write_error", "write error"), - attribute.Int("http.status_code", 200), - }, - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - got := semconv.OldHTTPServer{}.ResponseTraceAttrs(tt.resp) - assert.ElementsMatch(t, tt.want, got) - }) - } -} - -func TestV120RecordMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - server := semconv.NewHTTPServer(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - server.RecordMetrics(context.Background(), semconv.ServerMetricData{ - ServerName: "stuff", - ResponseSize: 200, - MetricAttributes: semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }, - MetricData: semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, - }) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - 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"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - -func TestV120ClientRequest(t *testing.T) { - body := strings.NewReader("Hello, world!") - url := "https://example.com:8888/foo/bar?stuff=morestuff" - req, err := http.NewRequest("POST", url, body) - assert.NoError(t, err) - req.Header.Set("User-Agent", "go-test-agent") - - want := []attribute.KeyValue{ - attribute.String("http.method", "POST"), - attribute.String("http.url", url), - attribute.String("net.peer.name", "example.com"), - attribute.Int("net.peer.port", 8888), - attribute.Int("http.request_content_length", body.Len()), - attribute.String("user_agent.original", "go-test-agent"), - } - got := semconv.OldHTTPClient{}.RequestTraceAttrs(req) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientResponse(t *testing.T) { - resp := http.Response{ - StatusCode: 200, - ContentLength: 123, - } - - want := []attribute.KeyValue{ - attribute.Int("http.response_content_length", 123), - attribute.Int("http.status_code", 200), - } - - got := semconv.OldHTTPClient{}.ResponseTraceAttrs(&resp) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - client := semconv.NewHTTPClient(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - opts := client.MetricOptions(semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }) - - ctx := context.Background() - - client.RecordResponseSize(ctx, 200, opts) - - client.RecordMetrics(ctx, semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, opts) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - attribute.String("http.method", "POST"), - attribute.Int64("http.status_code", 301), - attribute.String("key", "value"), - attribute.String("net.peer.name", "example.com"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []metricdata.Metrics{ - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} diff --git a/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go b/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go index 78866e6a6ef..cc7531047b0 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go @@ -104,17 +104,17 @@ func TestSDKIntegration(t *testing.T) { assertSpan(t, sr.Ended()[0], "/user/{id:[0-9]+}", trace.SpanKindServer, - attribute.String("net.host.name", "foobar"), - attribute.Int("http.status_code", http.StatusOK), - attribute.String("http.method", "GET"), + attribute.String("server.address", "foobar"), + attribute.Int("http.response.status_code", http.StatusOK), + attribute.String("http.request.method", "GET"), attribute.String("http.route", "/user/{id:[0-9]+}"), ) assertSpan(t, sr.Ended()[1], "/book/{title}", trace.SpanKindServer, - attribute.String("net.host.name", "foobar"), - attribute.Int("http.status_code", http.StatusOK), - attribute.String("http.method", "GET"), + attribute.String("server.address", "foobar"), + attribute.Int("http.response.status_code", http.StatusOK), + attribute.String("http.request.method", "GET"), attribute.String("http.route", "/book/{title}"), ) } @@ -136,15 +136,17 @@ func TestNotFoundIsNotError(t *testing.T) { assertSpan(t, sr.Ended()[0], "/does/not/exist", trace.SpanKindServer, - attribute.String("net.host.name", "foobar"), - attribute.Int("http.status_code", http.StatusNotFound), - attribute.String("http.method", "GET"), + attribute.String("server.address", "foobar"), + attribute.Int("http.response.status_code", http.StatusNotFound), + attribute.String("http.request.method", "GET"), attribute.String("http.route", "/does/not/exist"), ) assert.Equal(t, codes.Unset, sr.Ended()[0].Status().Code) } func assertSpan(t *testing.T, span sdktrace.ReadOnlySpan, name string, kind trace.SpanKind, attrs ...attribute.KeyValue) { + t.Helper() + assert.Equal(t, name, span.Name()) assert.Equal(t, kind, span.SpanKind()) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go index e9a5a73e71f..3c9c2385e93 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go @@ -16,7 +16,6 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/propagation" - semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" ) @@ -36,11 +35,11 @@ func TestRoundtrip(t *testing.T) { actualAttrs := make(map[attribute.Key]string) for _, attr := range attrs { - if attr.Key == semconv.NetSockPeerPortKey { - // Peer port will be non-deterministic - continue + if expectedAttrs[attr.Key] == "any" { + actualAttrs[attr.Key] = expectedAttrs[attr.Key] + } else { + actualAttrs[attr.Key] = attr.Value.Emit() } - actualAttrs[attr.Key] = attr.Value.Emit() } if diff := cmp.Diff(actualAttrs, expectedAttrs); diff != "" { @@ -71,17 +70,18 @@ func TestRoundtrip(t *testing.T) { address := ts.Listener.Addr() hp := strings.Split(address.String(), ":") expectedAttrs = map[attribute.Key]string{ - semconv.NetHostNameKey: hp[0], - semconv.NetHostPortKey: hp[1], - semconv.NetProtocolVersionKey: "1.1", - semconv.HTTPMethodKey: "GET", - semconv.HTTPSchemeKey: "http", - semconv.HTTPTargetKey: "/", - semconv.HTTPRequestContentLengthKey: "3", - semconv.HTTPClientIPKey: hp[0], - semconv.NetSockPeerAddrKey: hp[0], - semconv.NetTransportKey: "ip_tcp", - semconv.UserAgentOriginalKey: "Go-http-client/1.1", + "client.address": hp[0], + "http.request.body.size": "3", + "http.request.method": "GET", + "network.peer.address": hp[0], + "network.peer.port": "any", + "network.protocol.version": "1.1", + "network.transport": "tcp", + "server.address": "127.0.0.1", + "server.port": hp[1], + "url.path": "/", + "url.scheme": "http", + "user_agent.original": "Go-http-client/1.1", } client := ts.Client() diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env.go index cc92ee57207..2e053e999f2 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env.go @@ -20,7 +20,7 @@ import ( ) // OTelSemConvStabilityOptIn is an environment variable. -// That can be set to "old" or "http/dup" to opt into the new HTTP semantic conventions. +// That can be set to "http/dup" to keep getting the old HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -65,7 +65,7 @@ func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts Req if s.duplicate { return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts)...) } - return OldHTTPServer{}.RequestTraceAttrs(server, req) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) } func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { @@ -73,7 +73,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { return append([]attribute.KeyValue{OldHTTPServer{}.NetworkTransportAttr(network)}, CurrentHTTPServer{}.NetworkTransportAttr(network)) } return []attribute.KeyValue{ - OldHTTPServer{}.NetworkTransportAttr(network), + CurrentHTTPServer{}.NetworkTransportAttr(network), } } @@ -84,12 +84,12 @@ func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyVa if s.duplicate { return append(OldHTTPServer{}.ResponseTraceAttrs(resp), CurrentHTTPServer{}.ResponseTraceAttrs(resp)...) } - return OldHTTPServer{}.ResponseTraceAttrs(resp) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp) } // Route returns the attribute for the route. func (s HTTPServer) Route(route string) attribute.KeyValue { - return OldHTTPServer{}.Route(route) + return CurrentHTTPServer{}.Route(route) } // Status returns a span status code and message for an HTTP status code @@ -141,19 +141,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - if s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { - attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) - *addOpts = append(*addOpts, o) - s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) - s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) - s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) - *addOpts = (*addOpts)[:0] - metricAddOptionPool.Put(addOpts) - } - - if s.duplicate && s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { + if s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { attributes := CurrentHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) o := metric.WithAttributeSet(attribute.NewSet(attributes...)) recordOpts := metricRecordOptionPool.Get().(*[]metric.RecordOption) @@ -164,6 +152,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *recordOpts = (*recordOpts)[:0] metricRecordOptionPool.Put(recordOpts) } + + if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) + o := metric.WithAttributeSet(attribute.NewSet(attributes...)) + addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) + *addOpts = append(*addOpts, o) + s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) + s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) + s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) + *addOpts = (*addOpts)[:0] + metricAddOptionPool.Put(addOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -172,9 +172,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) } return server } @@ -198,9 +198,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) } return client @@ -211,7 +211,7 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { if c.duplicate { return append(OldHTTPClient{}.RequestTraceAttrs(req), CurrentHTTPClient{}.RequestTraceAttrs(req)...) } - return OldHTTPClient{}.RequestTraceAttrs(req) + return CurrentHTTPClient{}.RequestTraceAttrs(req) } // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. @@ -220,7 +220,7 @@ func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue return append(OldHTTPClient{}.ResponseTraceAttrs(resp), CurrentHTTPClient{}.ResponseTraceAttrs(resp)...) } - return OldHTTPClient{}.ResponseTraceAttrs(resp) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp) } func (c HTTPClient) Status(code int) (codes.Code, string) { @@ -234,11 +234,7 @@ func (c HTTPClient) Status(code int) (codes.Code, string) { } func (c HTTPClient) ErrorType(err error) attribute.KeyValue { - if c.duplicate { - return CurrentHTTPClient{}.ErrorType(err) - } - - return attribute.KeyValue{} + return CurrentHTTPClient{}.ErrorType(err) } type MetricOpts struct { @@ -257,17 +253,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["old"] = MetricOpts{ + opts["new"] = MetricOpts{ measurement: set, addOptions: set, } if c.duplicate { - attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["new"] = MetricOpts{ + opts["old"] = MetricOpts{ measurement: set, addOptions: set, } @@ -277,17 +273,17 @@ func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { } func (s HTTPClient) RecordMetrics(ctx context.Context, md MetricData, opts map[string]MetricOpts) { - if s.requestBytesCounter == nil || s.latencyMeasure == nil { + if s.requestBodySize == nil || s.requestDuration == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) - s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) + s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) + s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) if s.duplicate { - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000.0, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) } } @@ -305,5 +301,5 @@ func (s HTTPClient) TraceAttributes(host string) []attribute.KeyValue { return append(OldHTTPClient{}.TraceAttributes(host), CurrentHTTPClient{}.TraceAttributes(host)...) } - return OldHTTPClient{}.TraceAttributes(host) + return CurrentHTTPClient{}.TraceAttributes(host) } diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env_test.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env_test.go index 635f171805a..39b243ba915 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env_test.go @@ -68,16 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - network: "tcp", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), + attribute.String("network.transport", "tcp"), }, }, { @@ -152,15 +143,7 @@ func TestHTTPClientTraceAttributes(t *testing.T) { name: "with no optin set", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "with optin set to old only", - optinVal: "old", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { @@ -196,16 +179,7 @@ func TestClientTraceAttributes(t *testing.T) { host: "example.com", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - host: "example.com", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/gen.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/gen.go index ff9151d0934..c24c59e553c 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/gen.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/gen.go @@ -6,4 +6,3 @@ package test // import "go.opentelemetry.io/contrib/instrumentation/net/http/htt // Generate semconv/test package: //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/common_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace\" }" --out=common_test.go //go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/httpconv_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace\" }" --out=httpconv_test.go -//go:generate gotmpl --body=../../../../../../../../internal/shared/semconv/test/v1.20.0_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace\" }" --out=v1.20.0_test.go diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/httpconv_test.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/httpconv_test.go index 96170d3a0b9..2d0d70ef4fc 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/httpconv_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/httpconv_test.go @@ -78,49 +78,47 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPServer version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", + 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: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -128,44 +126,46 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPServer version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPServer version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.server.request.body.size", - Description: "Size of HTTP server request bodies.", + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.response.body.size", - Description: "Size of HTTP server response bodies.", + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.request.duration", - Description: "Duration of HTTP server requests.", - Unit: "s", + 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: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -221,7 +221,7 @@ func TestNewServerRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 6) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -416,35 +416,34 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPClient version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.client.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.client.request.body.size", + Description: "Size of HTTP client request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", + Name: "http.client.request.duration", + Description: "Duration of HTTP client requests.", + Unit: "s", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -452,31 +451,32 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPClient version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.client.request.body.size", - Description: "Size of HTTP client request bodies.", + Name: "http.client.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.client.request.duration", - Description: "Duration of HTTP client requests.", - Unit: "s", + Name: "http.client.duration", + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -508,9 +508,8 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) - // because of OldHTTPClient require.Len(t, rm.ScopeMetrics[0].Metrics, 2) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -532,7 +531,7 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 4) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -595,9 +594,6 @@ func TestClientRecordResponseSize(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - tests := []struct { name string setEnv bool @@ -621,11 +617,7 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Len(t, rm.ScopeMetrics, 1) - - // because of OldHTTPClient - require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + require.Empty(t, rm.ScopeMetrics) }, }, { @@ -647,7 +639,7 @@ func TestClientRecordResponseSize(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/v1.20.0_test.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/v1.20.0_test.go deleted file mode 100644 index a51c73f5786..00000000000 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/test/v1.20.0_test.go +++ /dev/null @@ -1,311 +0,0 @@ -// Code created by gotmpl. DO NOT MODIFY. -// source: internal/shared/semconv/test/v1.20.0_test.go.tmpl - -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package test - -import ( - "context" - "fmt" - "net/http" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/instrumentation" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" -) - -func TestV120TraceRequest(t *testing.T) { - // Anything but "http" or "http/dup" works. - t.Setenv("OTEL_SEMCONV_STABILITY_OPT_IN", "old") - serv := semconv.NewHTTPServer(nil) - want := func(req testServerReq) []attribute.KeyValue { - return []attribute.KeyValue{ - attribute.String("http.method", "GET"), - attribute.String("http.scheme", "http"), - attribute.String("net.host.name", req.hostname), - attribute.Int("net.host.port", req.serverPort), - attribute.String("net.sock.peer.addr", req.peerAddr), - attribute.Int("net.sock.peer.port", req.peerPort), - attribute.String("user_agent.original", "Go-http-client/1.1"), - attribute.String("http.client_ip", req.clientIP), - attribute.String("net.protocol.version", "1.1"), - attribute.String("http.target", "/"), - } - } - testTraceRequest(t, serv, want) -} - -func TestV120TraceResponse(t *testing.T) { - testCases := []struct { - name string - resp semconv.ResponseTelemetry - want []attribute.KeyValue - }{ - { - name: "empty", - resp: semconv.ResponseTelemetry{}, - want: nil, - }, - { - name: "no errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - WriteBytes: 802, - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.Int("http.response_content_length", 802), - attribute.Int("http.status_code", 200), - }, - }, - { - name: "with errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - ReadError: fmt.Errorf("read error"), - WriteBytes: 802, - WriteError: fmt.Errorf("write error"), - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.String("http.read_error", "read error"), - attribute.Int("http.response_content_length", 802), - attribute.String("http.write_error", "write error"), - attribute.Int("http.status_code", 200), - }, - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - got := semconv.OldHTTPServer{}.ResponseTraceAttrs(tt.resp) - assert.ElementsMatch(t, tt.want, got) - }) - } -} - -func TestV120RecordMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - server := semconv.NewHTTPServer(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - server.RecordMetrics(context.Background(), semconv.ServerMetricData{ - ServerName: "stuff", - ResponseSize: 200, - MetricAttributes: semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }, - MetricData: semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, - }) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - 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"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - -func TestV120ClientRequest(t *testing.T) { - body := strings.NewReader("Hello, world!") - url := "https://example.com:8888/foo/bar?stuff=morestuff" - req, err := http.NewRequest("POST", url, body) - assert.NoError(t, err) - req.Header.Set("User-Agent", "go-test-agent") - - want := []attribute.KeyValue{ - attribute.String("http.method", "POST"), - attribute.String("http.url", url), - attribute.String("net.peer.name", "example.com"), - attribute.Int("net.peer.port", 8888), - attribute.Int("http.request_content_length", body.Len()), - attribute.String("user_agent.original", "go-test-agent"), - } - got := semconv.OldHTTPClient{}.RequestTraceAttrs(req) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientResponse(t *testing.T) { - resp := http.Response{ - StatusCode: 200, - ContentLength: 123, - } - - want := []attribute.KeyValue{ - attribute.Int("http.response_content_length", 123), - attribute.Int("http.status_code", 200), - } - - got := semconv.OldHTTPClient{}.ResponseTraceAttrs(&resp) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - client := semconv.NewHTTPClient(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - opts := client.MetricOptions(semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }) - - ctx := context.Background() - - client.RecordResponseSize(ctx, 200, opts) - - client.RecordMetrics(ctx, semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, opts) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - attribute.String("http.method", "POST"), - attribute.Int64("http.status_code", 301), - attribute.String("key", "value"), - attribute.String("net.peer.name", "example.com"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []metricdata.Metrics{ - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go index 96f7888f68a..4b391250c9a 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go @@ -93,7 +93,7 @@ func TestHTTPRequestWithClientTrace(t *testing.T) { name: "http.getconn", attributes: []attribute.KeyValue{ attribute.Key("http.remote").String(address.String()), - attribute.Key("net.host.name").String(address.String()), + attribute.Key("server.address").String(address.String()), attribute.Key("http.conn.reused").Bool(false), attribute.Key("http.conn.wasidle").Bool(false), }, @@ -348,7 +348,7 @@ func TestWithoutSubSpans(t *testing.T) { {"http.getconn.start", func(t *testing.T, got attrMap) { assert.Equal(t, attribute.StringValue(fixture.Address), - got[attribute.Key("net.host.name")], + got[attribute.Key("server.address")], ) }}, {"http.getconn.done", func(t *testing.T, got attrMap) { diff --git a/instrumentation/net/http/otelhttp/handler_test.go b/instrumentation/net/http/otelhttp/handler_test.go index 4e96236ba16..d05f7624514 100644 --- a/instrumentation/net/http/otelhttp/handler_test.go +++ b/instrumentation/net/http/otelhttp/handler_test.go @@ -141,13 +141,13 @@ func TestHandlerBasics(t *testing.T) { require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) attrs := attribute.NewSet( - semconv.NetHostName(r.Host), - semconv.HTTPSchemeHTTP, - semconv.NetProtocolName("http"), - semconv.NetProtocolVersion(fmt.Sprintf("1.%d", r.ProtoMinor)), - semconv.HTTPMethod("GET"), + attribute.String("http.request.method", "GET"), + attribute.Int64("http.response.status_code", 200), + attribute.String("network.protocol.name", "http"), + attribute.String("network.protocol.version", fmt.Sprintf("1.%d", r.ProtoMinor)), + attribute.String("server.address", r.Host), + attribute.String("url.scheme", "http"), attribute.String("test", "attribute"), - semconv.HTTPStatusCode(200), ) assertScopeMetrics(t, rm.ScopeMetrics[0], attrs) @@ -181,43 +181,57 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut require.Len(t, sm.Metrics, 3) - want := metricdata.Metrics{ - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", - Unit: "By", - Data: metricdata.Sum[int64]{ - DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 0}}, - Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, + want := metricdata.ScopeMetrics{ + Scope: instrumentation.Scope{ + Name: ScopeName, + Version: Version(), }, - } - metricdatatest.AssertEqual(t, want, sm.Metrics[0], metricdatatest.IgnoreTimestamp()) - - want = metricdata.Metrics{ - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", - Unit: "By", - Data: metricdata.Sum[int64]{ - DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 11}}, - Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - }, - } - metricdatatest.AssertEqual(t, want, sm.Metrics[1], metricdatatest.IgnoreTimestamp()) - - want = metricdata.Metrics{ - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}}, - Temporality: metricdata.CumulativeTemporality, + Metrics: []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: attrs, + }, + }, + }, + }, + { + 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: attrs, + }, + }, + }, + }, + { + 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: attrs, + }, + }, + }, + }, }, } - metricdatatest.AssertEqual(t, want, sm.Metrics[2], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, want, sm, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) // verify that the custom start time, which is 10 minutes in the past, is respected. - assert.GreaterOrEqual(t, sm.Metrics[2].Data.(metricdata.Histogram[float64]).DataPoints[0].Sum, float64(10*time.Minute/time.Millisecond)) + assert.GreaterOrEqual(t, sm.Metrics[2].Data.(metricdata.Histogram[float64]).DataPoints[0].Sum, float64(10*time.Minute/time.Second)) } func TestHandlerEmittedAttributes(t *testing.T) { @@ -232,7 +246,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { w.WriteHeader(http.StatusOK) }, attributes: []attribute.KeyValue{ - attribute.Int("http.status_code", http.StatusOK), + attribute.Int("http.response.status_code", http.StatusOK), }, }, { @@ -241,7 +255,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { w.WriteHeader(http.StatusBadRequest) }, attributes: []attribute.KeyValue{ - attribute.Int("http.status_code", http.StatusBadRequest), + attribute.Int("http.response.status_code", http.StatusBadRequest), }, }, { @@ -249,7 +263,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { handler: func(w http.ResponseWriter, r *http.Request) { }, attributes: []attribute.KeyValue{ - attribute.Int("http.status_code", http.StatusOK), + attribute.Int("http.response.status_code", http.StatusOK), }, }, { @@ -259,7 +273,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { w.WriteHeader(http.StatusOK) }, attributes: []attribute.KeyValue{ - attribute.Int("http.status_code", http.StatusInternalServerError), + attribute.Int("http.response.status_code", http.StatusInternalServerError), }, }, } diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env.go b/instrumentation/net/http/otelhttp/internal/semconv/env.go index dc7306a58ca..a738691f178 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env.go @@ -20,7 +20,7 @@ import ( ) // OTelSemConvStabilityOptIn is an environment variable. -// That can be set to "old" or "http/dup" to opt into the new HTTP semantic conventions. +// That can be set to "http/dup" to keep getting the old HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -65,7 +65,7 @@ func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts Req if s.duplicate { return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts)...) } - return OldHTTPServer{}.RequestTraceAttrs(server, req) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) } func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { @@ -73,7 +73,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { return append([]attribute.KeyValue{OldHTTPServer{}.NetworkTransportAttr(network)}, CurrentHTTPServer{}.NetworkTransportAttr(network)) } return []attribute.KeyValue{ - OldHTTPServer{}.NetworkTransportAttr(network), + CurrentHTTPServer{}.NetworkTransportAttr(network), } } @@ -84,12 +84,12 @@ func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyVa if s.duplicate { return append(OldHTTPServer{}.ResponseTraceAttrs(resp), CurrentHTTPServer{}.ResponseTraceAttrs(resp)...) } - return OldHTTPServer{}.ResponseTraceAttrs(resp) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp) } // Route returns the attribute for the route. func (s HTTPServer) Route(route string) attribute.KeyValue { - return OldHTTPServer{}.Route(route) + return CurrentHTTPServer{}.Route(route) } // Status returns a span status code and message for an HTTP status code @@ -141,19 +141,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - if s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { - attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) - *addOpts = append(*addOpts, o) - s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) - s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) - s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) - *addOpts = (*addOpts)[:0] - metricAddOptionPool.Put(addOpts) - } - - if s.duplicate && s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { + if s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { attributes := CurrentHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) o := metric.WithAttributeSet(attribute.NewSet(attributes...)) recordOpts := metricRecordOptionPool.Get().(*[]metric.RecordOption) @@ -164,6 +152,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *recordOpts = (*recordOpts)[:0] metricRecordOptionPool.Put(recordOpts) } + + if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) + o := metric.WithAttributeSet(attribute.NewSet(attributes...)) + addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) + *addOpts = append(*addOpts, o) + s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) + s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) + s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) + *addOpts = (*addOpts)[:0] + metricAddOptionPool.Put(addOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -172,9 +172,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) } return server } @@ -198,9 +198,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) } return client @@ -211,7 +211,7 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { if c.duplicate { return append(OldHTTPClient{}.RequestTraceAttrs(req), CurrentHTTPClient{}.RequestTraceAttrs(req)...) } - return OldHTTPClient{}.RequestTraceAttrs(req) + return CurrentHTTPClient{}.RequestTraceAttrs(req) } // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. @@ -220,7 +220,7 @@ func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue return append(OldHTTPClient{}.ResponseTraceAttrs(resp), CurrentHTTPClient{}.ResponseTraceAttrs(resp)...) } - return OldHTTPClient{}.ResponseTraceAttrs(resp) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp) } func (c HTTPClient) Status(code int) (codes.Code, string) { @@ -234,11 +234,7 @@ func (c HTTPClient) Status(code int) (codes.Code, string) { } func (c HTTPClient) ErrorType(err error) attribute.KeyValue { - if c.duplicate { - return CurrentHTTPClient{}.ErrorType(err) - } - - return attribute.KeyValue{} + return CurrentHTTPClient{}.ErrorType(err) } type MetricOpts struct { @@ -257,17 +253,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["old"] = MetricOpts{ + opts["new"] = MetricOpts{ measurement: set, addOptions: set, } if c.duplicate { - attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["new"] = MetricOpts{ + opts["old"] = MetricOpts{ measurement: set, addOptions: set, } @@ -277,17 +273,17 @@ func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { } func (s HTTPClient) RecordMetrics(ctx context.Context, md MetricData, opts map[string]MetricOpts) { - if s.requestBytesCounter == nil || s.latencyMeasure == nil { + if s.requestBodySize == nil || s.requestDuration == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) - s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) + s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) + s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) if s.duplicate { - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000.0, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) } } @@ -305,5 +301,5 @@ func (s HTTPClient) TraceAttributes(host string) []attribute.KeyValue { return append(OldHTTPClient{}.TraceAttributes(host), CurrentHTTPClient{}.TraceAttributes(host)...) } - return OldHTTPClient{}.TraceAttributes(host) + return CurrentHTTPClient{}.TraceAttributes(host) } diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env_test.go b/instrumentation/net/http/otelhttp/internal/semconv/env_test.go index 635f171805a..39b243ba915 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env_test.go @@ -68,16 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - network: "tcp", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), + attribute.String("network.transport", "tcp"), }, }, { @@ -152,15 +143,7 @@ func TestHTTPClientTraceAttributes(t *testing.T) { name: "with no optin set", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "with optin set to old only", - optinVal: "old", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { @@ -196,16 +179,7 @@ func TestClientTraceAttributes(t *testing.T) { host: "example.com", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - host: "example.com", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { diff --git a/instrumentation/net/http/otelhttp/internal/semconv/test/gen.go b/instrumentation/net/http/otelhttp/internal/semconv/test/gen.go index aa1278680d4..e932d900ec4 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/test/gen.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/test/gen.go @@ -7,4 +7,3 @@ package test // import "go.opentelemetry.io/contrib/instrumentation/net/http/ote // Generate semconv/test package: //go:generate gotmpl --body=../../../../../../../internal/shared/semconv/test/common_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp\" }" --out=common_test.go //go:generate gotmpl --body=../../../../../../../internal/shared/semconv/test/httpconv_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp\" }" --out=httpconv_test.go -//go:generate gotmpl --body=../../../../../../../internal/shared/semconv/test/v1.20.0_test.go.tmpl "--data={ \"pkg\": \"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp\" }" --out=v1.20.0_test.go diff --git a/instrumentation/net/http/otelhttp/internal/semconv/test/httpconv_test.go b/instrumentation/net/http/otelhttp/internal/semconv/test/httpconv_test.go index 768cf7708c3..ed7d65d5e8b 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/test/httpconv_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/test/httpconv_test.go @@ -78,49 +78,47 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPServer version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", + 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: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -128,44 +126,46 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPServer version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPServer version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.server.request.body.size", - Description: "Size of HTTP server request bodies.", + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.response.body.size", - Description: "Size of HTTP server response bodies.", + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.request.duration", - Description: "Duration of HTTP server requests.", - Unit: "s", + 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: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -221,7 +221,7 @@ func TestNewServerRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 6) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -416,35 +416,34 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPClient version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.client.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.client.request.body.size", + Description: "Size of HTTP client request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", + Name: "http.client.request.duration", + Description: "Duration of HTTP client requests.", + Unit: "s", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -452,31 +451,32 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPClient version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.client.request.body.size", - Description: "Size of HTTP client request bodies.", + Name: "http.client.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.client.request.duration", - Description: "Duration of HTTP client requests.", - Unit: "s", + Name: "http.client.duration", + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -508,9 +508,8 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) - // because of OldHTTPClient require.Len(t, rm.ScopeMetrics[0].Metrics, 2) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -532,7 +531,7 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 4) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -595,9 +594,6 @@ func TestClientRecordResponseSize(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - tests := []struct { name string setEnv bool @@ -621,11 +617,7 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Len(t, rm.ScopeMetrics, 1) - - // because of OldHTTPClient - require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + require.Empty(t, rm.ScopeMetrics) }, }, { @@ -647,7 +639,7 @@ func TestClientRecordResponseSize(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/instrumentation/net/http/otelhttp/internal/semconv/test/v1.20.0_test.go b/instrumentation/net/http/otelhttp/internal/semconv/test/v1.20.0_test.go deleted file mode 100644 index f5caed6442b..00000000000 --- a/instrumentation/net/http/otelhttp/internal/semconv/test/v1.20.0_test.go +++ /dev/null @@ -1,311 +0,0 @@ -// Code created by gotmpl. DO NOT MODIFY. -// source: internal/shared/semconv/test/v1.20.0_test.go.tmpl - -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package test - -import ( - "context" - "fmt" - "net/http" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/instrumentation" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" -) - -func TestV120TraceRequest(t *testing.T) { - // Anything but "http" or "http/dup" works. - t.Setenv("OTEL_SEMCONV_STABILITY_OPT_IN", "old") - serv := semconv.NewHTTPServer(nil) - want := func(req testServerReq) []attribute.KeyValue { - return []attribute.KeyValue{ - attribute.String("http.method", "GET"), - attribute.String("http.scheme", "http"), - attribute.String("net.host.name", req.hostname), - attribute.Int("net.host.port", req.serverPort), - attribute.String("net.sock.peer.addr", req.peerAddr), - attribute.Int("net.sock.peer.port", req.peerPort), - attribute.String("user_agent.original", "Go-http-client/1.1"), - attribute.String("http.client_ip", req.clientIP), - attribute.String("net.protocol.version", "1.1"), - attribute.String("http.target", "/"), - } - } - testTraceRequest(t, serv, want) -} - -func TestV120TraceResponse(t *testing.T) { - testCases := []struct { - name string - resp semconv.ResponseTelemetry - want []attribute.KeyValue - }{ - { - name: "empty", - resp: semconv.ResponseTelemetry{}, - want: nil, - }, - { - name: "no errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - WriteBytes: 802, - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.Int("http.response_content_length", 802), - attribute.Int("http.status_code", 200), - }, - }, - { - name: "with errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - ReadError: fmt.Errorf("read error"), - WriteBytes: 802, - WriteError: fmt.Errorf("write error"), - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.String("http.read_error", "read error"), - attribute.Int("http.response_content_length", 802), - attribute.String("http.write_error", "write error"), - attribute.Int("http.status_code", 200), - }, - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - got := semconv.OldHTTPServer{}.ResponseTraceAttrs(tt.resp) - assert.ElementsMatch(t, tt.want, got) - }) - } -} - -func TestV120RecordMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - server := semconv.NewHTTPServer(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - server.RecordMetrics(context.Background(), semconv.ServerMetricData{ - ServerName: "stuff", - ResponseSize: 200, - MetricAttributes: semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }, - MetricData: semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, - }) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - 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"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - -func TestV120ClientRequest(t *testing.T) { - body := strings.NewReader("Hello, world!") - url := "https://example.com:8888/foo/bar?stuff=morestuff" - req, err := http.NewRequest("POST", url, body) - assert.NoError(t, err) - req.Header.Set("User-Agent", "go-test-agent") - - want := []attribute.KeyValue{ - attribute.String("http.method", "POST"), - attribute.String("http.url", url), - attribute.String("net.peer.name", "example.com"), - attribute.Int("net.peer.port", 8888), - attribute.Int("http.request_content_length", body.Len()), - attribute.String("user_agent.original", "go-test-agent"), - } - got := semconv.OldHTTPClient{}.RequestTraceAttrs(req) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientResponse(t *testing.T) { - resp := http.Response{ - StatusCode: 200, - ContentLength: 123, - } - - want := []attribute.KeyValue{ - attribute.Int("http.response_content_length", 123), - attribute.Int("http.status_code", 200), - } - - got := semconv.OldHTTPClient{}.ResponseTraceAttrs(&resp) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - client := semconv.NewHTTPClient(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - opts := client.MetricOptions(semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }) - - ctx := context.Background() - - client.RecordResponseSize(ctx, 200, opts) - - client.RecordMetrics(ctx, semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, opts) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - attribute.String("http.method", "POST"), - attribute.Int64("http.status_code", 301), - attribute.String("key", "value"), - attribute.String("net.peer.name", "example.com"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []metricdata.Metrics{ - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} diff --git a/instrumentation/net/http/otelhttp/transport_test.go b/instrumentation/net/http/otelhttp/transport_test.go index 7251b37657e..0b83ef144e2 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -30,7 +30,6 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" - semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" ) @@ -707,12 +706,15 @@ func TestTransportMetrics(t *testing.T) { require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) attrs := attribute.NewSet( - semconv.NetPeerName(host), - semconv.NetPeerPort(port), - semconv.HTTPMethod("GET"), - semconv.HTTPStatusCode(200), + attribute.String("http.request.method", "GET"), + attribute.Int("http.response.status_code", 200), + attribute.String("server.address", host), + attribute.Int("server.port", port), + attribute.String("url.scheme", "http"), + attribute.String("network.protocol.name", "http"), + attribute.String("network.protocol.version", "1.1"), ) - assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 13) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs) }) t.Run("make http request and buffer response", func(t *testing.T) { @@ -776,12 +778,15 @@ func TestTransportMetrics(t *testing.T) { require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) attrs := attribute.NewSet( - semconv.NetPeerName(host), - semconv.NetPeerPort(port), - semconv.HTTPMethod("GET"), - semconv.HTTPStatusCode(200), + attribute.String("http.request.method", "GET"), + attribute.Int("http.response.status_code", 200), + attribute.String("server.address", host), + attribute.Int("server.port", port), + attribute.String("url.scheme", "http"), + attribute.String("network.protocol.name", "http"), + attribute.String("network.protocol.version", "1.1"), ) - assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 13) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs) }) t.Run("make http request and close body before reading completely", func(t *testing.T) { @@ -840,57 +845,61 @@ func TestTransportMetrics(t *testing.T) { require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) attrs := attribute.NewSet( - semconv.NetPeerName(host), - semconv.NetPeerPort(port), - semconv.HTTPMethod("GET"), - semconv.HTTPStatusCode(200), + attribute.String("http.request.method", "GET"), + attribute.Int("http.response.status_code", 200), + attribute.String("server.address", host), + attribute.Int("server.port", port), + attribute.String("url.scheme", "http"), + attribute.String("network.protocol.name", "http"), + attribute.String("network.protocol.version", "1.1"), ) - assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 10) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs) }) } -func assertClientScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribute.Set, rxBytes int64) { +func assertClientScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribute.Set) { assert.Equal(t, instrumentation.Scope{ Name: "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp", Version: Version(), }, sm.Scope) - require.Len(t, sm.Metrics, 3) + require.Len(t, sm.Metrics, 2) - want := metricdata.Metrics{ - Name: "http.client.request.size", - Data: metricdata.Sum[int64]{ - DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 4}}, - Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, + want := metricdata.ScopeMetrics{ + Scope: instrumentation.Scope{ + Name: ScopeName, + Version: Version(), }, - Description: "Measures the size of HTTP request messages.", - Unit: "By", - } - metricdatatest.AssertEqual(t, want, sm.Metrics[0], metricdatatest.IgnoreTimestamp()) - - want = metricdata.Metrics{ - Name: "http.client.response.size", - Data: metricdata.Sum[int64]{ - DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: rxBytes}}, - Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - }, - Description: "Measures the size of HTTP response messages.", - Unit: "By", - } - metricdatatest.AssertEqual(t, want, sm.Metrics[1], metricdatatest.IgnoreTimestamp()) - - want = metricdata.Metrics{ - Name: "http.client.duration", - Data: metricdata.Histogram[float64]{ - DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}}, - Temporality: metricdata.CumulativeTemporality, + Metrics: []metricdata.Metrics{ + { + Name: "http.client.request.body.size", + Description: "Size of HTTP client request bodies.", + Unit: "By", + Data: metricdata.Histogram[int64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[int64]{ + { + Attributes: attrs, + }, + }, + }, + }, + { + Name: "http.client.request.duration", + Description: "Duration of HTTP client requests.", + Unit: "s", + Data: metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{ + { + Attributes: attrs, + }, + }, + }, + }, }, - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", } - metricdatatest.AssertEqual(t, want, sm.Metrics[2], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, want, sm, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) } func TestCustomAttributesHandling(t *testing.T) { @@ -1002,7 +1011,7 @@ func TestDefaultAttributesHandling(t *testing.T) { err = reader.Collect(ctx, &rm) assert.NoError(t, err) - assert.Len(t, rm.ScopeMetrics[0].Metrics, 3) + assert.Len(t, rm.ScopeMetrics[0].Metrics, 2) for _, m := range rm.ScopeMetrics[0].Metrics { switch m.Name { case clientRequestSize: diff --git a/internal/shared/semconv/env.go.tmpl b/internal/shared/semconv/env.go.tmpl index eda5e7eb81a..b0a159c6df8 100644 --- a/internal/shared/semconv/env.go.tmpl +++ b/internal/shared/semconv/env.go.tmpl @@ -20,7 +20,7 @@ import ( ) // OTelSemConvStabilityOptIn is an environment variable. -// That can be set to "old" or "http/dup" to opt into the new HTTP semantic conventions. +// That can be set to "http/dup" to keep getting the old HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -65,7 +65,7 @@ func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts Req if s.duplicate { return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts)...) } - return OldHTTPServer{}.RequestTraceAttrs(server, req) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) } func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { @@ -73,7 +73,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { return append([]attribute.KeyValue{OldHTTPServer{}.NetworkTransportAttr(network)}, CurrentHTTPServer{}.NetworkTransportAttr(network)) } return []attribute.KeyValue{ - OldHTTPServer{}.NetworkTransportAttr(network), + CurrentHTTPServer{}.NetworkTransportAttr(network), } } @@ -84,12 +84,12 @@ func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyVa if s.duplicate { return append(OldHTTPServer{}.ResponseTraceAttrs(resp), CurrentHTTPServer{}.ResponseTraceAttrs(resp)...) } - return OldHTTPServer{}.ResponseTraceAttrs(resp) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp) } // Route returns the attribute for the route. func (s HTTPServer) Route(route string) attribute.KeyValue { - return OldHTTPServer{}.Route(route) + return CurrentHTTPServer{}.Route(route) } // Status returns a span status code and message for an HTTP status code @@ -141,19 +141,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - if s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { - attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) - o := metric.WithAttributeSet(attribute.NewSet(attributes...)) - addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) - *addOpts = append(*addOpts, o) - s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) - s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) - s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) - *addOpts = (*addOpts)[:0] - metricAddOptionPool.Put(addOpts) - } - - if s.duplicate && s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { + if s.requestDurationHistogram != nil && s.requestBodySizeHistogram != nil && s.responseBodySizeHistogram != nil { attributes := CurrentHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) o := metric.WithAttributeSet(attribute.NewSet(attributes...)) recordOpts := metricRecordOptionPool.Get().(*[]metric.RecordOption) @@ -164,6 +152,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *recordOpts = (*recordOpts)[:0] metricRecordOptionPool.Put(recordOpts) } + + if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + attributes := OldHTTPServer{}.MetricAttributes(md.ServerName, md.Req, md.StatusCode, md.AdditionalAttributes) + o := metric.WithAttributeSet(attribute.NewSet(attributes...)) + addOpts := metricAddOptionPool.Get().(*[]metric.AddOption) + *addOpts = append(*addOpts, o) + s.requestBytesCounter.Add(ctx, md.RequestSize, *addOpts...) + s.responseBytesCounter.Add(ctx, md.ResponseSize, *addOpts...) + s.serverLatencyMeasure.Record(ctx, md.ElapsedTime, o) + *addOpts = (*addOpts)[:0] + metricAddOptionPool.Put(addOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -172,9 +172,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) } return server } @@ -198,9 +198,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) } return client @@ -211,7 +211,7 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { if c.duplicate { return append(OldHTTPClient{}.RequestTraceAttrs(req), CurrentHTTPClient{}.RequestTraceAttrs(req)...) } - return OldHTTPClient{}.RequestTraceAttrs(req) + return CurrentHTTPClient{}.RequestTraceAttrs(req) } // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. @@ -220,7 +220,7 @@ func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue return append(OldHTTPClient{}.ResponseTraceAttrs(resp), CurrentHTTPClient{}.ResponseTraceAttrs(resp)...) } - return OldHTTPClient{}.ResponseTraceAttrs(resp) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp) } func (c HTTPClient) Status(code int) (codes.Code, string) { @@ -234,11 +234,7 @@ func (c HTTPClient) Status(code int) (codes.Code, string) { } func (c HTTPClient) ErrorType(err error) attribute.KeyValue { - if c.duplicate { - return CurrentHTTPClient{}.ErrorType(err) - } - - return attribute.KeyValue{} + return CurrentHTTPClient{}.ErrorType(err) } type MetricOpts struct { @@ -257,17 +253,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["old"] = MetricOpts{ + opts["new"] = MetricOpts{ measurement: set, addOptions: set, } if c.duplicate { - attributes := CurrentHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) + attributes := OldHTTPClient{}.MetricAttributes(ma.Req, ma.StatusCode, ma.AdditionalAttributes) set := metric.WithAttributeSet(attribute.NewSet(attributes...)) - opts["new"] = MetricOpts{ + opts["old"] = MetricOpts{ measurement: set, addOptions: set, } @@ -277,17 +273,17 @@ func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { } func (s HTTPClient) RecordMetrics(ctx context.Context, md MetricData, opts map[string]MetricOpts) { - if s.requestBytesCounter == nil || s.latencyMeasure == nil { + if s.requestBodySize == nil || s.requestDuration == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) - s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) + s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) + s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) if s.duplicate { - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000.0, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) } } @@ -305,5 +301,5 @@ func (s HTTPClient) TraceAttributes(host string) []attribute.KeyValue { return append(OldHTTPClient{}.TraceAttributes(host), CurrentHTTPClient{}.TraceAttributes(host)...) } - return OldHTTPClient{}.TraceAttributes(host) + return CurrentHTTPClient{}.TraceAttributes(host) } diff --git a/internal/shared/semconv/env_test.go.tmpl b/internal/shared/semconv/env_test.go.tmpl index 635f171805a..39b243ba915 100644 --- a/internal/shared/semconv/env_test.go.tmpl +++ b/internal/shared/semconv/env_test.go.tmpl @@ -68,16 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - network: "tcp", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.transport", "ip_tcp"), + attribute.String("network.transport", "tcp"), }, }, { @@ -152,15 +143,7 @@ func TestHTTPClientTraceAttributes(t *testing.T) { name: "with no optin set", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "with optin set to old only", - optinVal: "old", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { @@ -196,16 +179,7 @@ func TestClientTraceAttributes(t *testing.T) { host: "example.com", wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), - }, - }, - { - name: "without an old optin", - optinVal: "old", - host: "example.com", - - wantAttributes: []attribute.KeyValue{ - attribute.String("net.host.name", "example.com"), + attribute.String("server.address", "example.com"), }, }, { diff --git a/internal/shared/semconv/test/httpconv_test.go.tmpl b/internal/shared/semconv/test/httpconv_test.go.tmpl index d9e96110378..a66f663dd70 100644 --- a/internal/shared/semconv/test/httpconv_test.go.tmpl +++ b/internal/shared/semconv/test/httpconv_test.go.tmpl @@ -78,49 +78,47 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPServer version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.server.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.server.request.body.size", + Description: "Size of HTTP server request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.response.size", - Description: "Measures the size of HTTP response messages.", + Name: "http.server.response.body.size", + Description: "Size of HTTP server response bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.server.duration", - Description: "Measures the duration of inbound HTTP requests.", - Unit: "ms", + 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: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -128,44 +126,46 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPServer version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPServer version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.server.request.body.size", - Description: "Size of HTTP server request bodies.", + Name: "http.server.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.response.body.size", - Description: "Size of HTTP server response bodies.", + Name: "http.server.response.size", + Description: "Measures the size of HTTP response messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.server.request.duration", - Description: "Duration of HTTP server requests.", - Unit: "s", + 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: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -221,7 +221,7 @@ func TestNewServerRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 6) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -416,35 +416,34 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // The OldHTTPClient version - expectedOldScopeMetric := metricdata.ScopeMetrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, Metrics: []metricdata.Metrics{ { - Name: "http.client.request.size", - Description: "Measures the size of HTTP request messages.", + Name: "http.client.request.body.size", + Description: "Size of HTTP client request bodies.", Unit: "By", - Data: metricdata.Sum[int64]{ + Data: metricdata.Histogram[int64]{ Temporality: metricdata.CumulativeTemporality, - IsMonotonic: true, - DataPoints: []metricdata.DataPoint[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, }, { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", + Name: "http.client.request.duration", + Description: "Duration of HTTP client requests.", + Unit: "s", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: oldAttrs, + Attributes: currAttrs, }, }, }, @@ -452,31 +451,32 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.Metrics, []metricdata.Metrics{ + // The OldHTTPClient version + expectedOldScopeMetric := expectedCurrentScopeMetric + expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ { - Name: "http.client.request.body.size", - Description: "Size of HTTP client request bodies.", + Name: "http.client.request.size", + Description: "Measures the size of HTTP request messages.", Unit: "By", - Data: metricdata.Histogram[int64]{ + Data: metricdata.Sum[int64]{ Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, }, { - Name: "http.client.request.duration", - Description: "Duration of HTTP client requests.", - Unit: "s", + Name: "http.client.duration", + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", Data: metricdata.Histogram[float64]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[float64]{ { - Attributes: currAttrs, + Attributes: oldAttrs, }, }, }, @@ -508,9 +508,8 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) - // because of OldHTTPClient require.Len(t, rm.ScopeMetrics[0].Metrics, 2) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -532,7 +531,7 @@ func TestNewClientRecordMetrics(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 4) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -595,9 +594,6 @@ func TestClientRecordResponseSize(t *testing.T) { }, } - // the CurrentHTTPClient version - expectedCurrentScopeMetric := expectedOldScopeMetric - tests := []struct { name string setEnv bool @@ -621,11 +617,7 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Len(t, rm.ScopeMetrics, 1) - - // because of OldHTTPClient - require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + require.Empty(t, rm.ScopeMetrics) }, }, { @@ -647,7 +639,7 @@ func TestClientRecordResponseSize(t *testing.T) { wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { require.Len(t, rm.ScopeMetrics, 1) require.Len(t, rm.ScopeMetrics[0].Metrics, 1) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/internal/shared/semconv/test/v1.20.0_test.go.tmpl b/internal/shared/semconv/test/v1.20.0_test.go.tmpl deleted file mode 100644 index c3630b013de..00000000000 --- a/internal/shared/semconv/test/v1.20.0_test.go.tmpl +++ /dev/null @@ -1,311 +0,0 @@ -// Code created by gotmpl. DO NOT MODIFY. -// source: internal/shared/semconv/test/v1.20.0_test.go.tmpl - -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package test - -import ( - "context" - "fmt" - "net/http" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "{{.pkg}}/internal/semconv" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/instrumentation" - sdkmetric "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" -) - -func TestV120TraceRequest(t *testing.T) { - // Anything but "http" or "http/dup" works. - t.Setenv("OTEL_SEMCONV_STABILITY_OPT_IN", "old") - serv := semconv.NewHTTPServer(nil) - want := func(req testServerReq) []attribute.KeyValue { - return []attribute.KeyValue{ - attribute.String("http.method", "GET"), - attribute.String("http.scheme", "http"), - attribute.String("net.host.name", req.hostname), - attribute.Int("net.host.port", req.serverPort), - attribute.String("net.sock.peer.addr", req.peerAddr), - attribute.Int("net.sock.peer.port", req.peerPort), - attribute.String("user_agent.original", "Go-http-client/1.1"), - attribute.String("http.client_ip", req.clientIP), - attribute.String("net.protocol.version", "1.1"), - attribute.String("http.target", "/"), - } - } - testTraceRequest(t, serv, want) -} - -func TestV120TraceResponse(t *testing.T) { - testCases := []struct { - name string - resp semconv.ResponseTelemetry - want []attribute.KeyValue - }{ - { - name: "empty", - resp: semconv.ResponseTelemetry{}, - want: nil, - }, - { - name: "no errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - WriteBytes: 802, - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.Int("http.response_content_length", 802), - attribute.Int("http.status_code", 200), - }, - }, - { - name: "with errors", - resp: semconv.ResponseTelemetry{ - StatusCode: 200, - ReadBytes: 701, - ReadError: fmt.Errorf("read error"), - WriteBytes: 802, - WriteError: fmt.Errorf("write error"), - }, - want: []attribute.KeyValue{ - attribute.Int("http.request_content_length", 701), - attribute.String("http.read_error", "read error"), - attribute.Int("http.response_content_length", 802), - attribute.String("http.write_error", "write error"), - attribute.Int("http.status_code", 200), - }, - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - got := semconv.OldHTTPServer{}.ResponseTraceAttrs(tt.resp) - assert.ElementsMatch(t, tt.want, got) - }) - } -} - -func TestV120RecordMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - server := semconv.NewHTTPServer(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - server.RecordMetrics(context.Background(), semconv.ServerMetricData{ - ServerName: "stuff", - ResponseSize: 200, - MetricAttributes: semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }, - MetricData: semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, - }) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - 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"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - { - 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: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - -func TestV120ClientRequest(t *testing.T) { - body := strings.NewReader("Hello, world!") - url := "https://example.com:8888/foo/bar?stuff=morestuff" - req, err := http.NewRequest("POST", url, body) - assert.NoError(t, err) - req.Header.Set("User-Agent", "go-test-agent") - - want := []attribute.KeyValue{ - attribute.String("http.method", "POST"), - attribute.String("http.url", url), - attribute.String("net.peer.name", "example.com"), - attribute.Int("net.peer.port", 8888), - attribute.Int("http.request_content_length", body.Len()), - attribute.String("user_agent.original", "go-test-agent"), - } - got := semconv.OldHTTPClient{}.RequestTraceAttrs(req) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientResponse(t *testing.T) { - resp := http.Response{ - StatusCode: 200, - ContentLength: 123, - } - - want := []attribute.KeyValue{ - attribute.Int("http.response_content_length", 123), - attribute.Int("http.status_code", 200), - } - - got := semconv.OldHTTPClient{}.ResponseTraceAttrs(&resp) - assert.ElementsMatch(t, want, got) -} - -func TestV120ClientMetrics(t *testing.T) { - reader := sdkmetric.NewManualReader() - mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - - client := semconv.NewHTTPClient(mp.Meter("test")) - req, err := http.NewRequest("POST", "http://example.com", nil) - assert.NoError(t, err) - - opts := client.MetricOptions(semconv.MetricAttributes{ - Req: req, - StatusCode: 301, - AdditionalAttributes: []attribute.KeyValue{ - attribute.String("key", "value"), - }, - }) - - ctx := context.Background() - - client.RecordResponseSize(ctx, 200, opts) - - client.RecordMetrics(ctx, semconv.MetricData{ - RequestSize: 100, - ElapsedTime: 300, - }, opts) - - rm := metricdata.ResourceMetrics{} - require.NoError(t, reader.Collect(context.Background(), &rm)) - require.Len(t, rm.ScopeMetrics, 1) - require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - - attrs := attribute.NewSet( - attribute.String("http.method", "POST"), - attribute.Int64("http.status_code", 301), - attribute.String("key", "value"), - attribute.String("net.peer.name", "example.com"), - ) - - expectedScopeMetric := metricdata.ScopeMetrics{ - Scope: instrumentation.Scope{ - Name: "test", - }, - Metrics: []metricdata.Metrics{ - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.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: attrs, - }, - }, - }, - }, - { - Name: "http.client.duration", - Description: "Measures the duration of outbound HTTP requests.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, expectedScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -}