From 93ed0b6156ecaf4545b004c05cfa20e4caed2f1d Mon Sep 17 00:00:00 2001 From: Reinaldy Rafli Date: Sun, 25 Aug 2024 17:14:42 +0700 Subject: [PATCH 1/4] fix(http): use route name from 'r.Pattern' instead --- http/route_name_below_go1.22.go | 9 ++++++ http/route_name_below_go1.22_test.go | 30 +++++++++++++++++++ http/route_name_go1.22.go | 22 ++++++++++++++ http/route_name_go1.22_test.go | 45 ++++++++++++++++++++++++++++ http/sentryhttp.go | 3 +- 5 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 http/route_name_below_go1.22.go create mode 100644 http/route_name_below_go1.22_test.go create mode 100644 http/route_name_go1.22.go create mode 100644 http/route_name_go1.22_test.go diff --git a/http/route_name_below_go1.22.go b/http/route_name_below_go1.22.go new file mode 100644 index 000000000..b947ee825 --- /dev/null +++ b/http/route_name_below_go1.22.go @@ -0,0 +1,9 @@ +//go:build !go1.22 + +package sentryhttp + +import "net/http" + +func getHTTPSpanName(r *http.Request) string { + return r.Method + " " + r.URL.Path +} diff --git a/http/route_name_below_go1.22_test.go b/http/route_name_below_go1.22_test.go new file mode 100644 index 000000000..94420230d --- /dev/null +++ b/http/route_name_below_go1.22_test.go @@ -0,0 +1,30 @@ +//go:build !go1.22 + +package sentryhttp + +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) + } + }) + } +} diff --git a/http/route_name_go1.22.go b/http/route_name_go1.22.go new file mode 100644 index 000000000..e4d7be6f8 --- /dev/null +++ b/http/route_name_go1.22.go @@ -0,0 +1,22 @@ +//go:build go1.22 + +package sentryhttp + +import ( + "net/http" + "strings" +) + +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 +} diff --git a/http/route_name_go1.22_test.go b/http/route_name_go1.22_test.go new file mode 100644 index 000000000..62aa8403b --- /dev/null +++ b/http/route_name_go1.22_test.go @@ -0,0 +1,45 @@ +//go:build go1.22 + +package sentryhttp + +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) + } + }) + } +} diff --git a/http/sentryhttp.go b/http/sentryhttp.go index 0eb998c57..d78c6b2c5 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -4,7 +4,6 @@ package sentryhttp import ( "context" - "fmt" "net/http" "time" @@ -102,7 +101,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { } transaction := sentry.StartTransaction(ctx, - fmt.Sprintf("%s %s", r.Method, r.URL.Path), + getHTTPSpanName(r), options..., ) transaction.SetData("http.request.method", r.Method) From ab919b65b85593f1fe24c8f2cc30124edcfb5be4 Mon Sep 17 00:00:00 2001 From: Reinaldy Rafli Date: Tue, 27 Aug 2024 07:47:12 +0700 Subject: [PATCH 2/4] fix(http): bump build version to 1.23 --- http/{route_name_below_go1.22.go => route_name_below_go1.23.go} | 2 +- ...ame_below_go1.22_test.go => route_name_below_go1.23_test.go} | 2 +- http/{route_name_go1.22.go => route_name_go1.23.go} | 2 +- http/{route_name_go1.22_test.go => route_name_go1.23_test.go} | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename http/{route_name_below_go1.22.go => route_name_below_go1.23.go} (86%) rename http/{route_name_below_go1.22_test.go => route_name_below_go1.23_test.go} (96%) rename http/{route_name_go1.22.go => route_name_go1.23.go} (95%) rename http/{route_name_go1.22_test.go => route_name_go1.23_test.go} (98%) diff --git a/http/route_name_below_go1.22.go b/http/route_name_below_go1.23.go similarity index 86% rename from http/route_name_below_go1.22.go rename to http/route_name_below_go1.23.go index b947ee825..d74a5c2e5 100644 --- a/http/route_name_below_go1.22.go +++ b/http/route_name_below_go1.23.go @@ -1,4 +1,4 @@ -//go:build !go1.22 +//go:build !go1.23 package sentryhttp diff --git a/http/route_name_below_go1.22_test.go b/http/route_name_below_go1.23_test.go similarity index 96% rename from http/route_name_below_go1.22_test.go rename to http/route_name_below_go1.23_test.go index 94420230d..4bf209d95 100644 --- a/http/route_name_below_go1.22_test.go +++ b/http/route_name_below_go1.23_test.go @@ -1,4 +1,4 @@ -//go:build !go1.22 +//go:build !go1.23 package sentryhttp diff --git a/http/route_name_go1.22.go b/http/route_name_go1.23.go similarity index 95% rename from http/route_name_go1.22.go rename to http/route_name_go1.23.go index e4d7be6f8..9f0ed36af 100644 --- a/http/route_name_go1.22.go +++ b/http/route_name_go1.23.go @@ -1,4 +1,4 @@ -//go:build go1.22 +//go:build go1.23 package sentryhttp diff --git a/http/route_name_go1.22_test.go b/http/route_name_go1.23_test.go similarity index 98% rename from http/route_name_go1.22_test.go rename to http/route_name_go1.23_test.go index 62aa8403b..ac891531b 100644 --- a/http/route_name_go1.22_test.go +++ b/http/route_name_go1.23_test.go @@ -1,4 +1,4 @@ -//go:build go1.22 +//go:build go1.23 package sentryhttp From ea993f8dae0b5811c364abc516714c9a70f322ff Mon Sep 17 00:00:00 2001 From: Reinaldy Rafli Date: Wed, 28 Aug 2024 07:42:21 +0700 Subject: [PATCH 3/4] ref(internal): move GetHTTPSpanName to internal package --- http/route_name_below_go1.23.go | 9 --------- http/sentryhttp.go | 3 ++- internal/traceutils/route_name_below_go1.23.go | 10 ++++++++++ .../traceutils}/route_name_below_go1.23_test.go | 4 ++-- {http => internal/traceutils}/route_name_go1.23.go | 5 +++-- .../traceutils}/route_name_go1.23_test.go | 10 +++++----- negroni/sentrynegroni.go | 4 ++-- 7 files changed, 24 insertions(+), 21 deletions(-) delete mode 100644 http/route_name_below_go1.23.go create mode 100644 internal/traceutils/route_name_below_go1.23.go rename {http => internal/traceutils}/route_name_below_go1.23_test.go (83%) rename {http => internal/traceutils}/route_name_go1.23.go (67%) rename {http => internal/traceutils}/route_name_go1.23_test.go (73%) diff --git a/http/route_name_below_go1.23.go b/http/route_name_below_go1.23.go deleted file mode 100644 index d74a5c2e5..000000000 --- a/http/route_name_below_go1.23.go +++ /dev/null @@ -1,9 +0,0 @@ -//go:build !go1.23 - -package sentryhttp - -import "net/http" - -func getHTTPSpanName(r *http.Request) string { - return r.Method + " " + r.URL.Path -} diff --git a/http/sentryhttp.go b/http/sentryhttp.go index d78c6b2c5..f6fa5445a 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -8,6 +8,7 @@ import ( "time" "github.com/getsentry/sentry-go" + "github.com/getsentry/sentry-go/internal/traceutils" ) // The identifier of the HTTP SDK. @@ -101,7 +102,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { } transaction := sentry.StartTransaction(ctx, - getHTTPSpanName(r), + traceutils.GetHTTPSpanName(r), options..., ) transaction.SetData("http.request.method", r.Method) diff --git a/internal/traceutils/route_name_below_go1.23.go b/internal/traceutils/route_name_below_go1.23.go new file mode 100644 index 000000000..09f03aaa9 --- /dev/null +++ b/internal/traceutils/route_name_below_go1.23.go @@ -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 +} diff --git a/http/route_name_below_go1.23_test.go b/internal/traceutils/route_name_below_go1.23_test.go similarity index 83% rename from http/route_name_below_go1.23_test.go rename to internal/traceutils/route_name_below_go1.23_test.go index 4bf209d95..ece97bce6 100644 --- a/http/route_name_below_go1.23_test.go +++ b/internal/traceutils/route_name_below_go1.23_test.go @@ -1,6 +1,6 @@ //go:build !go1.23 -package sentryhttp +package traceutils import ( "net/http" @@ -16,7 +16,7 @@ func TestGetHTTPSpanName(t *testing.T) { }{ { name: "Without Pattern", - got: getHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}}), + got: GetHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}}), want: "GET /", }, } diff --git a/http/route_name_go1.23.go b/internal/traceutils/route_name_go1.23.go similarity index 67% rename from http/route_name_go1.23.go rename to internal/traceutils/route_name_go1.23.go index 9f0ed36af..160675be9 100644 --- a/http/route_name_go1.23.go +++ b/internal/traceutils/route_name_go1.23.go @@ -1,13 +1,14 @@ //go:build go1.23 -package sentryhttp +package traceutils import ( "net/http" "strings" ) -func getHTTPSpanName(r *http.Request) string { +// 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. diff --git a/http/route_name_go1.23_test.go b/internal/traceutils/route_name_go1.23_test.go similarity index 73% rename from http/route_name_go1.23_test.go rename to internal/traceutils/route_name_go1.23_test.go index ac891531b..f7bb47cbf 100644 --- a/http/route_name_go1.23_test.go +++ b/internal/traceutils/route_name_go1.23_test.go @@ -1,6 +1,6 @@ //go:build go1.23 -package sentryhttp +package traceutils import ( "net/http" @@ -16,22 +16,22 @@ func TestGetHTTPSpanName(t *testing.T) { }{ { name: "Without Pattern", - got: getHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}}), + 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}"}), + 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}"}), + 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/"}), + got: GetHTTPSpanName(&http.Request{Method: "GET", URL: &url.URL{Path: "/"}, Pattern: "example.com/"}), want: "GET example.com/", }, } diff --git a/negroni/sentrynegroni.go b/negroni/sentrynegroni.go index 2a49695d5..29fd9dd56 100644 --- a/negroni/sentrynegroni.go +++ b/negroni/sentrynegroni.go @@ -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" ) @@ -87,7 +87,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) From 17b7d0e40021bb94c3326749fd6b204fc09f1403 Mon Sep 17 00:00:00 2001 From: Reinaldy Rafli Date: Wed, 28 Aug 2024 20:05:31 +0700 Subject: [PATCH 4/4] chore: changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfcd9ae08..c92de878c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) ## 0.28.1