Skip to content

Commit

Permalink
refactor(otelgin): refine span name formatting and deprecate `SpanNam…
Browse files Browse the repository at this point in the history
…eFormatter` option
  • Loading branch information
flc1125 committed Nov 28, 2024
1 parent 57ec64d commit ef7f3ab
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 22 deletions.
36 changes: 24 additions & 12 deletions instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
package otelgin // import "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin"

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

"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"
Expand Down Expand Up @@ -71,16 +72,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
oteltrace.WithAttributes(semconv.HTTPRoute(c.FullPath())),
oteltrace.WithSpanKind(oteltrace.SpanKindServer),
}
var spanName string
if cfg.SpanNameFormatter == nil {
spanName = c.FullPath()
} else {
spanName = cfg.SpanNameFormatter(c.Request)
}
if spanName == "" {
spanName = fmt.Sprintf("HTTP %s route not found", c.Request.Method)
}
ctx, span := tracer.Start(ctx, spanName, opts...)
ctx, span := tracer.Start(ctx, spanNameFormatter(c), opts...)
defer span.End()

// pass the span through the request context
Expand All @@ -103,6 +95,26 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
}
}

// spanNameFormatter is used to set span name by gin.Context.
func spanNameFormatter(c *gin.Context) string {
method, path := strings.ToUpper(c.Request.Method), c.FullPath()
if !slices.Contains([]string{
http.MethodGet, http.MethodHead,
http.MethodPost, http.MethodPut,
http.MethodPatch, http.MethodDelete,
http.MethodConnect, http.MethodOptions,
http.MethodTrace,
}, method) {
method = "HTTP"
}

if path != "" {
return method + " " + path
}

return method
}

// HTML will trace the rendering of the template as a child of the
// span in the given context. This is a replacement for
// gin.Context.HTML function - it invokes the original function after
Expand Down
10 changes: 5 additions & 5 deletions instrumentation/github.com/gin-gonic/gin/otelgin/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type config struct {
Propagators propagation.TextMapPropagator
Filters []Filter
GinFilters []GinFilter
SpanNameFormatter SpanNameFormatter
SpanNameFormatter SpanNameFormatter // Deprecated: since 0.58.0, remove in 0.59.0.
}

// Filter is a predicate used to determine whether a given http.request should
Expand All @@ -31,6 +31,7 @@ type Filter func(*http.Request) bool
type GinFilter func(*gin.Context) bool

// SpanNameFormatter is used to set span name by http.request.
// Deprecated: since 0.58.0, remove in 0.59.0.
type SpanNameFormatter func(r *http.Request) string

// Option specifies instrumentation configuration options.
Expand Down Expand Up @@ -86,8 +87,7 @@ func WithGinFilter(f ...GinFilter) 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 {
return optionFunc(func(c *config) {
c.SpanNameFormatter = f
})
// Deprecated: since 0.58.0, remove in 0.59.0.
func WithSpanNameFormatter(func(r *http.Request) string) Option {
return optionFunc(func(*config) {})
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func TestTrace200(t *testing.T) {
spans := sr.Ended()
require.Len(t, spans, 1)
span := spans[0]
assert.Equal(t, "/user/:id", span.Name())
assert.Equal(t, "GET /user/:id", span.Name())
assert.Equal(t, trace.SpanKindServer, span.SpanKind())
attr := span.Attributes()
assert.Contains(t, attr, attribute.String("net.host.name", "foobar"))
Expand Down Expand Up @@ -193,7 +193,7 @@ func TestError(t *testing.T) {
spans := sr.Ended()
require.Len(t, spans, 1)
span := spans[0]
assert.Equal(t, "/server_err", span.Name())
assert.Equal(t, "GET /server_err", span.Name())
attr := span.Attributes()
assert.Contains(t, attr, attribute.String("net.host.name", "foobar"))
assert.Contains(t, attr, attribute.Int("http.status_code", http.StatusInternalServerError))
Expand Down Expand Up @@ -268,8 +268,8 @@ func TestSpanName(t *testing.T) {
spanNameFormatter otelgin.SpanNameFormatter
wantSpanName string
}{
{"/user/1", nil, "/user/:id"},
{"/user/1", func(r *http.Request) string { return r.URL.Path }, "/user/1"},
{"/user/1", nil, "GET /user/:id"},
{"/user/1", func(r *http.Request) string { return r.URL.Path }, "GET /user/:id"},
}
for _, tc := range testCases {
t.Run(tc.requestPath, func(t *testing.T) {
Expand Down Expand Up @@ -317,7 +317,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, trace.SpanKindServer, span.SpanKind())
attr := span.Attributes()
assert.Contains(t, attr, attribute.String("http.method", "GET"))
Expand Down

0 comments on commit ef7f3ab

Please sign in to comment.