From 60f2e0c43d1269222ee30f98d9fa88cd5da40b8a Mon Sep 17 00:00:00 2001 From: Flc Date: Wed, 20 Nov 2024 00:09:48 +0800 Subject: [PATCH 01/20] feat(otelgin): enhance gin error tracking with span recording --- instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index 1affd4d6ca5..7c7bdeb83b3 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -9,14 +9,14 @@ import ( "fmt" "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 ( @@ -96,6 +96,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { } if len(c.Errors) > 0 { span.SetAttributes(attribute.String("gin.errors", c.Errors.String())) + span.RecordError(fmt.Errorf("gin errors: %s", c.Errors.String())) } } } From 78178375c40c4d2064e8cfed5733c1507bb75cf2 Mon Sep 17 00:00:00 2001 From: Flc Date: Wed, 20 Nov 2024 22:39:01 +0800 Subject: [PATCH 02/20] feat(otelgin): enhance gin error tracking with span recording --- instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index 7c7bdeb83b3..8c7e01011c2 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -9,6 +9,7 @@ import ( "fmt" "github.com/gin-gonic/gin" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" From dcc6c626ff2023e6b3cfffa981946c7b6ccd160c Mon Sep 17 00:00:00 2001 From: Flc Date: Wed, 20 Nov 2024 22:45:59 +0800 Subject: [PATCH 03/20] feat(otelgin): enhance gin error tracking with span recording --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 654e7c555e1..e87bd2e6547 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 + +- feat(otelgin): enhance gin error tracking with span recording (#6346) + ### Fixed - Fixed the value for configuring the OTLP exporter to use `grpc` instead of `grpc/protobuf` in `go.opentelemetry.io/contrib/config`. (#6338) From f5d9a1c02e2536f077fd09effa142823a7de1a73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flc=E3=82=9B?= Date: Thu, 21 Nov 2024 00:19:21 +0800 Subject: [PATCH 04/20] Update CHANGELOG.md Co-authored-by: Damien Mathieu <42@dmathieu.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e87bd2e6547..4c09ad39d7c 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 -- feat(otelgin): enhance gin error tracking with span recording (#6346) +- Record an error instead of setting the `gin.errors` attribute in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#6346) ### Fixed From 84f2f0eaedc5d78e10ce55624371f362b3a61729 Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 17:15:34 +0800 Subject: [PATCH 05/20] test(otelgin): Add tests for span recording on errors --- .golangci.yml | 1 + .../gin-gonic/gin/otelgin/gintrace.go | 4 +- .../gin-gonic/gin/otelgin/gintrace_test.go | 60 +++++++++++++++++++ .../github.com/gin-gonic/gin/otelgin/go.mod | 2 + .../github.com/gin-gonic/gin/otelgin/go.sum | 4 ++ 5 files changed, 69 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 512e11a3740..f21608817af 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -82,6 +82,7 @@ linters-settings: - "**/*/{bridges,instrumentation}/**/*.go" - "!**/*/bridges/prometheus/*.go" # prometheus bridge is meant to use the SDK - "!**/*/instrumentation/runtime/*.go" # runtime instrumentation is meant to use the SDK + - "!**/*/instrumentation/github.com/gin-gonic/gin/otelgin/*.go" # gin instrumentation is meant to use the SDK - "!**/*test/*.go" - "!**/*test/**/*.go" - "!**/*example/*.go" diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index 8c7e01011c2..d350be898df 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -6,6 +6,7 @@ package otelgin // import "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" import ( + "errors" "fmt" "github.com/gin-gonic/gin" @@ -96,8 +97,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { span.SetAttributes(semconv.HTTPStatusCode(status)) } if len(c.Errors) > 0 { - span.SetAttributes(attribute.String("gin.errors", c.Errors.String())) - span.RecordError(fmt.Errorf("gin errors: %s", c.Errors.String())) + span.RecordError(errors.New(c.Errors.String())) } } } diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go index ecef39e3aeb..a9a50936579 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go @@ -15,7 +15,10 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" + tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" @@ -95,3 +98,60 @@ func TestPropagationWithCustomPropagators(t *testing.T) { router.ServeHTTP(w, r) } + +func TestSpanRecordError(t *testing.T) { + exporter := tracetest.NewInMemoryExporter() + + router := gin.New() + router.Use(Middleware("foobar", WithTracerProvider( + tracesdk.NewTracerProvider( + tracesdk.WithSyncer(exporter), + )), + )) + + t.Run("test success", func(t *testing.T) { + defer exporter.Reset() + assert.Empty(t, exporter.GetSpans()) + + router.GET("/success", func(c *gin.Context) { + c.Status(http.StatusOK) + }) + r := httptest.NewRequest("GET", "/success", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Len(t, exporter.GetSpans(), 1) + + // Assert span status + span := exporter.GetSpans()[0] + assert.Equal(t, "/success", span.Name) + assert.NotEqual(t, codes.Error, span.Status.Code) + assert.Empty(t, span.Events) + }) + + // test success + t.Run("test error", func(t *testing.T) { + defer exporter.Reset() + assert.Empty(t, exporter.GetSpans()) + + router.GET("/error", func(c *gin.Context) { + assert.Error(t, c.AbortWithError(http.StatusInternalServerError, assert.AnError)) + }) + r := httptest.NewRequest("GET", "/error", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Len(t, exporter.GetSpans(), 1) + + // Assert span + span := exporter.GetSpans()[0] + assert.Equal(t, "/error", span.Name) + assert.Equal(t, codes.Error, span.Status.Code) + assert.Len(t, span.Events, 1) + + // Assert span events + assert.Equal(t, "exception", span.Events[0].Name) + }) +} diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod index 5dc945986e8..8ec4434fe64 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod @@ -9,6 +9,7 @@ require ( 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/sdk v1.32.0 go.opentelemetry.io/otel/trace v1.32.0 ) @@ -26,6 +27,7 @@ require ( github.com/go-playground/universal-translator v0.18.1 // indirect github.com/go-playground/validator/v10 v10.23.0 // indirect github.com/goccy/go-json v0.10.3 // indirect + github.com/google/uuid v1.6.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/cpuid/v2 v2.2.9 // indirect github.com/leodido/go-urn v1.4.0 // indirect diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum b/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum index a5ebd2b401a..1c1696af496 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum @@ -34,6 +34,8 @@ github.com/goccy/go-json v0.10.3/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PU github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= @@ -71,6 +73,8 @@ go.opentelemetry.io/otel v1.32.0 h1:WnBN+Xjcteh0zdk01SVqV55d/m62NJLJdIyb4y/WO5U= go.opentelemetry.io/otel v1.32.0/go.mod h1:00DCVSB0RQcnzlwyTfqtxSm+DRr9hpYrHjNGiBHVQIg= go.opentelemetry.io/otel/metric v1.32.0 h1:xV2umtmNcThh2/a/aCP+h64Xx5wsj8qqnkYZktzNa0M= go.opentelemetry.io/otel/metric v1.32.0/go.mod h1:jH7CIbbK6SH2V2wE16W05BHCtIDzauciCRLoc/SyMv8= +go.opentelemetry.io/otel/sdk v1.32.0 h1:RNxepc9vK59A8XsgZQouW8ue8Gkb4jpWtJm9ge5lEG4= +go.opentelemetry.io/otel/sdk v1.32.0/go.mod h1:LqgegDBjKMmb2GC6/PrTnteJG39I8/vJCAP9LlJXEjU= go.opentelemetry.io/otel/trace v1.32.0 h1:WIC9mYrXf8TmY/EXuULKc8hR17vE+Hjv2cssQDe03fM= go.opentelemetry.io/otel/trace v1.32.0/go.mod h1:+i4rkvCraA+tG6AzwloGaCtkx53Fa+L+V8e9a7YvhT8= golang.org/x/arch v0.12.0 h1:UsYJhbzPYGsT0HbEdmYcqtCv8UNGvnaL561NnIUvaKg= From e5ef74d22194b93b42cf734d43f44f0414637dad Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 17:24:32 +0800 Subject: [PATCH 06/20] refactor(otelgin): Remove unused dependencies and simplify tests --- .golangci.yml | 1 - .../gin-gonic/gin/otelgin/gintrace_test.go | 60 ------------------ .../github.com/gin-gonic/gin/otelgin/go.mod | 2 - .../github.com/gin-gonic/gin/otelgin/go.sum | 4 -- .../gin/otelgin/test/gintrace_test.go | 63 ++++++++++++++++++- 5 files changed, 60 insertions(+), 70 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index f21608817af..512e11a3740 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -82,7 +82,6 @@ linters-settings: - "**/*/{bridges,instrumentation}/**/*.go" - "!**/*/bridges/prometheus/*.go" # prometheus bridge is meant to use the SDK - "!**/*/instrumentation/runtime/*.go" # runtime instrumentation is meant to use the SDK - - "!**/*/instrumentation/github.com/gin-gonic/gin/otelgin/*.go" # gin instrumentation is meant to use the SDK - "!**/*test/*.go" - "!**/*test/**/*.go" - "!**/*example/*.go" diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go index a9a50936579..ecef39e3aeb 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go @@ -15,10 +15,7 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" - tracesdk "go.opentelemetry.io/otel/sdk/trace" - "go.opentelemetry.io/otel/sdk/trace/tracetest" "go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace/noop" @@ -98,60 +95,3 @@ func TestPropagationWithCustomPropagators(t *testing.T) { router.ServeHTTP(w, r) } - -func TestSpanRecordError(t *testing.T) { - exporter := tracetest.NewInMemoryExporter() - - router := gin.New() - router.Use(Middleware("foobar", WithTracerProvider( - tracesdk.NewTracerProvider( - tracesdk.WithSyncer(exporter), - )), - )) - - t.Run("test success", func(t *testing.T) { - defer exporter.Reset() - assert.Empty(t, exporter.GetSpans()) - - router.GET("/success", func(c *gin.Context) { - c.Status(http.StatusOK) - }) - r := httptest.NewRequest("GET", "/success", nil) - w := httptest.NewRecorder() - router.ServeHTTP(w, r) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Len(t, exporter.GetSpans(), 1) - - // Assert span status - span := exporter.GetSpans()[0] - assert.Equal(t, "/success", span.Name) - assert.NotEqual(t, codes.Error, span.Status.Code) - assert.Empty(t, span.Events) - }) - - // test success - t.Run("test error", func(t *testing.T) { - defer exporter.Reset() - assert.Empty(t, exporter.GetSpans()) - - router.GET("/error", func(c *gin.Context) { - assert.Error(t, c.AbortWithError(http.StatusInternalServerError, assert.AnError)) - }) - r := httptest.NewRequest("GET", "/error", nil) - w := httptest.NewRecorder() - router.ServeHTTP(w, r) - - assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Len(t, exporter.GetSpans(), 1) - - // Assert span - span := exporter.GetSpans()[0] - assert.Equal(t, "/error", span.Name) - assert.Equal(t, codes.Error, span.Status.Code) - assert.Len(t, span.Events, 1) - - // Assert span events - assert.Equal(t, "exception", span.Events[0].Name) - }) -} diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod index 8ec4434fe64..5dc945986e8 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod @@ -9,7 +9,6 @@ require ( 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/sdk v1.32.0 go.opentelemetry.io/otel/trace v1.32.0 ) @@ -27,7 +26,6 @@ require ( github.com/go-playground/universal-translator v0.18.1 // indirect github.com/go-playground/validator/v10 v10.23.0 // indirect github.com/goccy/go-json v0.10.3 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/cpuid/v2 v2.2.9 // indirect github.com/leodido/go-urn v1.4.0 // indirect diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum b/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum index 1c1696af496..a5ebd2b401a 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum @@ -34,8 +34,6 @@ github.com/goccy/go-json v0.10.3/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PU github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= -github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= -github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= @@ -73,8 +71,6 @@ go.opentelemetry.io/otel v1.32.0 h1:WnBN+Xjcteh0zdk01SVqV55d/m62NJLJdIyb4y/WO5U= go.opentelemetry.io/otel v1.32.0/go.mod h1:00DCVSB0RQcnzlwyTfqtxSm+DRr9hpYrHjNGiBHVQIg= go.opentelemetry.io/otel/metric v1.32.0 h1:xV2umtmNcThh2/a/aCP+h64Xx5wsj8qqnkYZktzNa0M= go.opentelemetry.io/otel/metric v1.32.0/go.mod h1:jH7CIbbK6SH2V2wE16W05BHCtIDzauciCRLoc/SyMv8= -go.opentelemetry.io/otel/sdk v1.32.0 h1:RNxepc9vK59A8XsgZQouW8ue8Gkb4jpWtJm9ge5lEG4= -go.opentelemetry.io/otel/sdk v1.32.0/go.mod h1:LqgegDBjKMmb2GC6/PrTnteJG39I8/vJCAP9LlJXEjU= go.opentelemetry.io/otel/trace v1.32.0 h1:WIC9mYrXf8TmY/EXuULKc8hR17vE+Hjv2cssQDe03fM= go.opentelemetry.io/otel/trace v1.32.0/go.mod h1:+i4rkvCraA+tG6AzwloGaCtkx53Fa+L+V8e9a7YvhT8= golang.org/x/arch v0.12.0 h1:UsYJhbzPYGsT0HbEdmYcqtCv8UNGvnaL561NnIUvaKg= 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..f6f6d3d1681 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 @@ -17,14 +17,14 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "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" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" - - "go.opentelemetry.io/otel/attribute" oteltrace "go.opentelemetry.io/otel/trace" + + "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" ) func init() { @@ -321,3 +321,60 @@ func TestWithGinFilter(t *testing.T) { assert.Len(t, sr.Ended(), 1) }) } + +func TestSpanRecordError(t *testing.T) { + exporter := tracetest.NewInMemoryExporter() + + router := gin.New() + router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider( + sdktrace.NewTracerProvider( + sdktrace.WithSyncer(exporter), + )), + )) + + t.Run("test success", func(t *testing.T) { + defer exporter.Reset() + assert.Empty(t, exporter.GetSpans()) + + router.GET("/success", func(c *gin.Context) { + c.Status(http.StatusOK) + }) + r := httptest.NewRequest("GET", "/success", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Len(t, exporter.GetSpans(), 1) + + // Assert span status + span := exporter.GetSpans()[0] + assert.Equal(t, "/success", span.Name) + assert.NotEqual(t, codes.Error, span.Status.Code) + assert.Empty(t, span.Events) + }) + + // test success + t.Run("test error", func(t *testing.T) { + defer exporter.Reset() + assert.Empty(t, exporter.GetSpans()) + + router.GET("/error", func(c *gin.Context) { + assert.Error(t, c.AbortWithError(http.StatusInternalServerError, assert.AnError)) + }) + r := httptest.NewRequest("GET", "/error", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusInternalServerError, w.Code) + assert.Len(t, exporter.GetSpans(), 1) + + // Assert span + span := exporter.GetSpans()[0] + assert.Equal(t, "/error", span.Name) + assert.Equal(t, codes.Error, span.Status.Code) + assert.Len(t, span.Events, 1) + + // Assert span events + assert.Equal(t, "exception", span.Events[0].Name) + }) +} From 9cdd7d6d8a631a3fda1f3f433310606f212a1e0b Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 17:25:22 +0800 Subject: [PATCH 07/20] refactor(otelgin): Remove unused dependencies and simplify tests --- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 1 - 1 file changed, 1 deletion(-) 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 f6f6d3d1681..9422ece6e08 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 @@ -353,7 +353,6 @@ func TestSpanRecordError(t *testing.T) { assert.Empty(t, span.Events) }) - // test success t.Run("test error", func(t *testing.T) { defer exporter.Reset() assert.Empty(t, exporter.GetSpans()) From cc1c61277c8d974e3e00b378bc8a9359723751ce Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 22:09:15 +0800 Subject: [PATCH 08/20] test(instrumentation): remove `gin.errors` assertion in gintrace_test.go --- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 1 - 1 file changed, 1 deletion(-) 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 9422ece6e08..0d7c19facba 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 @@ -120,7 +120,6 @@ func TestError(t *testing.T) { attr := span.Attributes() assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusInternalServerError)) - assert.Contains(t, attr, attribute.String("gin.errors", "Error #01: oh no\n")) // server errors set the status assert.Equal(t, codes.Error, span.Status().Code) } From fc2dd22e607bbd72655a9ff586d7de1ae41ebae2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flc=E3=82=9B?= Date: Thu, 21 Nov 2024 22:36:15 +0800 Subject: [PATCH 09/20] Update instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go Co-authored-by: Damien Mathieu <42@dmathieu.com> --- instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index d350be898df..bab71487eea 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -97,7 +97,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { span.SetAttributes(semconv.HTTPStatusCode(status)) } if len(c.Errors) > 0 { - span.RecordError(errors.New(c.Errors.String())) + span.RecordError(errors.Join(c.Errors)) } } } From 488835cb183e597f735c67b189e4ab339eba7e92 Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 23:07:13 +0800 Subject: [PATCH 10/20] feat(otelgin): refine gin trace error handling and test cases --- .../github.com/gin-gonic/gin/otelgin/gintrace.go | 2 +- .../gin-gonic/gin/otelgin/test/gintrace_test.go | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index bab71487eea..d350be898df 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -97,7 +97,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { span.SetAttributes(semconv.HTTPStatusCode(status)) } if len(c.Errors) > 0 { - span.RecordError(errors.Join(c.Errors)) + span.RecordError(errors.New(c.Errors.String())) } } } 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 0d7c19facba..146e562f3d7 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 @@ -91,6 +91,7 @@ func TestTrace200(t *testing.T) { assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusOK)) assert.Contains(t, attr, attribute.String("http.method", "GET")) assert.Contains(t, attr, attribute.String("http.route", "/user/:id")) + assert.Empty(t, span.Events()) } func TestError(t *testing.T) { @@ -120,6 +121,13 @@ func TestError(t *testing.T) { attr := span.Attributes() assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusInternalServerError)) + + // verify the error event + events := span.Events() + assert.Len(t, events, 1) + assert.Equal(t, "exception", events[0].Name) + assert.Contains(t, events[0].Attributes, attribute.String("exception.type", "*errors.errorString")) + assert.Contains(t, events[0].Attributes, attribute.String("exception.message", "Error #01: oh no\n")) // server errors set the status assert.Equal(t, codes.Error, span.Status().Code) } @@ -373,6 +381,9 @@ func TestSpanRecordError(t *testing.T) { assert.Len(t, span.Events, 1) // Assert span events + assert.Len(t, span.Events, 1) assert.Equal(t, "exception", span.Events[0].Name) + assert.Contains(t, span.Events[0].Attributes, attribute.String("exception.type", "*errors.errorString")) + assert.Contains(t, span.Events[0].Attributes, attribute.String("exception.message", "Error #01: "+assert.AnError.Error()+"\n")) }) } From 92ff3370e69cc3455b1af15f8519d2ee72a9a95d Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 23:12:39 +0800 Subject: [PATCH 11/20] feat(otelgin): refine gin trace error handling and test cases --- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 1 - 1 file changed, 1 deletion(-) 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 146e562f3d7..242b99e2f78 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 @@ -378,7 +378,6 @@ func TestSpanRecordError(t *testing.T) { span := exporter.GetSpans()[0] assert.Equal(t, "/error", span.Name) assert.Equal(t, codes.Error, span.Status.Code) - assert.Len(t, span.Events, 1) // Assert span events assert.Len(t, span.Events, 1) From 23949c776fab64674ba993203a2386e00e08d617 Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 23:51:23 +0800 Subject: [PATCH 12/20] test(otelgin): remove redundant code --- .../gin/otelgin/test/gintrace_test.go | 58 ------------------- 1 file changed, 58 deletions(-) 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 242b99e2f78..973520dd357 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 @@ -328,61 +328,3 @@ func TestWithGinFilter(t *testing.T) { assert.Len(t, sr.Ended(), 1) }) } - -func TestSpanRecordError(t *testing.T) { - exporter := tracetest.NewInMemoryExporter() - - router := gin.New() - router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider( - sdktrace.NewTracerProvider( - sdktrace.WithSyncer(exporter), - )), - )) - - t.Run("test success", func(t *testing.T) { - defer exporter.Reset() - assert.Empty(t, exporter.GetSpans()) - - router.GET("/success", func(c *gin.Context) { - c.Status(http.StatusOK) - }) - r := httptest.NewRequest("GET", "/success", nil) - w := httptest.NewRecorder() - router.ServeHTTP(w, r) - - assert.Equal(t, http.StatusOK, w.Code) - assert.Len(t, exporter.GetSpans(), 1) - - // Assert span status - span := exporter.GetSpans()[0] - assert.Equal(t, "/success", span.Name) - assert.NotEqual(t, codes.Error, span.Status.Code) - assert.Empty(t, span.Events) - }) - - t.Run("test error", func(t *testing.T) { - defer exporter.Reset() - assert.Empty(t, exporter.GetSpans()) - - router.GET("/error", func(c *gin.Context) { - assert.Error(t, c.AbortWithError(http.StatusInternalServerError, assert.AnError)) - }) - r := httptest.NewRequest("GET", "/error", nil) - w := httptest.NewRecorder() - router.ServeHTTP(w, r) - - assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Len(t, exporter.GetSpans(), 1) - - // Assert span - span := exporter.GetSpans()[0] - assert.Equal(t, "/error", span.Name) - assert.Equal(t, codes.Error, span.Status.Code) - - // Assert span events - assert.Len(t, span.Events, 1) - assert.Equal(t, "exception", span.Events[0].Name) - assert.Contains(t, span.Events[0].Attributes, attribute.String("exception.type", "*errors.errorString")) - assert.Contains(t, span.Events[0].Attributes, attribute.String("exception.message", "Error #01: "+assert.AnError.Error()+"\n")) - }) -} From 5f403b8fa5b073fe888594076c4e2c3544173820 Mon Sep 17 00:00:00 2001 From: Flc Date: Sun, 24 Nov 2024 00:20:47 +0800 Subject: [PATCH 13/20] chore(otelgin): refine error recording in gintrace --- .../github.com/gin-gonic/gin/otelgin/gintrace.go | 5 +++-- .../gin-gonic/gin/otelgin/test/gintrace_test.go | 11 ++++++++--- 2 files changed, 11 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 d350be898df..af07b423671 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -6,7 +6,6 @@ package otelgin // import "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" import ( - "errors" "fmt" "github.com/gin-gonic/gin" @@ -97,7 +96,9 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { span.SetAttributes(semconv.HTTPStatusCode(status)) } if len(c.Errors) > 0 { - span.RecordError(errors.New(c.Errors.String())) + for _, err := range c.Errors { + span.RecordError(err.Err) + } } } } 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 973520dd357..6474ebd4f3f 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 @@ -105,7 +105,8 @@ func TestError(t *testing.T) { // configure a handler that returns an error and 5xx status // code router.GET("/server_err", func(c *gin.Context) { - _ = c.AbortWithError(http.StatusInternalServerError, errors.New("oh no")) + _ = c.Error(errors.New("oh no one")) + _ = c.AbortWithError(http.StatusInternalServerError, errors.New("oh no two")) }) r := httptest.NewRequest("GET", "/server_err", nil) w := httptest.NewRecorder() @@ -124,10 +125,14 @@ func TestError(t *testing.T) { // verify the error event events := span.Events() - assert.Len(t, events, 1) + assert.Len(t, events, 2) assert.Equal(t, "exception", events[0].Name) assert.Contains(t, events[0].Attributes, attribute.String("exception.type", "*errors.errorString")) - assert.Contains(t, events[0].Attributes, attribute.String("exception.message", "Error #01: oh no\n")) + assert.Contains(t, events[0].Attributes, attribute.String("exception.message", "oh no one")) + assert.Equal(t, "exception", events[1].Name) + assert.Contains(t, events[1].Attributes, attribute.String("exception.type", "*errors.errorString")) + assert.Contains(t, events[1].Attributes, attribute.String("exception.message", "oh no two")) + // server errors set the status assert.Equal(t, codes.Error, span.Status().Code) } From 05c91e7b95e4c52d6141b666022f7502c26007f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flc=E3=82=9B?= Date: Mon, 25 Nov 2024 19:13:04 +0800 Subject: [PATCH 14/20] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15da7553d70..4115bd63521 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 -- Record an error instead of setting the `gin.errors` attribute in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#6346) +- Record errors instead of setting the `gin.errors` attribute in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#6346) ### Fixed From 8c62a6318052c64d387133bd0f56e5f732bb352b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flc=E3=82=9B?= Date: Mon, 25 Nov 2024 19:13:28 +0800 Subject: [PATCH 15/20] Update instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6474ebd4f3f..f9263c20a76 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 @@ -123,7 +123,7 @@ func TestError(t *testing.T) { assert.Contains(t, attr, attribute.String("net.host.name", "foobar")) assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusInternalServerError)) - // verify the error event + // verify the error events events := span.Events() assert.Len(t, events, 2) assert.Equal(t, "exception", events[0].Name) From 0a2260da5e0c5ed0285d947cd44220d282d9d84d Mon Sep 17 00:00:00 2001 From: Flc Date: Mon, 25 Nov 2024 22:21:32 +0800 Subject: [PATCH 16/20] chore(otelgin): adjust imports sorting --- instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go | 3 +-- .../github.com/gin-gonic/gin/otelgin/gintrace_test.go | 3 +-- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index af07b423671..7582d5a4184 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/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go index ecef39e3aeb..053c124fbae 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace_test.go @@ -14,12 +14,11 @@ import ( "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" + b3prop "go.opentelemetry.io/contrib/propagators/b3" "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() { 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 f9263c20a76..b7b71db2f35 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 @@ -17,14 +17,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "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" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" oteltrace "go.opentelemetry.io/otel/trace" - - "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" ) func init() { From 3ab2b2c4eb6b5c3d64b56480652b438a806a3991 Mon Sep 17 00:00:00 2001 From: Flc Date: Tue, 26 Nov 2024 00:08:05 +0800 Subject: [PATCH 17/20] feat(otelgin): enhance error handling and test coverage in gintrace --- .../gin-gonic/gin/otelgin/gintrace.go | 1 + .../gin/otelgin/test/gintrace_test.go | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index 7582d5a4184..18ede5d6b9e 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -95,6 +95,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { span.SetAttributes(semconv.HTTPStatusCode(status)) } if len(c.Errors) > 0 { + span.SetStatus(codes.Error, "gin error") for _, err := range c.Errors { span.RecordError(err.Err) } 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 23cb66723ce..ad416a240dd 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 @@ -236,6 +236,27 @@ func TestSpanStatus(t *testing.T) { assert.Equal(t, tc.wantSpanStatus, sr.Ended()[0].Status().Code, "should only set Error status for HTTP statuses >= 500") }) } + + t.Run("The status code is 200, but an error is returned", func(t *testing.T) { + sr := tracetest.NewSpanRecorder() + provider := sdktrace.NewTracerProvider( + sdktrace.WithSpanProcessor(sr), + ) + + router := gin.New() + router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider))) + router.GET("/", func(c *gin.Context) { + _ = c.Error(errors.New("something went wrong")) + c.JSON(http.StatusOK, nil) + }) + + router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/", nil)) + + require.Len(t, sr.Ended(), 1) + assert.Equal(t, codes.Error, sr.Ended()[0].Status().Code) + require.Len(t, sr.Ended()[0].Events(), 1) + assert.Contains(t, sr.Ended()[0].Events()[0].Attributes, attribute.String("exception.message", "something went wrong")) + }) } func TestSpanName(t *testing.T) { From a69013679ad26e66adf5a584c012fe46a19edd38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flc=E3=82=9B?= Date: Tue, 26 Nov 2024 10:25:37 +0800 Subject: [PATCH 18/20] Update instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go Co-authored-by: Tyler Yahn --- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ad416a240dd..b480f7db9d8 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 @@ -198,7 +198,7 @@ func TestError(t *testing.T) { // verify the error events events := span.Events() - assert.Len(t, events, 2) + require.Len(t, events, 2) assert.Equal(t, "exception", events[0].Name) assert.Contains(t, events[0].Attributes, attribute.String("exception.type", "*errors.errorString")) assert.Contains(t, events[0].Attributes, attribute.String("exception.message", "oh no one")) From 724c6d0a73013eb8a66d6046c7c88e7074c65efa Mon Sep 17 00:00:00 2001 From: Flc Date: Tue, 26 Nov 2024 17:51:54 +0800 Subject: [PATCH 19/20] chore(otelgin): Enhance gintrace tests with span status checks and error descriptions --- .../github.com/gin-gonic/gin/otelgin/gintrace.go | 2 +- .../gin-gonic/gin/otelgin/test/gintrace_test.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index 18ede5d6b9e..ef666d37cac 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -95,7 +95,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { span.SetAttributes(semconv.HTTPStatusCode(status)) } if len(c.Errors) > 0 { - span.SetStatus(codes.Error, "gin error") + span.SetStatus(codes.Error, c.Errors.String()) for _, err := range c.Errors { span.RecordError(err.Err) } 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 b480f7db9d8..099fdd484d0 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,8 +18,6 @@ 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" @@ -28,6 +26,9 @@ 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() { @@ -165,6 +166,8 @@ func TestTrace200(t *testing.T) { 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) + assert.Empty(t, span.Status().Description) } func TestError(t *testing.T) { @@ -208,6 +211,7 @@ func TestError(t *testing.T) { // server errors set the status assert.Equal(t, codes.Error, span.Status().Code) + assert.Equal(t, "Error #01: oh no one\nError #02: oh no two\n", span.Status().Description) } func TestSpanStatus(t *testing.T) { From 19ceaecc1638f675419158be29e26fffb1f3cd14 Mon Sep 17 00:00:00 2001 From: Flc Date: Tue, 26 Nov 2024 17:53:08 +0800 Subject: [PATCH 20/20] chore(otelgin): Enhance gintrace tests with span status checks and error descriptions --- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 099fdd484d0..f63d1955f0e 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() {