Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

otelgin: add routeName argument to SpanNameFormatter function #5741

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254)
- Add the `WithSource` option to the `go.opentelemetry.io/contrib/bridges/otelslog` log bridge to set the `code.*` attributes in the log record that includes the source location where the record was emitted. (#6253)
- Add `ContextWithStartTime` and `StartTimeFromContext` to `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`, which allows setting the start time using go context. (#6137)
- Add `routeName` argument to `SpanNameFormatter` function in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#5741)

### Fixed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
if cfg.SpanNameFormatter == nil {
spanName = c.FullPath()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be considered to adjust to the form of: method path.

} 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)
Expand Down
12 changes: 7 additions & 5 deletions instrumentation/github.com/gin-gonic/gin/otelgin/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ type Filter func(*http.Request) bool
// gin.Context has FullPath() method, which returns a matched route full path.
type GinFilter func(*gin.Context) 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 {
Expand Down Expand Up @@ -84,9 +84,11 @@ 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 {
// 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion to add:

if f != nil {
	....
}

})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package test

import (
"errors"
"fmt"
"html/template"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -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) {
Expand All @@ -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, tc.wantSpanName, sr.Ended()[0].Name(), "span name not correct")
Expand All @@ -186,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)
}),
),
)
Expand All @@ -208,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"))
Expand Down
Loading