From d0f8cc4e80b73b4f75d5a37efede4a58596886d3 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Tue, 12 Mar 2024 12:28:13 -0500 Subject: [PATCH] Address PR feedback --- .../net/http/otelhttp/internal/semconv/README.md | 10 +++++----- .../net/http/otelhttp/internal/semconv/dup.go | 4 ++-- .../net/http/otelhttp/internal/semconv/dup_test.go | 1 - .../net/http/otelhttp/internal/semconv/env.go | 2 +- .../net/http/otelhttp/internal/semconv/v1.24.0.go | 7 ++----- 5 files changed, 10 insertions(+), 14 deletions(-) diff --git a/instrumentation/net/http/otelhttp/internal/semconv/README.md b/instrumentation/net/http/otelhttp/internal/semconv/README.md index dbdd39f5eec..efb9741ddb4 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/README.md +++ b/instrumentation/net/http/otelhttp/internal/semconv/README.md @@ -1,14 +1,14 @@ # Migration Plan -- Current release - v0.48.0 +- Current release - v0.49.0 - Support only for v1.20.0 semantic conventions -- v0.49.0 +- v0.50.0 - Add support for v1.20.0, v1.24.0, and both semantic conventions - Default to v1.20.0 - Warn users when using v1.20.0 -- v0.50.0 +- v0.51.0 - Default to v1.24.0 - Warn users when using v1.20.0 or both semantic conventions -- v0.51.0 +- v0.52.0 - Remove support for v1.20.0 - Remove support for both semantic conventions - All users will be using v1.24.0 @@ -19,4 +19,4 @@ The goal is to allow users to upgrade versions of the `otelhttp` package and ena When the user first upgrades to v0.49 they will receive a warning that they are using v1.20.0 semantic conventions. This is an indication that action needs to be taken or their dashboards and alerts will be affected. They can then set the environment variable `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE` to `http/dup` and receive both the original v1.20.0 attributes and the new v1.24.0 attributes. This will allow them to update their dashboards and alerts to use the new attributes and validate they are not missing any data. - +Once the user has completed the migration, they can then set `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE` to `http` to complete the migration to the stable http metric names. diff --git a/instrumentation/net/http/otelhttp/internal/semconv/dup.go b/instrumentation/net/http/otelhttp/internal/semconv/dup.go index 25850eca89e..44e5087452f 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/dup.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/dup.go @@ -17,6 +17,7 @@ package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/ import ( "io" "net/http" + "slices" "strings" "go.opentelemetry.io/otel/attribute" @@ -116,8 +117,7 @@ func (d dupHTTPServer) TraceRequest(server string, req *http.Request) []attribut i += 2 } - // // TODO: When we drop go1.20 support use slices.clip(). - return attrs[:i:i] + return slices.Clip(attrs[:i]) } func (d dupHTTPServer) method(method string, attrs []attribute.KeyValue) int { diff --git a/instrumentation/net/http/otelhttp/internal/semconv/dup_test.go b/instrumentation/net/http/otelhttp/internal/semconv/dup_test.go index d1bfc1a8a14..6ced89f11f6 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/dup_test.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/dup_test.go @@ -97,7 +97,6 @@ func TestDupMethod(t *testing.T) { } func TestDupTraceResponse(t *testing.T) { - // Anything but "http" or "http/dup" works t.Setenv("OTEL_HTTP_CLIENT_COMPATIBILITY_MODE", "http/dup") serv := NewHTTPServer() want := []attribute.KeyValue{ diff --git a/instrumentation/net/http/otelhttp/internal/semconv/env.go b/instrumentation/net/http/otelhttp/internal/semconv/env.go index 98c5020b145..33a3aeaf190 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/env.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/env.go @@ -77,7 +77,7 @@ func NewHTTPServer() HTTPServer { return dupHTTPServer{} default: warnOnce.Do(func() { - otel.Handle(errors.New("deprecated: old semantic conventions are being used. Use the environment variable OTEL_HTTP_CLIENT_COMPATIBILITY_MODE to opt into the new conventions. This will be removed in a future release")) + otel.Handle(errors.New("deprecated: old semantic conventions are being used. Use the environment variable OTEL_HTTP_CLIENT_COMPATIBILITY_MODE to opt into the new conventions. Setting it to `http/dup` will provide combined old-and-new attributes. Setting it to `http` will switch directly to the new attributes. This will be removed in a future release")) }) return oldHTTPServer{} } diff --git a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go index 073d207e5f2..09956280d5a 100644 --- a/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go +++ b/instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go @@ -16,6 +16,7 @@ package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/ import ( "net/http" + "slices" "strings" "go.opentelemetry.io/otel/attribute" @@ -43,9 +44,6 @@ var _ HTTPServer = newHTTPServer{} // 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 newHTTPServer) TraceRequest(server string, req *http.Request) []attribute.KeyValue { - // old http.target http.scheme net.host.name net.host.port http.scheme net.host.name net.host.port http.method net.sock.peer.addr net.sock.peer.port user_agent.original http.method http.status_code net.protocol.version - // new http.request.header server.address server.port network.local.address network.local.port client.address client.port url.path url.query url.scheme user_agent.original server.address server.port url.scheme http.request.method http.response.status_code error.type network.protocol.name network.protocol.version http.request.method_original http.response.header http.request.method network.peer.address network.peer.port network.transport http.request.method http.response.status_code error.type network.protocol.name network.protocol.version - const MaxAttributes = 11 attrs := make([]attribute.KeyValue, MaxAttributes) var host string @@ -106,8 +104,7 @@ func (n newHTTPServer) TraceRequest(server string, req *http.Request) []attribut i++ } - // // TODO: When we drop go1.20 support use slices.clip(). - return attrs[:i:i] + return slices.Clip(attrs[:i]) } func (n newHTTPServer) method(method string, attrs []attribute.KeyValue) int {