diff --git a/CHANGELOG.md b/CHANGELOG.md index 408194fd3..a41aed05f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ### Changed - Upgrade OpenTelemetry semantic conventions to v1.18.0. ([#162](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/162)) +- Remove the HTTP path from span names in `net/http`, `gin-gonic/gin`, and `gorilla/mux` instrumentations. ([#161](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/161)) ## [v0.2.1-alpha] - 2023-05-15 diff --git a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go index 48f449c22..49027c2bd 100644 --- a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go +++ b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go @@ -18,7 +18,6 @@ import ( "bytes" "encoding/binary" "errors" - "fmt" "os" @@ -41,6 +40,8 @@ import ( //go:generate go run github.com/cilium/ebpf/cmd/bpf2go -target amd64,arm64 -cc clang -cflags $CFLAGS bpf ./bpf/probe.bpf.c +const instrumentedPkg = "github.com/gin-gonic/gin" + // Event represents an event in the gin-gonic/gin server during an HTTP // request-response. type Event struct { @@ -66,7 +67,7 @@ func New() *Instrumentor { // LibraryName returns the gin-gonic/gin package import path. func (h *Instrumentor) LibraryName() string { - return "github.com/gin-gonic/gin" + return instrumentedPkg } // FuncNames returns the function names from "github.com/gin-gonic/gin" that are @@ -194,7 +195,6 @@ func (h *Instrumentor) Run(eventsChan chan<- *events.Event) { func (h *Instrumentor) convertEvent(e *Event) *events.Event { method := unix.ByteSliceToString(e.Method[:]) path := unix.ByteSliceToString(e.Path[:]) - name := fmt.Sprintf("%s %s", method, path) sc := trace.NewSpanContext(trace.SpanContextConfig{ TraceID: e.SpanContext.TraceID, @@ -203,8 +203,11 @@ func (h *Instrumentor) convertEvent(e *Event) *events.Event { }) return &events.Event{ - Library: h.LibraryName(), - Name: name, + Library: h.LibraryName(), + // Do not include the high-cardinality path here (there is no + // templatized path manifest to reference, given we are instrumenting + // Engine.ServeHTTP which is not passed a Gin Context). + Name: method, Kind: trace.SpanKindServer, StartTime: int64(e.StartTime), EndTime: int64(e.EndTime), diff --git a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe_test.go b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe_test.go new file mode 100644 index 000000000..04e1c8a12 --- /dev/null +++ b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe_test.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gin + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/auto/pkg/instrumentors/context" + "go.opentelemetry.io/auto/pkg/instrumentors/events" + "go.opentelemetry.io/otel/attribute" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" + "go.opentelemetry.io/otel/trace" +) + +func TestInstrumentorConvertEvent(t *testing.T) { + start := time.Now() + end := start.Add(1 * time.Second) + + traceID := trace.TraceID{1} + spanID := trace.SpanID{1} + + i := New() + got := i.convertEvent(&Event{ + StartTime: uint64(start.UnixNano()), + EndTime: uint64(end.UnixNano()), + // "GET" + Method: [7]byte{0x47, 0x45, 0x54}, + // "/foo/bar" + Path: [100]byte{0x2f, 0x66, 0x6f, 0x6f, 0x2f, 0x62, 0x61, 0x72}, + SpanContext: context.EBPFSpanContext{TraceID: traceID, SpanID: spanID}, + }) + + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }) + want := &events.Event{ + Library: instrumentedPkg, + Name: "GET", + Kind: trace.SpanKindServer, + StartTime: int64(start.UnixNano()), + EndTime: int64(end.UnixNano()), + SpanContext: &sc, + Attributes: []attribute.KeyValue{ + semconv.HTTPMethodKey.String("GET"), + semconv.HTTPTargetKey.String("/foo/bar"), + }, + } + assert.Equal(t, want, got) +} diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go index 3fc29b74e..20c8c751d 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go @@ -18,7 +18,6 @@ import ( "bytes" "encoding/binary" "errors" - "fmt" "os" "go.opentelemetry.io/auto/pkg/instrumentors/bpffs" @@ -40,6 +39,8 @@ import ( //go:generate go run github.com/cilium/ebpf/cmd/bpf2go -target amd64,arm64 -cc clang -cflags $CFLAGS bpf ./bpf/probe.bpf.c +const instrumentedPkg = "github.com/gorilla/mux" + // Event represents an event in the gorilla/mux server during an HTTP // request-response. type Event struct { @@ -65,7 +66,7 @@ func New() *Instrumentor { // LibraryName returns the gorilla/mux package import path. func (g *Instrumentor) LibraryName() string { - return "github.com/gorilla/mux" + return instrumentedPkg } // FuncNames returns the function names from "github.com/gorilla/mux" that are @@ -191,7 +192,6 @@ func (g *Instrumentor) Run(eventsChan chan<- *events.Event) { func (g *Instrumentor) convertEvent(e *Event) *events.Event { method := unix.ByteSliceToString(e.Method[:]) path := unix.ByteSliceToString(e.Path[:]) - name := fmt.Sprintf("%s %s", method, path) sc := trace.NewSpanContext(trace.SpanContextConfig{ TraceID: e.SpanContext.TraceID, @@ -200,8 +200,10 @@ func (g *Instrumentor) convertEvent(e *Event) *events.Event { }) return &events.Event{ - Library: g.LibraryName(), - Name: name, + Library: g.LibraryName(), + // Do not include the high-cardinality path here (there is no + // templatized path manifest to reference). + Name: method, Kind: trace.SpanKindServer, StartTime: int64(e.StartTime), EndTime: int64(e.EndTime), diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe_test.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe_test.go new file mode 100644 index 000000000..ae5ab09ba --- /dev/null +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe_test.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package mux + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/auto/pkg/instrumentors/context" + "go.opentelemetry.io/auto/pkg/instrumentors/events" + "go.opentelemetry.io/otel/attribute" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" + "go.opentelemetry.io/otel/trace" +) + +func TestInstrumentorConvertEvent(t *testing.T) { + start := time.Now() + end := start.Add(1 * time.Second) + + traceID := trace.TraceID{1} + spanID := trace.SpanID{1} + + i := New() + got := i.convertEvent(&Event{ + StartTime: uint64(start.UnixNano()), + EndTime: uint64(end.UnixNano()), + // "GET" + Method: [7]byte{0x47, 0x45, 0x54}, + // "/foo/bar" + Path: [100]byte{0x2f, 0x66, 0x6f, 0x6f, 0x2f, 0x62, 0x61, 0x72}, + SpanContext: context.EBPFSpanContext{TraceID: traceID, SpanID: spanID}, + }) + + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }) + want := &events.Event{ + Library: instrumentedPkg, + Name: "GET", + Kind: trace.SpanKindServer, + StartTime: int64(start.UnixNano()), + EndTime: int64(end.UnixNano()), + SpanContext: &sc, + Attributes: []attribute.KeyValue{ + semconv.HTTPMethodKey.String("GET"), + semconv.HTTPTargetKey.String("/foo/bar"), + }, + } + assert.Equal(t, want, got) +} diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go index 7a209c4a9..7813fb36e 100644 --- a/pkg/instrumentors/bpf/net/http/server/probe.go +++ b/pkg/instrumentors/bpf/net/http/server/probe.go @@ -18,7 +18,6 @@ import ( "bytes" "encoding/binary" "errors" - "fmt" "os" "go.opentelemetry.io/auto/pkg/instrumentors/bpffs" @@ -40,6 +39,8 @@ import ( //go:generate go run github.com/cilium/ebpf/cmd/bpf2go -target amd64,arm64 -cc clang -cflags $CFLAGS bpf ./bpf/probe.bpf.c +const instrumentedPkg = "net/http" + // Event represents an event in an HTTP server during an HTTP // request-response. type Event struct { @@ -65,7 +66,7 @@ func New() *Instrumentor { // LibraryName returns the net/http package name. func (h *Instrumentor) LibraryName() string { - return "net/http" + return instrumentedPkg } // FuncNames returns the function names from "net/http" that are instrumented. @@ -192,7 +193,6 @@ func (h *Instrumentor) Run(eventsChan chan<- *events.Event) { func (h *Instrumentor) convertEvent(e *Event) *events.Event { method := unix.ByteSliceToString(e.Method[:]) path := unix.ByteSliceToString(e.Path[:]) - name := fmt.Sprintf("%s %s", method, path) sc := trace.NewSpanContext(trace.SpanContextConfig{ TraceID: e.SpanContext.TraceID, @@ -201,8 +201,10 @@ func (h *Instrumentor) convertEvent(e *Event) *events.Event { }) return &events.Event{ - Library: h.LibraryName(), - Name: name, + Library: h.LibraryName(), + // Do not include the high-cardinality path here (there is no + // templatized path manifest to reference). + Name: method, Kind: trace.SpanKindServer, StartTime: int64(e.StartTime), EndTime: int64(e.EndTime), diff --git a/pkg/instrumentors/bpf/net/http/server/probe_test.go b/pkg/instrumentors/bpf/net/http/server/probe_test.go new file mode 100644 index 000000000..23dc3e5ed --- /dev/null +++ b/pkg/instrumentors/bpf/net/http/server/probe_test.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/auto/pkg/instrumentors/context" + "go.opentelemetry.io/auto/pkg/instrumentors/events" + "go.opentelemetry.io/otel/attribute" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" + "go.opentelemetry.io/otel/trace" +) + +func TestInstrumentorConvertEvent(t *testing.T) { + start := time.Now() + end := start.Add(1 * time.Second) + + traceID := trace.TraceID{1} + spanID := trace.SpanID{1} + + i := New() + got := i.convertEvent(&Event{ + StartTime: uint64(start.UnixNano()), + EndTime: uint64(end.UnixNano()), + // "GET" + Method: [7]byte{0x47, 0x45, 0x54}, + // "/foo/bar" + Path: [100]byte{0x2f, 0x66, 0x6f, 0x6f, 0x2f, 0x62, 0x61, 0x72}, + SpanContext: context.EBPFSpanContext{TraceID: traceID, SpanID: spanID}, + }) + + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + }) + want := &events.Event{ + Library: instrumentedPkg, + Name: "GET", + Kind: trace.SpanKindServer, + StartTime: int64(start.UnixNano()), + EndTime: int64(end.UnixNano()), + SpanContext: &sc, + Attributes: []attribute.KeyValue{ + semconv.HTTPMethodKey.String("GET"), + semconv.HTTPTargetKey.String("/foo/bar"), + }, + } + assert.Equal(t, want, got) +} diff --git a/test/e2e/gin/traces.json b/test/e2e/gin/traces.json index 79840ad51..17535def5 100644 --- a/test/e2e/gin/traces.json +++ b/test/e2e/gin/traces.json @@ -45,7 +45,7 @@ } ], "kind": 2, - "name": "GET /hello-gin", + "name": "GET", "parentSpanId": "", "spanId": "xxxxx", "status": {}, diff --git a/test/e2e/gin/verify.bats b/test/e2e/gin/verify.bats index 198590c16..08e9dc321 100644 --- a/test/e2e/gin/verify.bats +++ b/test/e2e/gin/verify.bats @@ -9,9 +9,9 @@ LIBRARY_NAME="github.com/gin-gonic/gin" assert_equal "$result" '"sample-app"' } -@test "${LIBRARY_NAME} :: emits a span name '{http.method} {http.target}' (per semconv)" { +@test "${LIBRARY_NAME} :: emits a span name '{http.method}' (per semconv)" { result=$(span_names_for ${LIBRARY_NAME}) - assert_equal "$result" '"GET /hello-gin"' + assert_equal "$result" '"GET"' } @test "${LIBRARY_NAME} :: includes http.method attribute" { diff --git a/test/e2e/gorillamux/traces.json b/test/e2e/gorillamux/traces.json index 46064e0a6..14fac6529 100644 --- a/test/e2e/gorillamux/traces.json +++ b/test/e2e/gorillamux/traces.json @@ -45,7 +45,7 @@ } ], "kind": 2, - "name": "GET /users/foo", + "name": "GET", "parentSpanId": "", "spanId": "xxxxx", "status": {}, @@ -74,7 +74,7 @@ } ], "kind": 2, - "name": "GET /users/foo", + "name": "GET", "parentSpanId": "", "spanId": "xxxxx", "status": {}, diff --git a/test/e2e/gorillamux/verify.bats b/test/e2e/gorillamux/verify.bats index 711ac3b7a..9b0624218 100644 --- a/test/e2e/gorillamux/verify.bats +++ b/test/e2e/gorillamux/verify.bats @@ -9,9 +9,9 @@ LIBRARY_NAME="github.com/gorilla/mux" assert_equal "$result" '"sample-app"' } -@test "${LIBRARY_NAME} :: emits a span name '{http.method} {http.target}' (per semconv)" { +@test "${LIBRARY_NAME} :: emits a span name '{http.method}' (per semconv)" { result=$(span_names_for ${LIBRARY_NAME}) - assert_equal "$result" '"GET /users/foo"' + assert_equal "$result" '"GET"' } @test "${LIBRARY_NAME} :: includes http.method attribute" { diff --git a/test/e2e/nethttp/traces.json b/test/e2e/nethttp/traces.json index 7a8f957ed..c6f740302 100644 --- a/test/e2e/nethttp/traces.json +++ b/test/e2e/nethttp/traces.json @@ -45,7 +45,7 @@ } ], "kind": 2, - "name": "GET /hello", + "name": "GET", "parentSpanId": "", "spanId": "xxxxx", "status": {}, diff --git a/test/e2e/nethttp/verify.bats b/test/e2e/nethttp/verify.bats index ff9f6b3ee..ff7d16472 100644 --- a/test/e2e/nethttp/verify.bats +++ b/test/e2e/nethttp/verify.bats @@ -9,9 +9,9 @@ LIBRARY_NAME="net/http" assert_equal "$result" '"sample-app"' } -@test "${LIBRARY_NAME} :: emits a span name '{http.method} {http.target}' (per semconv)" { +@test "${LIBRARY_NAME} :: emits a span name '{http.method}' (per semconv)" { result=$(span_names_for ${LIBRARY_NAME}) - assert_equal "$result" '"GET /hello"' + assert_equal "$result" '"GET"' } @test "${LIBRARY_NAME} :: includes http.method attribute" {