From 2f21ae0607078ed319f2ebcc9e9235e9433eb00c Mon Sep 17 00:00:00 2001 From: Qingsheng Ma Date: Thu, 6 Jun 2024 12:45:37 +0800 Subject: [PATCH 1/3] add routeName argument to SpanNameFormatter function --- CHANGELOG.md | 1 + .../github.com/gin-gonic/gin/otelgin/gintrace.go | 2 +- .../github.com/gin-gonic/gin/otelgin/option.go | 12 +++++++----- .../gin-gonic/gin/otelgin/test/gintrace_test.go | 6 ++++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 245710157ac..0c8e541bcd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `go.opentelemetry.io/contrib/config` add support to configure periodic reader interval and timeout. (#5661) - Add support to configure views when creating MeterProvider using the config package. (#5654) +- Add `routeName` argument to `SpanNameFormatter` function in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. ### Fixed diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go index df0fbf9c52b..eddb924cc61 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go @@ -66,7 +66,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc { if cfg.SpanNameFormatter == nil { spanName = c.FullPath() } else { - spanName = cfg.SpanNameFormatter(c.Request) + spanName = cfg.SpanNameFormatter(c.FullPath(), c.Request) } if spanName == "" { spanName = fmt.Sprintf("HTTP %s route not found", c.Request.Method) diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/option.go b/instrumentation/github.com/gin-gonic/gin/otelgin/option.go index 113576ca00a..c3df9de84c3 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/option.go +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/option.go @@ -23,8 +23,8 @@ type config struct { // be traced. A Filter must return true if the request should be traced. type Filter func(*http.Request) bool -// SpanNameFormatter is used to set span name by http.request. -type SpanNameFormatter func(r *http.Request) string +// SpanNameFormatter is used to set span name by http.Request. +type SpanNameFormatter func(routeName string, r *http.Request) string // Option specifies instrumentation configuration options. type Option interface { @@ -70,9 +70,11 @@ func WithFilter(f ...Filter) Option { }) } -// WithSpanNameFormatter takes a function that will be called on every -// request and the returned string will become the Span Name. -func WithSpanNameFormatter(f func(r *http.Request) string) Option { +// WithSpanNameFormatter specifies a function to use for generating a custom span +// name. By default, the route name (path template or regexp) is used. The route +// name is provided so you can use it in the span name without needing to +// duplicate the logic for extracting it from the request. +func WithSpanNameFormatter(f func(routeName string, r *http.Request) string) Option { return optionFunc(func(c *config) { c.SpanNameFormatter = f }) 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 afd6460983a..4ced082d6e2 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 @@ -7,6 +7,7 @@ package test import ( "errors" + "fmt" "html/template" "net/http" "net/http/httptest" @@ -160,7 +161,8 @@ func TestSpanName(t *testing.T) { wantSpanName string }{ {"/user/1", nil, "/user/:id"}, - {"/user/1", func(r *http.Request) string { return r.URL.Path }, "/user/1"}, + {"/user/1", func(routeName string, r *http.Request) string { return r.URL.Path }, "/user/1"}, + {"/user/1", func(routeName string, r *http.Request) string { return fmt.Sprintf("%s %s", r.Method, routeName) }, "GET /user/:id"}, } for _, tc := range testCases { t.Run(tc.requestPath, func(t *testing.T) { @@ -171,7 +173,7 @@ func TestSpanName(t *testing.T) { router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider), otelgin.WithSpanNameFormatter(tc.spanNameFormatter))) router.GET("/user/:id", func(c *gin.Context) {}) - router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", tc.requestPath, nil)) + router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, tc.requestPath, nil)) require.Len(t, sr.Ended(), 1, "should emit a span") assert.Equal(t, sr.Ended()[0].Name(), tc.wantSpanName, "span name not correct") From bd5f50c3475d19749adac126a413b6fe757953bd Mon Sep 17 00:00:00 2001 From: Jason Ma Date: Thu, 6 Jun 2024 16:43:43 +0800 Subject: [PATCH 2/3] 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 6b8fdb251e2..0140e0909c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `go.opentelemetry.io/contrib/config` add support to configure periodic reader interval and timeout. (#5661) - Add support to configure views when creating MeterProvider using the config package. (#5654) -- Add `routeName` argument to `SpanNameFormatter` function in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. +- Add `routeName` argument to `SpanNameFormatter` function in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#5741) ### Fixed From e9e288a13a595d7a4b78e723f0808fa0a6e3696d Mon Sep 17 00:00:00 2001 From: Qingsheng Ma Date: Fri, 7 Jun 2024 00:09:04 +0800 Subject: [PATCH 3/3] fix unit test --- .../github.com/gin-gonic/gin/otelgin/test/gintrace_test.go | 6 +++--- 1 file changed, 3 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 408f87c0ed4..dba680d4fc0 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 @@ -188,8 +188,8 @@ func TestHTTPRouteWithSpanNameFormatter(t *testing.T) { router := gin.New() router.Use(otelgin.Middleware("foobar", otelgin.WithTracerProvider(provider), - otelgin.WithSpanNameFormatter(func(r *http.Request) string { - return r.URL.Path + otelgin.WithSpanNameFormatter(func(routeName string, r *http.Request) string { + return fmt.Sprintf("%s %s", r.Method, routeName) }), ), ) @@ -210,7 +210,7 @@ func TestHTTPRouteWithSpanNameFormatter(t *testing.T) { spans := sr.Ended() require.Len(t, spans, 1) span := spans[0] - assert.Equal(t, "/user/123", span.Name()) + assert.Equal(t, "GET /user/:id", span.Name()) assert.Equal(t, oteltrace.SpanKindServer, span.SpanKind()) attr := span.Attributes() assert.Contains(t, attr, attribute.String("http.method", "GET"))