From 004d605e888177464c00c94cd736eaa3ae023bbe Mon Sep 17 00:00:00 2001 From: Flc Date: Fri, 22 Nov 2024 12:44:00 +0800 Subject: [PATCH 1/5] tests(otelgin): move test files and simplify main package dependencies --- .../gin-gonic/gin/otelgin/gintrace.go | 3 +- .../gin-gonic/gin/otelgin/gintrace_test.go | 97 ------------------- .../github.com/gin-gonic/gin/otelgin/go.mod | 3 - .../gin/otelgin/test/gintrace_test.go | 84 +++++++++++++++- .../gin-gonic/gin/otelgin/test/go.mod | 10 +- 5 files changed, 87 insertions(+), 110 deletions(-) delete mode 100644 instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index 1affd4d6ca5..27156eed22a 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -10,13 +10,14 @@ import ( "github.com/gin-gonic/gin" - "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" semconv "go.opentelemetry.io/otel/semconv/v1.20.0" oteltrace "go.opentelemetry.io/otel/trace" + + "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil" ) const ( diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go deleted file mode 100644 index ecef39e3aeb..00000000000 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -// Based on https://github.com/DataDog/dd-trace-go/blob/8fb554ff7cf694267f9077ae35e27ce4689ed8b6/contrib/gin-gonic/gin/gintrace_test.go - -package otelgin - -import ( - "context" - "net/http" - "net/http/httptest" - "testing" - - "github.com/gin-gonic/gin" - "github.com/stretchr/testify/assert" - - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/propagation" - "go.opentelemetry.io/otel/trace" - "go.opentelemetry.io/otel/trace/noop" - - b3prop "go.opentelemetry.io/contrib/propagators/b3" -) - -func init() { - gin.SetMode(gin.ReleaseMode) // silence annoying log msgs -} - -func TestGetSpanNotInstrumented(t *testing.T) { - router := gin.New() - router.GET("/ping", func(c *gin.Context) { - // Assert we don't have a span on the context. - span := trace.SpanFromContext(c.Request.Context()) - ok := !span.SpanContext().IsValid() - assert.True(t, ok) - _, _ = c.Writer.Write([]byte("ok")) - }) - r := httptest.NewRequest("GET", "/ping", nil) - w := httptest.NewRecorder() - router.ServeHTTP(w, r) - response := w.Result() //nolint:bodyclose // False positive for httptest.ResponseRecorder: https://github.com/timakin/bodyclose/issues/59. - assert.Equal(t, http.StatusOK, response.StatusCode) -} - -func TestPropagationWithGlobalPropagators(t *testing.T) { - provider := noop.NewTracerProvider() - otel.SetTextMapPropagator(b3prop.New()) - - r := httptest.NewRequest("GET", "/user/123", nil) - w := httptest.NewRecorder() - - ctx := context.Background() - sc := trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: trace.TraceID{0x01}, - SpanID: trace.SpanID{0x01}, - }) - ctx = trace.ContextWithRemoteSpanContext(ctx, sc) - ctx, _ = provider.Tracer(ScopeName).Start(ctx, "test") - otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(r.Header)) - - router := gin.New() - router.Use(Middleware("foobar", WithTracerProvider(provider))) - router.GET("/user/:id", func(c *gin.Context) { - span := trace.SpanFromContext(c.Request.Context()) - assert.Equal(t, sc.TraceID(), span.SpanContext().TraceID()) - assert.Equal(t, sc.SpanID(), span.SpanContext().SpanID()) - }) - - router.ServeHTTP(w, r) -} - -func TestPropagationWithCustomPropagators(t *testing.T) { - provider := noop.NewTracerProvider() - b3 := b3prop.New() - - r := httptest.NewRequest("GET", "/user/123", nil) - w := httptest.NewRecorder() - - ctx := context.Background() - sc := trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: trace.TraceID{0x01}, - SpanID: trace.SpanID{0x01}, - }) - ctx = trace.ContextWithRemoteSpanContext(ctx, sc) - ctx, _ = provider.Tracer(ScopeName).Start(ctx, "test") - b3.Inject(ctx, propagation.HeaderCarrier(r.Header)) - - router := gin.New() - router.Use(Middleware("foobar", WithTracerProvider(provider), WithPropagators(b3))) - router.GET("/user/:id", func(c *gin.Context) { - span := trace.SpanFromContext(c.Request.Context()) - assert.Equal(t, sc.TraceID(), span.SpanContext().TraceID()) - assert.Equal(t, sc.SpanID(), span.SpanContext().SpanID()) - }) - - router.ServeHTTP(w, r) -} diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod index 0d66379a99a..c4edad3a4f1 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod @@ -2,12 +2,9 @@ module go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otel go 1.22 -replace go.opentelemetry.io/contrib/propagators/b3 => ../../../../../propagators/b3 - require ( github.com/gin-gonic/gin v1.10.0 github.com/stretchr/testify v1.9.0 - go.opentelemetry.io/contrib/propagators/b3 v1.32.0 go.opentelemetry.io/otel v1.32.0 go.opentelemetry.io/otel/trace v1.32.0 ) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go index fb8698422dc..2ef3003aeae 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go @@ -16,21 +16,95 @@ import ( "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/net/context" - "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/propagation" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" + "go.opentelemetry.io/otel/trace" + "go.opentelemetry.io/otel/trace/noop" - "go.opentelemetry.io/otel/attribute" - oteltrace "go.opentelemetry.io/otel/trace" + "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" + b3prop "go.opentelemetry.io/contrib/propagators/b3" ) func init() { gin.SetMode(gin.ReleaseMode) // silence annoying log msgs } +func TestGetSpanNotInstrumented(t *testing.T) { + router := gin.New() + router.GET("/ping", func(c *gin.Context) { + // Assert we don't have a span on the context. + span := trace.SpanFromContext(c.Request.Context()) + ok := !span.SpanContext().IsValid() + assert.True(t, ok) + _, _ = c.Writer.Write([]byte("ok")) + }) + r := httptest.NewRequest("GET", "/ping", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + response := w.Result() //nolint:bodyclose // False positive for httptest.ResponseRecorder: https://github.com/timakin/bodyclose/issues/59. + assert.Equal(t, http.StatusOK, response.StatusCode) +} + +func TestPropagationWithGlobalPropagators(t *testing.T) { + provider := noop.NewTracerProvider() + otel.SetTextMapPropagator(b3prop.New()) + + r := httptest.NewRequest("GET", "/user/123", nil) + w := httptest.NewRecorder() + + ctx := context.Background() + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: trace.TraceID{0x01}, + SpanID: trace.SpanID{0x01}, + }) + ctx = trace.ContextWithRemoteSpanContext(ctx, sc) + ctx, _ = provider.Tracer(otelgin.ScopeName).Start(ctx, "test") + otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(r.Header)) + + router := gin.New() + router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider))) + router.GET("/user/:id", func(c *gin.Context) { + span := trace.SpanFromContext(c.Request.Context()) + assert.Equal(t, sc.TraceID(), span.SpanContext().TraceID()) + assert.Equal(t, sc.SpanID(), span.SpanContext().SpanID()) + }) + + router.ServeHTTP(w, r) +} + +func TestPropagationWithCustomPropagators(t *testing.T) { + provider := noop.NewTracerProvider() + b3 := b3prop.New() + + r := httptest.NewRequest("GET", "/user/123", nil) + w := httptest.NewRecorder() + + ctx := context.Background() + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: trace.TraceID{0x01}, + SpanID: trace.SpanID{0x01}, + }) + ctx = trace.ContextWithRemoteSpanContext(ctx, sc) + ctx, _ = provider.Tracer(otelgin.ScopeName).Start(ctx, "test") + b3.Inject(ctx, propagation.HeaderCarrier(r.Header)) + + router := gin.New() + router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider), otelgin.WithPropagators(b3))) + router.GET("/user/:id", func(c *gin.Context) { + span := trace.SpanFromContext(c.Request.Context()) + assert.Equal(t, sc.TraceID(), span.SpanContext().TraceID()) + assert.Equal(t, sc.SpanID(), span.SpanContext().SpanID()) + }) + + router.ServeHTTP(w, r) +} + func TestChildSpanFromGlobalTracer(t *testing.T) { sr := tracetest.NewSpanRecorder() otel.SetTracerProvider(sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr))) @@ -85,7 +159,7 @@ func TestTrace200(t *testing.T) { require.Len(t, spans, 1) span := spans[0] assert.Equal(t, "/user/:id", span.Name()) - assert.Equal(t, oteltrace.SpanKindServer, span.SpanKind()) + assert.Equal(t, trace.SpanKindServer, span.SpanKind()) attr := span.Attributes() assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusOK)) @@ -209,7 +283,7 @@ func TestHTTPRouteWithSpanNameFormatter(t *testing.T) { require.Len(t, spans, 1) span := spans[0] assert.Equal(t, "/user/123", span.Name()) - assert.Equal(t, oteltrace.SpanKindServer, span.SpanKind()) + assert.Equal(t, trace.SpanKindServer, span.SpanKind()) attr := span.Attributes() assert.Contains(t, attr, attribute.String("http.method", "GET")) assert.Contains(t, attr, attribute.String("http.route", "/user/:id")) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod b/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod index d5d7bf2b3cd..1d0ebefecde 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod @@ -6,9 +6,11 @@ require ( github.com/gin-gonic/gin v1.10.0 github.com/stretchr/testify v1.9.0 go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin v0.57.0 + go.opentelemetry.io/contrib/propagators/b3 v1.32.0 go.opentelemetry.io/otel v1.32.0 go.opentelemetry.io/otel/sdk v1.32.0 go.opentelemetry.io/otel/trace v1.32.0 + golang.org/x/net v0.31.0 ) require ( @@ -39,13 +41,13 @@ require ( go.opentelemetry.io/otel/metric v1.32.0 // indirect golang.org/x/arch v0.12.0 // indirect golang.org/x/crypto v0.29.0 // indirect - golang.org/x/net v0.31.0 // indirect golang.org/x/sys v0.27.0 // indirect golang.org/x/text v0.20.0 // indirect google.golang.org/protobuf v1.35.2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) -replace go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin => ../ - -replace go.opentelemetry.io/contrib/propagators/b3 => ../../../../../../propagators/b3 +replace ( + go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin => ../ + go.opentelemetry.io/contrib/propagators/b3 => ../../../../../../propagators/b3 +) From ee9ef1e3781bdfbfc9cb2b3860c64493d2357f2e Mon Sep 17 00:00:00 2001 From: Flc Date: Fri, 22 Nov 2024 12:47:11 +0800 Subject: [PATCH 2/5] docs(CHANGELOG.md): Update changelog with changes. --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f022916f572..210d069ea20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Added support for providing `endpoint`, `pollingIntervalMs` and `initialSamplingRate` using environment variable `OTEL_TRACES_SAMPLER_ARG` in `go.opentelemetry.io/contrib/samples/jaegerremote`. (#6310) - Added support exporting logs via OTLP over gRPC in `go.opentelemetry.io/contrib/config`. (#6340) +### Changed + +- + ### Fixed - Fixed the value for configuring the OTLP exporter to use `grpc` instead of `grpc/protobuf` in `go.opentelemetry.io/contrib/config`. (#6338) @@ -60,6 +64,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed +- Move the test files of `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin` to the tests directory and simplify the dependencies of the main package. (#6360) + - The function signature of `NewLogProcessor` in `go.opentelemetry.io/contrib/processors/minsev` has changed to accept the added `Severitier` interface instead of a `log.Severity`. (#6116) - Updated `go.opentelemetry.io/contrib/config` to use the [v0.3.0](https://github.com/open-telemetry/opentelemetry-configuration/releases/tag/v0.3.0) release of schema which includes backwards incompatible changes. (#6126) - `NewSDK` in `go.opentelemetry.io/contrib/config` now returns a no-op SDK if `disabled` is set to `true`. (#6185) From 07eef28bd77f31be443c5dceaf82c179ce1fb82f Mon Sep 17 00:00:00 2001 From: Flc Date: Fri, 22 Nov 2024 12:47:43 +0800 Subject: [PATCH 3/5] docs(CHANGELOG.md): Update changelog with changes. --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 210d069ea20..40d2c55476b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed -- +- Move the test files of `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin` to the tests directory and simplify the dependencies of the main package. (#6360) ### Fixed @@ -64,8 +64,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed -- Move the test files of `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin` to the tests directory and simplify the dependencies of the main package. (#6360) - - The function signature of `NewLogProcessor` in `go.opentelemetry.io/contrib/processors/minsev` has changed to accept the added `Severitier` interface instead of a `log.Severity`. (#6116) - Updated `go.opentelemetry.io/contrib/config` to use the [v0.3.0](https://github.com/open-telemetry/opentelemetry-configuration/releases/tag/v0.3.0) release of schema which includes backwards incompatible changes. (#6126) - `NewSDK` in `go.opentelemetry.io/contrib/config` now returns a no-op SDK if `disabled` is set to `true`. (#6185) From 5699ce0896591b1e58a1fcbc7d3b707e15ccc0bf Mon Sep 17 00:00:00 2001 From: Flc Date: Fri, 22 Nov 2024 16:48:16 +0800 Subject: [PATCH 4/5] docs(CHANGELOG): remove changed --- CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d2c55476b..f022916f572 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Added support for providing `endpoint`, `pollingIntervalMs` and `initialSamplingRate` using environment variable `OTEL_TRACES_SAMPLER_ARG` in `go.opentelemetry.io/contrib/samples/jaegerremote`. (#6310) - Added support exporting logs via OTLP over gRPC in `go.opentelemetry.io/contrib/config`. (#6340) -### Changed - -- Move the test files of `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin` to the tests directory and simplify the dependencies of the main package. (#6360) - ### Fixed - Fixed the value for configuring the OTLP exporter to use `grpc` instead of `grpc/protobuf` in `go.opentelemetry.io/contrib/config`. (#6338) From 05a19e9a359809fbba898988d43d16d44feaf2a6 Mon Sep 17 00:00:00 2001 From: Flc Date: Mon, 25 Nov 2024 22:25:02 +0800 Subject: [PATCH 5/5] chore(otelgin): adjust imports sorting --- instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go | 3 +-- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index 27156eed22a..1affd4d6ca5 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -10,14 +10,13 @@ import ( "github.com/gin-gonic/gin" + "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" semconv "go.opentelemetry.io/otel/semconv/v1.20.0" oteltrace "go.opentelemetry.io/otel/trace" - - "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil" ) const ( diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go index 2ef3003aeae..e648640ab58 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go @@ -18,6 +18,8 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/net/context" + "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" + b3prop "go.opentelemetry.io/contrib/propagators/b3" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -26,9 +28,6 @@ import ( "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" - - "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" - b3prop "go.opentelemetry.io/contrib/propagators/b3" ) func init() {