Skip to content

Commit

Permalink
Address PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MadVikingGod committed Mar 12, 2024
1 parent c5dfc91 commit d0f8cc4
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 14 deletions.
10 changes: 5 additions & 5 deletions instrumentation/net/http/otelhttp/internal/semconv/README.md
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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.
4 changes: 2 additions & 2 deletions instrumentation/net/http/otelhttp/internal/semconv/dup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/net/http/otelhttp/internal/semconv/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}
Expand Down
7 changes: 2 additions & 5 deletions instrumentation/net/http/otelhttp/internal/semconv/v1.24.0.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package semconv // import "go.opentelemetry.io/contrib/instrumentation/net/http/

import (
"net/http"
"slices"
"strings"

"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit d0f8cc4

Please sign in to comment.