Skip to content

Commit

Permalink
fix(http): use route name from 'r.Pattern' instead (#875)
Browse files Browse the repository at this point in the history
* fix(http): use route name from 'r.Pattern' instead

* fix(http): bump build version to 1.23

* ref(internal): move GetHTTPSpanName to internal package

* chore: changelog entry

---------

Co-authored-by: Michi Hoffmann <cleptric@users.noreply.github.com>
  • Loading branch information
aldy505 and cleptric committed Sep 3, 2024
1 parent e5d46d5 commit 7085a1d
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Add trace origin to span data ([#849](https://github.com/getsentry/sentry-go/pull/849))
- Add ability to skip frames in stacktrace ([#852](https://github.com/getsentry/sentry-go/pull/852))
- Remove Martini integration ([#861](https://github.com/getsentry/sentry-go/pull/861))
- Use value from http.Request.Pattern as HTTP server span name ([#875](https://github.com/getsentry/sentry-go/pull/875))
- Fix closure functions name grouping ([#877](https://github.com/getsentry/sentry-go/pull/877))


Expand Down
4 changes: 2 additions & 2 deletions http/sentryhttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ package sentryhttp

import (
"context"
"fmt"
"net/http"
"time"

"github.com/getsentry/sentry-go"
"github.com/getsentry/sentry-go/internal/traceutils"
)

// The identifier of the HTTP SDK.
Expand Down Expand Up @@ -102,7 +102,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
}

transaction := sentry.StartTransaction(ctx,
fmt.Sprintf("%s %s", r.Method, r.URL.Path),
traceutils.GetHTTPSpanName(r),
options...,
)
transaction.SetData("http.request.method", r.Method)
Expand Down
10 changes: 10 additions & 0 deletions internal/traceutils/route_name_below_go1.23.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//go:build !go1.23

package traceutils

import "net/http"

// GetHTTPSpanName grab needed fields from *http.Request to generate a span name for `http.server` span op.
func GetHTTPSpanName(r *http.Request) string {
return r.Method + " " + r.URL.Path
}
30 changes: 30 additions & 0 deletions internal/traceutils/route_name_below_go1.23_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//go:build !go1.23

package traceutils

import (
"net/http"
"net/url"
"testing"
)

func TestGetHTTPSpanName(t *testing.T) {
tests := []struct {
name string
got string
want string
}{
{
name: "Without Pattern",
got: GetHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}}),
want: "GET /",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.got != tt.want {
t.Errorf("got %q; want %q", tt.got, tt.want)
}
})
}
}
23 changes: 23 additions & 0 deletions internal/traceutils/route_name_go1.23.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//go:build go1.23

package traceutils

import (
"net/http"
"strings"
)

// GetHTTPSpanName grab needed fields from *http.Request to generate a span name for `http.server` span op.
func GetHTTPSpanName(r *http.Request) string {
if r.Pattern != "" {
// If value does not start with HTTP methods, add them.
// The method and the path should be separated by a space.
if parts := strings.SplitN(r.Pattern, " ", 2); len(parts) == 1 {
return r.Method + " " + r.Pattern
}

return r.Pattern
}

return r.Method + " " + r.URL.Path
}
45 changes: 45 additions & 0 deletions internal/traceutils/route_name_go1.23_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
//go:build go1.23

package traceutils

import (
"net/http"
"net/url"
"testing"
)

func TestGetHTTPSpanName(t *testing.T) {
tests := []struct {
name string
got string
want string
}{
{
name: "Without Pattern",
got: GetHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}}),
want: "GET /",
},
{
name: "Pattern with method",
got: GetHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}, Pattern: "POST /foo/{bar}"}),
want: "POST /foo/{bar}",
},
{
name: "Pattern without method",
got: GetHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}, Pattern: "/foo/{bar}"}),
want: "GET /foo/{bar}",
},
{
name: "Pattern without slash",
got: GetHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}, Pattern: "example.com/"}),
want: "GET example.com/",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.got != tt.want {
t.Errorf("got %q; want %q", tt.got, tt.want)
}
})
}
}
4 changes: 2 additions & 2 deletions negroni/sentrynegroni.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package sentrynegroni

import (
"context"
"fmt"
"net/http"
"time"

"github.com/getsentry/sentry-go"
"github.com/getsentry/sentry-go/internal/traceutils"
"github.com/urfave/negroni"
)

Expand Down Expand Up @@ -71,7 +71,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.Ha
// We don't mind getting an existing transaction back so we don't need to
// check if it is.
transaction := sentry.StartTransaction(ctx,
fmt.Sprintf("%s %s", r.Method, r.URL.Path),
traceutils.GetHTTPSpanName(r),
options...,
)
transaction.SetData("http.request.method", r.Method)
Expand Down

0 comments on commit 7085a1d

Please sign in to comment.