diff --git a/CHANGELOG.md b/CHANGELOG.md index edbb73f1415..1fe593d2f16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,13 +26,6 @@ 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` - Improve performance by reducing allocations for http request when using `OTEL_SEMCONV_STABILITY_OPT_IN=http/dup` in the modules below. (#7180) - `go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful` - `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin` @@ -40,9 +33,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `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) - Improve performance by reducing allocations in the gRPC stats handler in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#7186) - Update `http.route` attribute to support `request.Pattern` in `go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux`. (#7108) 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 7d45b2dd00a..f8a2a5afa66 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 "http/dup" to keep getting the old HTTP semantic conventions. +// That can be set to "http/dup" to opt into the new HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -62,9 +62,9 @@ type HTTPServer struct { // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) + attrs := OldHTTPServer{}.RequestTraceAttrs(server, req, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.RequestTraceAttrs(server, req, attrs) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts, attrs) } return attrs } @@ -77,7 +77,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { } } return []attribute.KeyValue{ - CurrentHTTPServer{}.NetworkTransportAttr(network), + OldHTTPServer{}.NetworkTransportAttr(network), } } @@ -85,9 +85,9 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { // // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.ResponseTraceAttrs(resp) + attrs := OldHTTPServer{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -146,19 +146,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - 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) - *recordOpts = append(*recordOpts, o) - s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) - s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) - s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) - *recordOpts = (*recordOpts)[:0] - metricRecordOptionPool.Put(recordOpts) - } - - if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + 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) @@ -169,6 +157,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *addOpts = (*addOpts)[:0] metricAddOptionPool.Put(addOpts) } + + if s.duplicate && 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) + *recordOpts = append(*recordOpts, o) + s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) + s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) + s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) + *recordOpts = (*recordOpts)[:0] + metricRecordOptionPool.Put(recordOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -177,9 +177,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) } return server } @@ -203,9 +203,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) } return client @@ -213,7 +213,7 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { // RequestTraceAttrs returns attributes for an HTTP request made by a client. func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.RequestTraceAttrs(req) + attrs := CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) if c.duplicate { return OldHTTPClient{}.RequestTraceAttrs(req, attrs) } @@ -222,9 +222,9 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.ResponseTraceAttrs(resp) + attrs := OldHTTPClient{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if c.duplicate { - return OldHTTPClient{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -259,17 +259,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - 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, } if c.duplicate { - 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, } @@ -279,17 +279,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.requestBodySize == nil || s.requestDuration == nil { + if s.requestBytesCounter == nil || s.latencyMeasure == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) if s.duplicate { - 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()) } } 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 63fa7576a8e..d5383b20dbe 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,7 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("network.transport", "tcp"), + attribute.String("net.transport", "ip_tcp"), }, }, { diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv.go index 41ee6756faa..6f2fb2db949 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv.go @@ -45,7 +45,7 @@ type CurrentHTTPServer struct{} // // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. -func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { +func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts, attrs []attribute.KeyValue) []attribute.KeyValue { count := 3 // ServerAddress, Method, Scheme var host string @@ -119,7 +119,6 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, o count++ } - attrs := make([]attribute.KeyValue, 0, count) attrs = append(attrs, semconvNew.ServerAddress(host), method, @@ -208,7 +207,7 @@ func (n CurrentHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:rev // // If any of the fields in the ResponseTelemetry are not set the attribute will // be omitted. -func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { +func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry, attrs []attribute.KeyValue) []attribute.KeyValue { var count int if resp.ReadBytes > 0 { @@ -221,25 +220,23 @@ func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribut count++ } - attributes := make([]attribute.KeyValue, 0, count) - if resp.ReadBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPRequestBodySize(int(resp.ReadBytes)), ) } if resp.WriteBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseBodySize(int(resp.WriteBytes)), ) } if resp.StatusCode > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode), ) } - return attributes + return attrs } // Route returns the attribute for the route. @@ -331,7 +328,7 @@ func (n CurrentHTTPServer) MetricAttributes(server string, req *http.Request, st type CurrentHTTPClient struct{} // RequestTraceAttrs returns trace attributes for an HTTP request made by a client. -func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { +func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.request.method @@ -342,7 +339,6 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV - network.protocol.name - network.protocol.version */ - numOfAttributes := 3 // URL, server address, proto, and method. var urlHost string if req.URL != nil { @@ -358,28 +354,8 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } eligiblePort := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort) - if eligiblePort > 0 { - numOfAttributes++ - } - useragent := req.UserAgent() - if useragent != "" { - numOfAttributes++ - } - protoName, protoVersion := netProtocol(req.Proto) - if protoName != "" && protoName != "http" { - numOfAttributes++ - } - if protoVersion != "" { - numOfAttributes++ - } - method, originalMethod := n.method(req.Method) - if originalMethod != (attribute.KeyValue{}) { - numOfAttributes++ - } - - attrs := make([]attribute.KeyValue, 0, numOfAttributes) attrs = append(attrs, method) if originalMethod != (attribute.KeyValue{}) { @@ -413,22 +389,13 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } // ResponseTraceAttrs returns trace attributes for an HTTP response made by a client. -func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { +func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.response.status_code - error.type */ - var count int - if resp.StatusCode > 0 { - count++ - } - - if isErrorStatusCode(resp.StatusCode) { - count++ - } - attrs := make([]attribute.KeyValue, 0, count) if resp.StatusCode > 0 { attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode)) } diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv_test.go index 42853084fa0..059726d07df 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv_test.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconv/httpconv_test.go @@ -305,7 +305,7 @@ func TestRequestTraceAttrs_HTTPRoute(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/path/abc123", nil) req.Pattern = tt.pattern - attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}) + attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}, []attribute.KeyValue{}) var gotRoute string for _, attr := range attrs { @@ -358,7 +358,7 @@ func TestRequestTraceAttrs_ClientIP(t *testing.T) { } var found bool - for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts) { + for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts, []attribute.KeyValue{}) { if attr.Key != "client.address" { continue } 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 51ef50be7d0..f773e31ed82 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,47 +78,49 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPServer version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPServer version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -126,46 +128,44 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // The OldHTTPServer version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, 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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -305,7 +305,7 @@ func TestNewTraceResponse(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp) + got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) }) } @@ -366,7 +366,7 @@ func TestClientRequest(t *testing.T) { attribute.Int("server.port", 8888), attribute.String("network.protocol.version", "1.1"), } - got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req) + got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) assert.ElementsMatch(t, want, got) } @@ -380,7 +380,7 @@ func TestClientResponse(t *testing.T) { } for _, tt := range testcases { - got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp) + got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) } } @@ -416,34 +416,35 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPClient version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPClient version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -451,32 +452,31 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // The OldHTTPClient version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -508,8 +508,9 @@ 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, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -531,7 +532,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -594,6 +595,9 @@ func TestClientRecordResponseSize(t *testing.T) { }, } + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + tests := []struct { name string setEnv bool @@ -617,7 +621,11 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Empty(t, rm.ScopeMetrics) + 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()) }, }, { @@ -639,7 +647,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/test/restful_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/test/restful_test.go index eab2c737ed3..f196177077b 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/test/restful_test.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/test/restful_test.go @@ -95,9 +95,9 @@ func TestChildSpanNames(t *testing.T) { t, spans[0], "/user/{id:[0-9]+}", - attribute.String("server.address", "foobar"), - attribute.Int("http.response.status_code", http.StatusOK), - attribute.String("http.request.method", "GET"), + attribute.String("net.host.name", "foobar"), + attribute.Int("http.status_code", http.StatusOK), + attribute.String("http.method", "GET"), attribute.String("http.route", "/user/{id:[0-9]+}"), ) @@ -110,9 +110,9 @@ func TestChildSpanNames(t *testing.T) { t, spans[1], "/book/{title}", - attribute.String("server.address", "foobar"), - attribute.Int("http.response.status_code", http.StatusOK), - attribute.String("http.request.method", "GET"), + attribute.String("net.host.name", "foobar"), + attribute.Int("http.status_code", http.StatusOK), + attribute.String("http.method", "GET"), attribute.String("http.route", "/book/{title}"), ) } 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 dbb3716e628..0e6bd5c9bc1 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 "http/dup" to keep getting the old HTTP semantic conventions. +// That can be set to "http/dup" to opt into the new HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -62,9 +62,9 @@ type HTTPServer struct { // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) + attrs := OldHTTPServer{}.RequestTraceAttrs(server, req, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.RequestTraceAttrs(server, req, attrs) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts, attrs) } return attrs } @@ -77,7 +77,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { } } return []attribute.KeyValue{ - CurrentHTTPServer{}.NetworkTransportAttr(network), + OldHTTPServer{}.NetworkTransportAttr(network), } } @@ -85,9 +85,9 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { // // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.ResponseTraceAttrs(resp) + attrs := OldHTTPServer{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -146,19 +146,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - 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) - *recordOpts = append(*recordOpts, o) - s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) - s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) - s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) - *recordOpts = (*recordOpts)[:0] - metricRecordOptionPool.Put(recordOpts) - } - - if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + 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) @@ -169,6 +157,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *addOpts = (*addOpts)[:0] metricAddOptionPool.Put(addOpts) } + + if s.duplicate && 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) + *recordOpts = append(*recordOpts, o) + s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) + s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) + s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) + *recordOpts = (*recordOpts)[:0] + metricRecordOptionPool.Put(recordOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -177,9 +177,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) } return server } @@ -203,9 +203,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) } return client @@ -213,7 +213,7 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { // RequestTraceAttrs returns attributes for an HTTP request made by a client. func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.RequestTraceAttrs(req) + attrs := CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) if c.duplicate { return OldHTTPClient{}.RequestTraceAttrs(req, attrs) } @@ -222,9 +222,9 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.ResponseTraceAttrs(resp) + attrs := OldHTTPClient{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if c.duplicate { - return OldHTTPClient{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -259,17 +259,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - 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, } if c.duplicate { - 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, } @@ -279,17 +279,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.requestBodySize == nil || s.requestDuration == nil { + if s.requestBytesCounter == nil || s.latencyMeasure == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) if s.duplicate { - 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()) } } 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 63fa7576a8e..d5383b20dbe 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,7 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("network.transport", "tcp"), + attribute.String("net.transport", "ip_tcp"), }, }, { diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv.go index 2d822972acc..e6b2e7f8b14 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv.go @@ -45,7 +45,7 @@ type CurrentHTTPServer struct{} // // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. -func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { +func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts, attrs []attribute.KeyValue) []attribute.KeyValue { count := 3 // ServerAddress, Method, Scheme var host string @@ -119,7 +119,6 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, o count++ } - attrs := make([]attribute.KeyValue, 0, count) attrs = append(attrs, semconvNew.ServerAddress(host), method, @@ -208,7 +207,7 @@ func (n CurrentHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:rev // // If any of the fields in the ResponseTelemetry are not set the attribute will // be omitted. -func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { +func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry, attrs []attribute.KeyValue) []attribute.KeyValue { var count int if resp.ReadBytes > 0 { @@ -221,25 +220,23 @@ func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribut count++ } - attributes := make([]attribute.KeyValue, 0, count) - if resp.ReadBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPRequestBodySize(int(resp.ReadBytes)), ) } if resp.WriteBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseBodySize(int(resp.WriteBytes)), ) } if resp.StatusCode > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode), ) } - return attributes + return attrs } // Route returns the attribute for the route. @@ -331,7 +328,7 @@ func (n CurrentHTTPServer) MetricAttributes(server string, req *http.Request, st type CurrentHTTPClient struct{} // RequestTraceAttrs returns trace attributes for an HTTP request made by a client. -func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { +func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.request.method @@ -342,7 +339,6 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV - network.protocol.name - network.protocol.version */ - numOfAttributes := 3 // URL, server address, proto, and method. var urlHost string if req.URL != nil { @@ -358,28 +354,8 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } eligiblePort := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort) - if eligiblePort > 0 { - numOfAttributes++ - } - useragent := req.UserAgent() - if useragent != "" { - numOfAttributes++ - } - protoName, protoVersion := netProtocol(req.Proto) - if protoName != "" && protoName != "http" { - numOfAttributes++ - } - if protoVersion != "" { - numOfAttributes++ - } - method, originalMethod := n.method(req.Method) - if originalMethod != (attribute.KeyValue{}) { - numOfAttributes++ - } - - attrs := make([]attribute.KeyValue, 0, numOfAttributes) attrs = append(attrs, method) if originalMethod != (attribute.KeyValue{}) { @@ -413,22 +389,13 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } // ResponseTraceAttrs returns trace attributes for an HTTP response made by a client. -func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { +func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.response.status_code - error.type */ - var count int - if resp.StatusCode > 0 { - count++ - } - - if isErrorStatusCode(resp.StatusCode) { - count++ - } - attrs := make([]attribute.KeyValue, 0, count) if resp.StatusCode > 0 { attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode)) } diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv_test.go index 42853084fa0..059726d07df 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv/httpconv_test.go @@ -305,7 +305,7 @@ func TestRequestTraceAttrs_HTTPRoute(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/path/abc123", nil) req.Pattern = tt.pattern - attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}) + attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}, []attribute.KeyValue{}) var gotRoute string for _, attr := range attrs { @@ -358,7 +358,7 @@ func TestRequestTraceAttrs_ClientIP(t *testing.T) { } var found bool - for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts) { + for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts, []attribute.KeyValue{}) { if attr.Key != "client.address" { continue } 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 f9d5e50bc8e..028673c09d9 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,47 +78,49 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPServer version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPServer version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -126,46 +128,44 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // The OldHTTPServer version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, 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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -305,7 +305,7 @@ func TestNewTraceResponse(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp) + got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) }) } @@ -366,7 +366,7 @@ func TestClientRequest(t *testing.T) { attribute.Int("server.port", 8888), attribute.String("network.protocol.version", "1.1"), } - got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req) + got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) assert.ElementsMatch(t, want, got) } @@ -380,7 +380,7 @@ func TestClientResponse(t *testing.T) { } for _, tt := range testcases { - got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp) + got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) } } @@ -416,34 +416,35 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPClient version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPClient version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -451,32 +452,31 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // The OldHTTPClient version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -508,8 +508,9 @@ 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, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -531,7 +532,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -594,6 +595,9 @@ func TestClientRecordResponseSize(t *testing.T) { }, } + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + tests := []struct { name string setEnv bool @@ -617,7 +621,11 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Empty(t, rm.ScopeMetrics) + 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()) }, }, { @@ -639,7 +647,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, 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 58e9c9b97b6..44c66ff4ab6 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 @@ -21,7 +21,6 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" - "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconv" b3prop "go.opentelemetry.io/contrib/propagators/b3" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -166,9 +165,9 @@ func TestTrace200(t *testing.T) { assert.Equal(t, "GET /user/:id", span.Name()) assert.Equal(t, trace.SpanKindServer, span.SpanKind()) attr := span.Attributes() - assert.Contains(t, attr, attribute.String("server.address", "foobar")) + assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusOK)) - assert.Contains(t, attr, attribute.String("http.request.method", "GET")) + assert.Contains(t, attr, attribute.String("http.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, "GET /server_err", span.Name()) attr := span.Attributes() - assert.Contains(t, attr, attribute.String("server.address", "foobar")) + assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusInternalServerError)) // verify the error events @@ -345,7 +344,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.request.method", "GET")) + assert.Contains(t, attr, attribute.String("http.method", "GET")) assert.Contains(t, attr, attribute.String("http.route", "/user/:id")) } @@ -533,12 +532,12 @@ func TestMetrics(t *testing.T) { assert.Equal(t, otelgin.Version(), sm.Scope.Version) attrs := []attribute.KeyValue{ - attribute.String("http.request.method", "GET"), - attribute.Int64("http.response.status_code", tt.wantStatus), - 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"), + attribute.String("http.method", "GET"), + attribute.String("http.scheme", "http"), + attribute.Int64("http.status_code", tt.wantStatus), + attribute.String("net.protocol.name", "http"), + attribute.String("net.protocol.version", fmt.Sprintf("1.%d", r.ProtoMinor)), + attribute.String("net.host.name", "foobar"), } if tt.wantRouteAttr != "" { attrs = append(attrs, attribute.String("http.route", tt.wantRouteAttr)) @@ -552,37 +551,35 @@ func TestMetrics(t *testing.T) { } metricdatatest.AssertEqual(t, 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]{ - { - Attributes: attribute.NewSet(attrs...), - }, - }, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{ + Attributes: attribute.NewSet(attrs...), Value: 0, + }}, }, }, sm.Metrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) metricdatatest.AssertEqual(t, metricdata.Metrics{ - 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]{ - { - Attributes: attribute.NewSet(attrs...), - }, - }, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{ + Attributes: attribute.NewSet(attrs...), Value: 3, + }}, }, }, sm.Metrics[1], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue(), metricdatatest.IgnoreExemplars()) metricdatatest.AssertEqual(t, metricdata.Metrics{ - 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]{ @@ -595,46 +592,3 @@ func TestMetrics(t *testing.T) { }) } } - -func TestServerWithSemConvStabilityOptIn(t *testing.T) { - tests := []struct { - name string - setEnv bool - wantExistsHTTPMethod bool - }{ - { - "not set", - false, - false, - }, - { - "set to http/dup", - true, - true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.setEnv { - t.Setenv(semconv.OTelSemConvStabilityOptIn, "http/dup") - } - - attrs := semconv.NewHTTPServer(nil). - RequestTraceAttrs("foobar", - httptest.NewRequest("GET", "/user/123", nil), - semconv.RequestTraceAttrsOpts{ - HTTPClientIP: "127.0.0.1", - }) - - var existsHTTPMethod bool - for _, attr := range attrs { - if attr.Key == "http.method" { - existsHTTPMethod = true - break - } - } - assert.Equal(t, tt.wantExistsHTTPMethod, existsHTTPMethod) - }) - } -} 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 a6a26969e5c..88e373a0c54 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 "http/dup" to keep getting the old HTTP semantic conventions. +// That can be set to "http/dup" to opt into the new HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -62,9 +62,9 @@ type HTTPServer struct { // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) + attrs := OldHTTPServer{}.RequestTraceAttrs(server, req, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.RequestTraceAttrs(server, req, attrs) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts, attrs) } return attrs } @@ -77,7 +77,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { } } return []attribute.KeyValue{ - CurrentHTTPServer{}.NetworkTransportAttr(network), + OldHTTPServer{}.NetworkTransportAttr(network), } } @@ -85,9 +85,9 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { // // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.ResponseTraceAttrs(resp) + attrs := OldHTTPServer{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -146,19 +146,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - 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) - *recordOpts = append(*recordOpts, o) - s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) - s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) - s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) - *recordOpts = (*recordOpts)[:0] - metricRecordOptionPool.Put(recordOpts) - } - - if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + 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) @@ -169,6 +157,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *addOpts = (*addOpts)[:0] metricAddOptionPool.Put(addOpts) } + + if s.duplicate && 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) + *recordOpts = append(*recordOpts, o) + s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) + s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) + s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) + *recordOpts = (*recordOpts)[:0] + metricRecordOptionPool.Put(recordOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -177,9 +177,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) } return server } @@ -203,9 +203,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) } return client @@ -213,7 +213,7 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { // RequestTraceAttrs returns attributes for an HTTP request made by a client. func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.RequestTraceAttrs(req) + attrs := CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) if c.duplicate { return OldHTTPClient{}.RequestTraceAttrs(req, attrs) } @@ -222,9 +222,9 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.ResponseTraceAttrs(resp) + attrs := OldHTTPClient{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if c.duplicate { - return OldHTTPClient{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -259,17 +259,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - 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, } if c.duplicate { - 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, } @@ -279,17 +279,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.requestBodySize == nil || s.requestDuration == nil { + if s.requestBytesCounter == nil || s.latencyMeasure == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) if s.duplicate { - 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()) } } 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 63fa7576a8e..d5383b20dbe 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,7 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("network.transport", "tcp"), + attribute.String("net.transport", "ip_tcp"), }, }, { diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv.go index b80a272b461..6f3cd772a19 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv.go @@ -45,7 +45,7 @@ type CurrentHTTPServer struct{} // // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. -func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { +func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts, attrs []attribute.KeyValue) []attribute.KeyValue { count := 3 // ServerAddress, Method, Scheme var host string @@ -119,7 +119,6 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, o count++ } - attrs := make([]attribute.KeyValue, 0, count) attrs = append(attrs, semconvNew.ServerAddress(host), method, @@ -208,7 +207,7 @@ func (n CurrentHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:rev // // If any of the fields in the ResponseTelemetry are not set the attribute will // be omitted. -func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { +func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry, attrs []attribute.KeyValue) []attribute.KeyValue { var count int if resp.ReadBytes > 0 { @@ -221,25 +220,23 @@ func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribut count++ } - attributes := make([]attribute.KeyValue, 0, count) - if resp.ReadBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPRequestBodySize(int(resp.ReadBytes)), ) } if resp.WriteBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseBodySize(int(resp.WriteBytes)), ) } if resp.StatusCode > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode), ) } - return attributes + return attrs } // Route returns the attribute for the route. @@ -331,7 +328,7 @@ func (n CurrentHTTPServer) MetricAttributes(server string, req *http.Request, st type CurrentHTTPClient struct{} // RequestTraceAttrs returns trace attributes for an HTTP request made by a client. -func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { +func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.request.method @@ -342,7 +339,6 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV - network.protocol.name - network.protocol.version */ - numOfAttributes := 3 // URL, server address, proto, and method. var urlHost string if req.URL != nil { @@ -358,28 +354,8 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } eligiblePort := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort) - if eligiblePort > 0 { - numOfAttributes++ - } - useragent := req.UserAgent() - if useragent != "" { - numOfAttributes++ - } - protoName, protoVersion := netProtocol(req.Proto) - if protoName != "" && protoName != "http" { - numOfAttributes++ - } - if protoVersion != "" { - numOfAttributes++ - } - method, originalMethod := n.method(req.Method) - if originalMethod != (attribute.KeyValue{}) { - numOfAttributes++ - } - - attrs := make([]attribute.KeyValue, 0, numOfAttributes) attrs = append(attrs, method) if originalMethod != (attribute.KeyValue{}) { @@ -413,22 +389,13 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } // ResponseTraceAttrs returns trace attributes for an HTTP response made by a client. -func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { +func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.response.status_code - error.type */ - var count int - if resp.StatusCode > 0 { - count++ - } - - if isErrorStatusCode(resp.StatusCode) { - count++ - } - attrs := make([]attribute.KeyValue, 0, count) if resp.StatusCode > 0 { attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode)) } diff --git a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv_test.go b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv_test.go index 42853084fa0..059726d07df 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv_test.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/internal/semconv/httpconv_test.go @@ -305,7 +305,7 @@ func TestRequestTraceAttrs_HTTPRoute(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/path/abc123", nil) req.Pattern = tt.pattern - attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}) + attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}, []attribute.KeyValue{}) var gotRoute string for _, attr := range attrs { @@ -358,7 +358,7 @@ func TestRequestTraceAttrs_ClientIP(t *testing.T) { } var found bool - for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts) { + for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts, []attribute.KeyValue{}) { if attr.Key != "client.address" { continue } 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 7afe8237a48..7f4214b1316 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,47 +78,49 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPServer version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPServer version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -126,46 +128,44 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // The OldHTTPServer version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, 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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -305,7 +305,7 @@ func TestNewTraceResponse(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp) + got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) }) } @@ -366,7 +366,7 @@ func TestClientRequest(t *testing.T) { attribute.Int("server.port", 8888), attribute.String("network.protocol.version", "1.1"), } - got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req) + got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) assert.ElementsMatch(t, want, got) } @@ -380,7 +380,7 @@ func TestClientResponse(t *testing.T) { } for _, tt := range testcases { - got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp) + got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) } } @@ -416,34 +416,35 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPClient version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPClient version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -451,32 +452,31 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // The OldHTTPClient version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -508,8 +508,9 @@ 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, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -531,7 +532,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -594,6 +595,9 @@ func TestClientRecordResponseSize(t *testing.T) { }, } + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + tests := []struct { name string setEnv bool @@ -617,7 +621,11 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Empty(t, rm.ScopeMetrics) + 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()) }, }, { @@ -639,7 +647,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, 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 a837eb45425..0325c369888 100644 --- a/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go +++ b/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go @@ -136,9 +136,9 @@ func TestSDKIntegration(t *testing.T) { assertSpan(t, sr.Ended()[0], tt.expected, trace.SpanKindServer, - attribute.String("server.address", "foobar"), - attribute.Int("http.response.status_code", http.StatusOK), - attribute.String("http.request.method", "GET"), + attribute.String("net.host.name", "foobar"), + attribute.Int("http.status_code", http.StatusOK), + attribute.String("http.method", "GET"), attribute.String("http.route", tt.expected), ) }) @@ -162,9 +162,9 @@ func TestNotFoundIsNotError(t *testing.T) { assertSpan(t, sr.Ended()[0], "/does/not/exist", trace.SpanKindServer, - attribute.String("server.address", "foobar"), - attribute.Int("http.response.status_code", http.StatusNotFound), - attribute.String("http.request.method", "GET"), + attribute.String("net.host.name", "foobar"), + attribute.Int("http.status_code", http.StatusNotFound), + attribute.String("http.method", "GET"), attribute.String("http.route", "/does/not/exist"), ) assert.Equal(t, codes.Unset, sr.Ended()[0].Status().Code) diff --git a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go index 3c9c2385e93..346295759a7 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/httptrace_test.go @@ -16,6 +16,7 @@ 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" ) @@ -35,6 +36,10 @@ 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 { @@ -70,18 +75,17 @@ func TestRoundtrip(t *testing.T) { address := ts.Listener.Addr() hp := strings.Split(address.String(), ":") expectedAttrs = map[attribute.Key]string{ - "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", + 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 := 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 287c3a0b537..5760040591d 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 "http/dup" to keep getting the old HTTP semantic conventions. +// That can be set to "http/dup" to opt into the new HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -62,9 +62,9 @@ type HTTPServer struct { // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) + attrs := OldHTTPServer{}.RequestTraceAttrs(server, req, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.RequestTraceAttrs(server, req, attrs) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts, attrs) } return attrs } @@ -77,7 +77,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { } } return []attribute.KeyValue{ - CurrentHTTPServer{}.NetworkTransportAttr(network), + OldHTTPServer{}.NetworkTransportAttr(network), } } @@ -85,9 +85,9 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { // // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.ResponseTraceAttrs(resp) + attrs := OldHTTPServer{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -146,19 +146,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - 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) - *recordOpts = append(*recordOpts, o) - s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) - s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) - s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) - *recordOpts = (*recordOpts)[:0] - metricRecordOptionPool.Put(recordOpts) - } - - if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + 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) @@ -169,6 +157,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *addOpts = (*addOpts)[:0] metricAddOptionPool.Put(addOpts) } + + if s.duplicate && 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) + *recordOpts = append(*recordOpts, o) + s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) + s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) + s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) + *recordOpts = (*recordOpts)[:0] + metricRecordOptionPool.Put(recordOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -177,9 +177,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) } return server } @@ -203,9 +203,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) } return client @@ -213,7 +213,7 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { // RequestTraceAttrs returns attributes for an HTTP request made by a client. func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.RequestTraceAttrs(req) + attrs := CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) if c.duplicate { return OldHTTPClient{}.RequestTraceAttrs(req, attrs) } @@ -222,9 +222,9 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.ResponseTraceAttrs(resp) + attrs := OldHTTPClient{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if c.duplicate { - return OldHTTPClient{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -259,17 +259,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - 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, } if c.duplicate { - 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, } @@ -279,17 +279,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.requestBodySize == nil || s.requestDuration == nil { + if s.requestBytesCounter == nil || s.latencyMeasure == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) if s.duplicate { - 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()) } } 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 63fa7576a8e..d5383b20dbe 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/env_test.go @@ -68,7 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("network.transport", "tcp"), + attribute.String("net.transport", "ip_tcp"), }, }, { diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv.go index e016f0176fc..1a57633948c 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv.go @@ -45,7 +45,7 @@ type CurrentHTTPServer struct{} // // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. -func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { +func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts, attrs []attribute.KeyValue) []attribute.KeyValue { count := 3 // ServerAddress, Method, Scheme var host string @@ -119,7 +119,6 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, o count++ } - attrs := make([]attribute.KeyValue, 0, count) attrs = append(attrs, semconvNew.ServerAddress(host), method, @@ -208,7 +207,7 @@ func (n CurrentHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:rev // // If any of the fields in the ResponseTelemetry are not set the attribute will // be omitted. -func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { +func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry, attrs []attribute.KeyValue) []attribute.KeyValue { var count int if resp.ReadBytes > 0 { @@ -221,25 +220,23 @@ func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribut count++ } - attributes := make([]attribute.KeyValue, 0, count) - if resp.ReadBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPRequestBodySize(int(resp.ReadBytes)), ) } if resp.WriteBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseBodySize(int(resp.WriteBytes)), ) } if resp.StatusCode > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode), ) } - return attributes + return attrs } // Route returns the attribute for the route. @@ -331,7 +328,7 @@ func (n CurrentHTTPServer) MetricAttributes(server string, req *http.Request, st type CurrentHTTPClient struct{} // RequestTraceAttrs returns trace attributes for an HTTP request made by a client. -func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { +func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.request.method @@ -342,7 +339,6 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV - network.protocol.name - network.protocol.version */ - numOfAttributes := 3 // URL, server address, proto, and method. var urlHost string if req.URL != nil { @@ -358,28 +354,8 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } eligiblePort := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort) - if eligiblePort > 0 { - numOfAttributes++ - } - useragent := req.UserAgent() - if useragent != "" { - numOfAttributes++ - } - protoName, protoVersion := netProtocol(req.Proto) - if protoName != "" && protoName != "http" { - numOfAttributes++ - } - if protoVersion != "" { - numOfAttributes++ - } - method, originalMethod := n.method(req.Method) - if originalMethod != (attribute.KeyValue{}) { - numOfAttributes++ - } - - attrs := make([]attribute.KeyValue, 0, numOfAttributes) attrs = append(attrs, method) if originalMethod != (attribute.KeyValue{}) { @@ -413,22 +389,13 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } // ResponseTraceAttrs returns trace attributes for an HTTP response made by a client. -func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { +func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.response.status_code - error.type */ - var count int - if resp.StatusCode > 0 { - count++ - } - - if isErrorStatusCode(resp.StatusCode) { - count++ - } - attrs := make([]attribute.KeyValue, 0, count) if resp.StatusCode > 0 { attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode)) } diff --git a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv_test.go b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv_test.go index 42853084fa0..059726d07df 100644 --- a/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv_test.go +++ b/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/httpconv_test.go @@ -305,7 +305,7 @@ func TestRequestTraceAttrs_HTTPRoute(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/path/abc123", nil) req.Pattern = tt.pattern - attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}) + attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}, []attribute.KeyValue{}) var gotRoute string for _, attr := range attrs { @@ -358,7 +358,7 @@ func TestRequestTraceAttrs_ClientIP(t *testing.T) { } var found bool - for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts) { + for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts, []attribute.KeyValue{}) { if attr.Key != "client.address" { continue } 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 fb7051b4ad1..afa6bfcaca3 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,47 +78,49 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPServer version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPServer version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -126,46 +128,44 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // The OldHTTPServer version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, 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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -305,7 +305,7 @@ func TestNewTraceResponse(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp) + got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) }) } @@ -366,7 +366,7 @@ func TestClientRequest(t *testing.T) { attribute.Int("server.port", 8888), attribute.String("network.protocol.version", "1.1"), } - got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req) + got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) assert.ElementsMatch(t, want, got) } @@ -380,7 +380,7 @@ func TestClientResponse(t *testing.T) { } for _, tt := range testcases { - got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp) + got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) } } @@ -416,34 +416,35 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPClient version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPClient version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -451,32 +452,31 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // The OldHTTPClient version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -508,8 +508,9 @@ 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, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -531,7 +532,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -594,6 +595,9 @@ func TestClientRecordResponseSize(t *testing.T) { }, } + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + tests := []struct { name string setEnv bool @@ -617,7 +621,11 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Empty(t, rm.ScopeMetrics) + 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()) }, }, { @@ -639,7 +647,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } diff --git a/instrumentation/net/http/otelhttp/handler_test.go b/instrumentation/net/http/otelhttp/handler_test.go index d05f7624514..891fab3138c 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( - 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"), + semconv.NetHostName(r.Host), + semconv.HTTPSchemeHTTP, + semconv.NetProtocolName("http"), + semconv.NetProtocolVersion(fmt.Sprintf("1.%d", r.ProtoMinor)), + semconv.HTTPMethod("GET"), attribute.String("test", "attribute"), + semconv.HTTPStatusCode(200), ) assertScopeMetrics(t, rm.ScopeMetrics[0], attrs) @@ -174,6 +174,8 @@ func TestHandlerBasics(t *testing.T) { } func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribute.Set) { + t.Helper() + assert.Equal(t, instrumentation.Scope{ Name: "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp", Version: Version(), @@ -188,42 +190,32 @@ func assertScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribut }, 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]{ + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 0}}, Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ - { - Attributes: attrs, - }, - }, + IsMonotonic: true, }, }, { - 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]{ + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 11}}, Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ - { - Attributes: attrs, - }, - }, + IsMonotonic: true, }, }, { - 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]{ + DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}}, Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, }, }, }, @@ -246,7 +238,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { w.WriteHeader(http.StatusOK) }, attributes: []attribute.KeyValue{ - attribute.Int("http.response.status_code", http.StatusOK), + attribute.Int("http.status_code", http.StatusOK), }, }, { @@ -255,7 +247,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { w.WriteHeader(http.StatusBadRequest) }, attributes: []attribute.KeyValue{ - attribute.Int("http.response.status_code", http.StatusBadRequest), + attribute.Int("http.status_code", http.StatusBadRequest), }, }, { @@ -263,7 +255,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { handler: func(w http.ResponseWriter, r *http.Request) { }, attributes: []attribute.KeyValue{ - attribute.Int("http.response.status_code", http.StatusOK), + attribute.Int("http.status_code", http.StatusOK), }, }, { @@ -273,7 +265,7 @@ func TestHandlerEmittedAttributes(t *testing.T) { w.WriteHeader(http.StatusOK) }, attributes: []attribute.KeyValue{ - attribute.Int("http.response.status_code", http.StatusInternalServerError), + attribute.Int("http.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 93862651955..bd7bdb8cbce 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 "http/dup" to keep getting the old HTTP semantic conventions. +// That can be set to "http/dup" to opt into the new HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -62,9 +62,9 @@ type HTTPServer struct { // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) + attrs := OldHTTPServer{}.RequestTraceAttrs(server, req, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.RequestTraceAttrs(server, req, attrs) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts, attrs) } return attrs } @@ -77,7 +77,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { } } return []attribute.KeyValue{ - CurrentHTTPServer{}.NetworkTransportAttr(network), + OldHTTPServer{}.NetworkTransportAttr(network), } } @@ -85,9 +85,9 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { // // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.ResponseTraceAttrs(resp) + attrs := OldHTTPServer{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -146,19 +146,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - 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) - *recordOpts = append(*recordOpts, o) - s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) - s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) - s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) - *recordOpts = (*recordOpts)[:0] - metricRecordOptionPool.Put(recordOpts) - } - - if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + 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) @@ -169,6 +157,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *addOpts = (*addOpts)[:0] metricAddOptionPool.Put(addOpts) } + + if s.duplicate && 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) + *recordOpts = append(*recordOpts, o) + s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) + s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) + s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) + *recordOpts = (*recordOpts)[:0] + metricRecordOptionPool.Put(recordOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -177,9 +177,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) } return server } @@ -203,9 +203,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) } return client @@ -213,7 +213,7 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { // RequestTraceAttrs returns attributes for an HTTP request made by a client. func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.RequestTraceAttrs(req) + attrs := CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) if c.duplicate { return OldHTTPClient{}.RequestTraceAttrs(req, attrs) } @@ -222,9 +222,9 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.ResponseTraceAttrs(resp) + attrs := OldHTTPClient{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if c.duplicate { - return OldHTTPClient{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -259,17 +259,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - 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, } if c.duplicate { - 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, } @@ -279,17 +279,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.requestBodySize == nil || s.requestDuration == nil { + if s.requestBytesCounter == nil || s.latencyMeasure == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) if s.duplicate { - 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()) } } diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env_test.go b/instrumentation/net/http/otelhttp/internal/semconv/env_test.go index 63fa7576a8e..d5383b20dbe 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env_test.go @@ -68,7 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("network.transport", "tcp"), + attribute.String("net.transport", "ip_tcp"), }, }, { diff --git a/instrumentation/net/http/otelhttp/internal/semconv/httpconv.go b/instrumentation/net/http/otelhttp/internal/semconv/httpconv.go index 53976b0d5a6..2318959b810 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/httpconv.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/httpconv.go @@ -45,7 +45,7 @@ type CurrentHTTPServer struct{} // // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. -func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { +func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts, attrs []attribute.KeyValue) []attribute.KeyValue { count := 3 // ServerAddress, Method, Scheme var host string @@ -119,7 +119,6 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, o count++ } - attrs := make([]attribute.KeyValue, 0, count) attrs = append(attrs, semconvNew.ServerAddress(host), method, @@ -208,7 +207,7 @@ func (n CurrentHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:rev // // If any of the fields in the ResponseTelemetry are not set the attribute will // be omitted. -func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { +func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry, attrs []attribute.KeyValue) []attribute.KeyValue { var count int if resp.ReadBytes > 0 { @@ -221,25 +220,23 @@ func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribut count++ } - attributes := make([]attribute.KeyValue, 0, count) - if resp.ReadBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPRequestBodySize(int(resp.ReadBytes)), ) } if resp.WriteBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseBodySize(int(resp.WriteBytes)), ) } if resp.StatusCode > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode), ) } - return attributes + return attrs } // Route returns the attribute for the route. @@ -331,7 +328,7 @@ func (n CurrentHTTPServer) MetricAttributes(server string, req *http.Request, st type CurrentHTTPClient struct{} // RequestTraceAttrs returns trace attributes for an HTTP request made by a client. -func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { +func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.request.method @@ -342,7 +339,6 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV - network.protocol.name - network.protocol.version */ - numOfAttributes := 3 // URL, server address, proto, and method. var urlHost string if req.URL != nil { @@ -358,28 +354,8 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } eligiblePort := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort) - if eligiblePort > 0 { - numOfAttributes++ - } - useragent := req.UserAgent() - if useragent != "" { - numOfAttributes++ - } - protoName, protoVersion := netProtocol(req.Proto) - if protoName != "" && protoName != "http" { - numOfAttributes++ - } - if protoVersion != "" { - numOfAttributes++ - } - method, originalMethod := n.method(req.Method) - if originalMethod != (attribute.KeyValue{}) { - numOfAttributes++ - } - - attrs := make([]attribute.KeyValue, 0, numOfAttributes) attrs = append(attrs, method) if originalMethod != (attribute.KeyValue{}) { @@ -413,22 +389,13 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } // ResponseTraceAttrs returns trace attributes for an HTTP response made by a client. -func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { +func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.response.status_code - error.type */ - var count int - if resp.StatusCode > 0 { - count++ - } - - if isErrorStatusCode(resp.StatusCode) { - count++ - } - attrs := make([]attribute.KeyValue, 0, count) if resp.StatusCode > 0 { attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode)) } diff --git a/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go b/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go index 42853084fa0..059726d07df 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/httpconv_test.go @@ -305,7 +305,7 @@ func TestRequestTraceAttrs_HTTPRoute(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/path/abc123", nil) req.Pattern = tt.pattern - attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}) + attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}, []attribute.KeyValue{}) var gotRoute string for _, attr := range attrs { @@ -358,7 +358,7 @@ func TestRequestTraceAttrs_ClientIP(t *testing.T) { } var found bool - for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts) { + for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts, []attribute.KeyValue{}) { if attr.Key != "client.address" { continue } 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 28b83404ee3..fdfc1caed3d 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/test/httpconv_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/test/httpconv_test.go @@ -78,47 +78,49 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPServer version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPServer version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -126,46 +128,44 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // The OldHTTPServer version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, 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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -305,7 +305,7 @@ func TestNewTraceResponse(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp) + got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) }) } @@ -366,7 +366,7 @@ func TestClientRequest(t *testing.T) { attribute.Int("server.port", 8888), attribute.String("network.protocol.version", "1.1"), } - got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req) + got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) assert.ElementsMatch(t, want, got) } @@ -380,7 +380,7 @@ func TestClientResponse(t *testing.T) { } for _, tt := range testcases { - got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp) + got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) } } @@ -416,34 +416,35 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPClient version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPClient version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -451,32 +452,31 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // The OldHTTPClient version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -508,8 +508,9 @@ 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, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -531,7 +532,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -594,6 +595,9 @@ func TestClientRecordResponseSize(t *testing.T) { }, } + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + tests := []struct { name string setEnv bool @@ -617,7 +621,11 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Empty(t, rm.ScopeMetrics) + 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()) }, }, { @@ -639,7 +647,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, 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 0b83ef144e2..058f7f672e8 100644 --- a/instrumentation/net/http/otelhttp/transport_test.go +++ b/instrumentation/net/http/otelhttp/transport_test.go @@ -706,15 +706,12 @@ func TestTransportMetrics(t *testing.T) { require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) attrs := attribute.NewSet( - 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"), + attribute.String("http.method", "GET"), + attribute.Int("http.status_code", 200), + attribute.String("net.peer.name", host), + attribute.Int("net.peer.port", port), ) - assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 13) }) t.Run("make http request and buffer response", func(t *testing.T) { @@ -778,15 +775,12 @@ func TestTransportMetrics(t *testing.T) { require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) attrs := attribute.NewSet( - 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"), + attribute.String("http.method", "GET"), + attribute.Int("http.status_code", 200), + attribute.String("net.peer.name", host), + attribute.Int("net.peer.port", port), ) - assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 13) }) t.Run("make http request and close body before reading completely", func(t *testing.T) { @@ -845,25 +839,24 @@ func TestTransportMetrics(t *testing.T) { require.NoError(t, err) require.Len(t, rm.ScopeMetrics, 1) attrs := attribute.NewSet( - 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"), + attribute.String("http.method", "GET"), + attribute.Int("http.status_code", 200), + attribute.String("net.peer.name", host), + attribute.Int("net.peer.port", port), ) - assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs) + assertClientScopeMetrics(t, rm.ScopeMetrics[0], attrs, 10) }) } -func assertClientScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribute.Set) { +func assertClientScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs attribute.Set, rxBytes int64) { + t.Helper() + assert.Equal(t, instrumentation.Scope{ Name: "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp", Version: Version(), }, sm.Scope) - require.Len(t, sm.Metrics, 2) + require.Len(t, sm.Metrics, 3) want := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ @@ -872,29 +865,32 @@ func assertClientScopeMetrics(t *testing.T, sm metricdata.ScopeMetrics, attrs at }, 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]{ + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 4}}, Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[int64]{ - { - Attributes: attrs, - }, - }, + IsMonotonic: true, }, }, { - Name: "http.client.request.duration", - Description: "Duration of HTTP client requests.", - Unit: "s", + Name: "http.client.response.size", + Description: "Measures the size of HTTP response messages.", + Unit: "By", + Data: metricdata.Sum[int64]{ + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: rxBytes}}, + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + }, + }, + { + Name: "http.client.duration", + Description: "Measures the duration of outbound HTTP requests.", + Unit: "ms", Data: metricdata.Histogram[float64]{ + DataPoints: []metricdata.HistogramDataPoint[float64]{{Attributes: attrs}}, Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attrs, - }, - }, }, }, }, @@ -1011,7 +1007,7 @@ func TestDefaultAttributesHandling(t *testing.T) { err = reader.Collect(ctx, &rm) assert.NoError(t, err) - assert.Len(t, rm.ScopeMetrics[0].Metrics, 2) + assert.Len(t, rm.ScopeMetrics[0].Metrics, 3) 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 e93fb784564..301e9ab8561 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 "http/dup" to keep getting the old HTTP semantic conventions. +// That can be set to "http/dup" to opt into the new HTTP semantic conventions. const OTelSemConvStabilityOptIn = "OTEL_SEMCONV_STABILITY_OPT_IN" type ResponseTelemetry struct { @@ -62,9 +62,9 @@ type HTTPServer struct { // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts) + attrs := OldHTTPServer{}.RequestTraceAttrs(server, req, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.RequestTraceAttrs(server, req, attrs) + return CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts, attrs) } return attrs } @@ -77,7 +77,7 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { } } return []attribute.KeyValue{ - CurrentHTTPServer{}.NetworkTransportAttr(network), + OldHTTPServer{}.NetworkTransportAttr(network), } } @@ -85,9 +85,9 @@ func (s HTTPServer) NetworkTransportAttr(network string) []attribute.KeyValue { // // If any of the fields in the ResponseTelemetry are not set the attribute will be omitted. func (s HTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { - attrs := CurrentHTTPServer{}.ResponseTraceAttrs(resp) + attrs := OldHTTPServer{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if s.duplicate { - return OldHTTPServer{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPServer{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -146,19 +146,7 @@ var ( ) func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { - 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) - *recordOpts = append(*recordOpts, o) - s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) - s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) - s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) - *recordOpts = (*recordOpts)[:0] - metricRecordOptionPool.Put(recordOpts) - } - - if s.duplicate && s.requestBytesCounter != nil && s.responseBytesCounter != nil && s.serverLatencyMeasure != nil { + 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) @@ -169,6 +157,18 @@ func (s HTTPServer) RecordMetrics(ctx context.Context, md ServerMetricData) { *addOpts = (*addOpts)[:0] metricAddOptionPool.Put(addOpts) } + + if s.duplicate && 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) + *recordOpts = append(*recordOpts, o) + s.requestBodySizeHistogram.Record(ctx, md.RequestSize, *recordOpts...) + s.responseBodySizeHistogram.Record(ctx, md.ResponseSize, *recordOpts...) + s.requestDurationHistogram.Record(ctx, md.ElapsedTime/1000.0, o) + *recordOpts = (*recordOpts)[:0] + metricRecordOptionPool.Put(recordOpts) + } } func NewHTTPServer(meter metric.Meter) HTTPServer { @@ -177,9 +177,9 @@ func NewHTTPServer(meter metric.Meter) HTTPServer { server := HTTPServer{ duplicate: duplicate, } - server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) + server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) if duplicate { - server.requestBytesCounter, server.responseBytesCounter, server.serverLatencyMeasure = OldHTTPServer{}.createMeasures(meter) + server.requestBodySizeHistogram, server.responseBodySizeHistogram, server.requestDurationHistogram = CurrentHTTPServer{}.createMeasures(meter) } return server } @@ -203,9 +203,9 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { client := HTTPClient{ duplicate: duplicate, } - client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) + client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) if duplicate { - client.requestBytesCounter, client.responseBytesCounter, client.latencyMeasure = OldHTTPClient{}.createMeasures(meter) + client.requestBodySize, client.requestDuration = CurrentHTTPClient{}.createMeasures(meter) } return client @@ -213,7 +213,7 @@ func NewHTTPClient(meter metric.Meter) HTTPClient { // RequestTraceAttrs returns attributes for an HTTP request made by a client. func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.RequestTraceAttrs(req) + attrs := CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) if c.duplicate { return OldHTTPClient{}.RequestTraceAttrs(req, attrs) } @@ -222,9 +222,9 @@ func (c HTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { // ResponseTraceAttrs returns metric attributes for an HTTP request made by a client. func (c HTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { - attrs := CurrentHTTPClient{}.ResponseTraceAttrs(resp) + attrs := OldHTTPClient{}.ResponseTraceAttrs(resp, []attribute.KeyValue{}) if c.duplicate { - return OldHTTPClient{}.ResponseTraceAttrs(resp, attrs) + return CurrentHTTPClient{}.ResponseTraceAttrs(resp, attrs) } return attrs } @@ -259,17 +259,17 @@ func (o MetricOpts) AddOptions() metric.AddOption { func (c HTTPClient) MetricOptions(ma MetricAttributes) map[string]MetricOpts { opts := map[string]MetricOpts{} - 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, } if c.duplicate { - 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, } @@ -279,17 +279,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.requestBodySize == nil || s.requestDuration == nil { + if s.requestBytesCounter == nil || s.latencyMeasure == nil { // This will happen if an HTTPClient{} is used instead of NewHTTPClient(). return } - s.requestBodySize.Record(ctx, md.RequestSize, opts["new"].MeasurementOption()) - s.requestDuration.Record(ctx, md.ElapsedTime/1000, opts["new"].MeasurementOption()) + s.requestBytesCounter.Add(ctx, md.RequestSize, opts["old"].AddOptions()) + s.latencyMeasure.Record(ctx, md.ElapsedTime, opts["old"].MeasurementOption()) if s.duplicate { - 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()) } } diff --git a/internal/shared/semconv/env_test.go.tmpl b/internal/shared/semconv/env_test.go.tmpl index 63fa7576a8e..d5383b20dbe 100644 --- a/internal/shared/semconv/env_test.go.tmpl +++ b/internal/shared/semconv/env_test.go.tmpl @@ -68,7 +68,7 @@ func TestServerNetworkTransportAttr(t *testing.T) { network: "tcp", wantAttributes: []attribute.KeyValue{ - attribute.String("network.transport", "tcp"), + attribute.String("net.transport", "ip_tcp"), }, }, { diff --git a/internal/shared/semconv/httpconv.go.tmpl b/internal/shared/semconv/httpconv.go.tmpl index 9f4ffe05077..9ca544d3d15 100644 --- a/internal/shared/semconv/httpconv.go.tmpl +++ b/internal/shared/semconv/httpconv.go.tmpl @@ -45,7 +45,7 @@ type CurrentHTTPServer struct{} // // If the primary server name is not known, server should be an empty string. // The req Host will be used to determine the server instead. -func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue { +func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts, attrs []attribute.KeyValue) []attribute.KeyValue { count := 3 // ServerAddress, Method, Scheme var host string @@ -119,7 +119,6 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, o count++ } - attrs := make([]attribute.KeyValue, 0, count) attrs = append(attrs, semconvNew.ServerAddress(host), method, @@ -208,7 +207,7 @@ func (n CurrentHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:rev // // If any of the fields in the ResponseTelemetry are not set the attribute will // be omitted. -func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue { +func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry, attrs []attribute.KeyValue) []attribute.KeyValue { var count int if resp.ReadBytes > 0 { @@ -221,25 +220,23 @@ func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribut count++ } - attributes := make([]attribute.KeyValue, 0, count) - if resp.ReadBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPRequestBodySize(int(resp.ReadBytes)), ) } if resp.WriteBytes > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseBodySize(int(resp.WriteBytes)), ) } if resp.StatusCode > 0 { - attributes = append(attributes, + attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode), ) } - return attributes + return attrs } // Route returns the attribute for the route. @@ -331,7 +328,7 @@ func (n CurrentHTTPServer) MetricAttributes(server string, req *http.Request, st type CurrentHTTPClient struct{} // RequestTraceAttrs returns trace attributes for an HTTP request made by a client. -func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyValue { +func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.request.method @@ -342,7 +339,6 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV - network.protocol.name - network.protocol.version */ - numOfAttributes := 3 // URL, server address, proto, and method. var urlHost string if req.URL != nil { @@ -358,28 +354,8 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } eligiblePort := requiredHTTPPort(req.URL != nil && req.URL.Scheme == "https", requestPort) - if eligiblePort > 0 { - numOfAttributes++ - } - useragent := req.UserAgent() - if useragent != "" { - numOfAttributes++ - } - protoName, protoVersion := netProtocol(req.Proto) - if protoName != "" && protoName != "http" { - numOfAttributes++ - } - if protoVersion != "" { - numOfAttributes++ - } - method, originalMethod := n.method(req.Method) - if originalMethod != (attribute.KeyValue{}) { - numOfAttributes++ - } - - attrs := make([]attribute.KeyValue, 0, numOfAttributes) attrs = append(attrs, method) if originalMethod != (attribute.KeyValue{}) { @@ -413,22 +389,13 @@ func (n CurrentHTTPClient) RequestTraceAttrs(req *http.Request) []attribute.KeyV } // ResponseTraceAttrs returns trace attributes for an HTTP response made by a client. -func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response) []attribute.KeyValue { +func (n CurrentHTTPClient) ResponseTraceAttrs(resp *http.Response, attrs []attribute.KeyValue) []attribute.KeyValue { /* below attributes are returned: - http.response.status_code - error.type */ - var count int - if resp.StatusCode > 0 { - count++ - } - - if isErrorStatusCode(resp.StatusCode) { - count++ - } - attrs := make([]attribute.KeyValue, 0, count) if resp.StatusCode > 0 { attrs = append(attrs, semconvNew.HTTPResponseStatusCode(resp.StatusCode)) } diff --git a/internal/shared/semconv/httpconv_test.go.tmpl b/internal/shared/semconv/httpconv_test.go.tmpl index 42853084fa0..059726d07df 100644 --- a/internal/shared/semconv/httpconv_test.go.tmpl +++ b/internal/shared/semconv/httpconv_test.go.tmpl @@ -305,7 +305,7 @@ func TestRequestTraceAttrs_HTTPRoute(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/path/abc123", nil) req.Pattern = tt.pattern - attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}) + attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{}, []attribute.KeyValue{}) var gotRoute string for _, attr := range attrs { @@ -358,7 +358,7 @@ func TestRequestTraceAttrs_ClientIP(t *testing.T) { } var found bool - for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts) { + for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts, []attribute.KeyValue{}) { if attr.Key != "client.address" { continue } diff --git a/internal/shared/semconv/test/httpconv_test.go.tmpl b/internal/shared/semconv/test/httpconv_test.go.tmpl index 20a78a12126..98a7ae62136 100644 --- a/internal/shared/semconv/test/httpconv_test.go.tmpl +++ b/internal/shared/semconv/test/httpconv_test.go.tmpl @@ -78,47 +78,49 @@ func TestNewServerRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPServer version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPServer version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -126,46 +128,44 @@ func TestNewServerRecordMetrics(t *testing.T) { }, } - // The OldHTTPServer version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPServer version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -199,7 +199,7 @@ func TestNewServerRecordMetrics(t *testing.T) { // because of OldHTTPServer require.Len(t, rm.ScopeMetrics[0].Metrics, 3) - metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, 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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -305,7 +305,7 @@ func TestNewTraceResponse(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp) + got := semconv.CurrentHTTPServer{}.ResponseTraceAttrs(tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) }) } @@ -366,7 +366,7 @@ func TestClientRequest(t *testing.T) { attribute.Int("server.port", 8888), attribute.String("network.protocol.version", "1.1"), } - got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req) + got := semconv.CurrentHTTPClient{}.RequestTraceAttrs(req, []attribute.KeyValue{}) assert.ElementsMatch(t, want, got) } @@ -380,7 +380,7 @@ func TestClientResponse(t *testing.T) { } for _, tt := range testcases { - got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp) + got := semconv.CurrentHTTPClient{}.ResponseTraceAttrs(&tt.resp, []attribute.KeyValue{}) assert.ElementsMatch(t, tt.want, got) } } @@ -416,34 +416,35 @@ func TestNewClientRecordMetrics(t *testing.T) { attribute.String("url.scheme", "http"), ) - // the CurrentHTTPClient version - expectedCurrentScopeMetric := metricdata.ScopeMetrics{ + // The OldHTTPClient version + expectedOldScopeMetric := metricdata.ScopeMetrics{ Scope: instrumentation.Scope{ Name: "test", }, 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, }, }, }, @@ -451,32 +452,31 @@ func TestNewClientRecordMetrics(t *testing.T) { }, } - // The OldHTTPClient version - expectedOldScopeMetric := expectedCurrentScopeMetric - expectedOldScopeMetric.Metrics = append(expectedOldScopeMetric.Metrics, []metricdata.Metrics{ + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + expectedCurrentScopeMetric.Metrics = append(expectedCurrentScopeMetric.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, }, }, }, @@ -508,8 +508,9 @@ 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, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, { @@ -531,7 +532,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, } @@ -594,6 +595,9 @@ func TestClientRecordResponseSize(t *testing.T) { }, } + // the CurrentHTTPClient version + expectedCurrentScopeMetric := expectedOldScopeMetric + tests := []struct { name string setEnv bool @@ -617,7 +621,11 @@ func TestClientRecordResponseSize(t *testing.T) { return semconv.NewHTTPClient(mp.Meter("test")) }, wantFunc: func(t *testing.T, rm metricdata.ResourceMetrics) { - require.Empty(t, rm.ScopeMetrics) + 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()) }, }, { @@ -639,7 +647,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, expectedOldScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) + metricdatatest.AssertEqual(t, expectedCurrentScopeMetric, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) }, }, }