Skip to content

Commit

Permalink
otelgin: Using c.FullPath() to set http.route attribute (#5734)
Browse files Browse the repository at this point in the history
As `spanName` can be customized with `SpanNameFormatter`, so the
`spanName` may not be the same with `http.route`, e.g. the `spanName`
can be `GET /users/:id`, but the `http.route` is `/users/:id`.
This PR using
[`c.FullPath()`](https://github.com/gin-gonic/gin/blob/v1.10.0/context.go#L165-L173)
to set `http.route` attribute in `otelgin` to keep `http.route` not
affected by `SpanNameFormatter`.

---------

Co-authored-by: Chester Cheung <cheung.zhy.csu@gmail.com>
  • Loading branch information
NeoCN and hanyuancheung authored Jun 6, 2024
1 parent c932948 commit 0430e80
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- The superfluous `response.WriteHeader` call in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` when the response writer is flushed. (#5634)
- Custom attributes targeting metrics recorded by the `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` are not ignored anymore. (#5129)
- Use `c.FullPath()` method to set `http.route` attribute in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#5734)

### Deprecated

Expand Down
4 changes: 1 addition & 3 deletions instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
ctx := cfg.Propagators.Extract(savedCtx, propagation.HeaderCarrier(c.Request.Header))
opts := []oteltrace.SpanStartOption{
oteltrace.WithAttributes(semconvutil.HTTPServerRequest(service, c.Request)...),
oteltrace.WithAttributes(semconv.HTTPRoute(c.FullPath())),
oteltrace.WithSpanKind(oteltrace.SpanKindServer),
}
var spanName string
Expand All @@ -70,9 +71,6 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
}
if spanName == "" {
spanName = fmt.Sprintf("HTTP %s route not found", c.Request.Method)
} else {
rAttr := semconv.HTTPRoute(spanName)
opts = append(opts, oteltrace.WithAttributes(rAttr))
}
ctx, span := tracer.Start(ctx, spanName, opts...)
defer span.End()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,42 @@ func TestSpanName(t *testing.T) {
}
}

func TestHTTPRouteWithSpanNameFormatter(t *testing.T) {
sr := tracetest.NewSpanRecorder()
provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr))

router := gin.New()
router.Use(otelgin.Middleware("foobar",
otelgin.WithTracerProvider(provider),
otelgin.WithSpanNameFormatter(func(r *http.Request) string {
return r.URL.Path
}),
),
)
router.GET("/user/:id", func(c *gin.Context) {
id := c.Param("id")
_, _ = c.Writer.Write([]byte(id))
})

r := httptest.NewRequest("GET", "/user/123", nil)
w := httptest.NewRecorder()

// do and verify the request
router.ServeHTTP(w, r)
response := w.Result()
require.Equal(t, http.StatusOK, response.StatusCode)

// verify traces look good
spans := sr.Ended()
require.Len(t, spans, 1)
span := spans[0]
assert.Equal(t, "/user/123", span.Name())
assert.Equal(t, oteltrace.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"))
}

func TestHTML(t *testing.T) {
sr := tracetest.NewSpanRecorder()
provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr))
Expand Down

0 comments on commit 0430e80

Please sign in to comment.