Name Sentry transactions after the route they matched (closes #185)
check / check (push) Waiting to run
check / check (push) Waiting to run
sentryhttp names each transaction after the concrete path, which the BeforeSendTransaction hook has to withhold, so with tracing on the transactions would fall into one Sentry group per method, such as POST /(redacted). A middleware installed beside every sentryhttp handler now renames the transaction to the method and chi route pattern once the request is routed, and sets its source to route. The hook keeps a route-sourced name and withholds every other name as before. A path that matches nothing at the top level keeps the withheld name; one that matches no route inside a route group, or that the group's middleware turns away, is named after the group's pattern, such as GET /pages/*. Tracing stays off. Model: opus-5-5
This commit was merged in pull request #516.
This commit is contained in:
@@ -16,6 +16,7 @@ import (
|
||||
"github.com/go-chi/chi"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/server"
|
||||
)
|
||||
|
||||
@@ -130,11 +131,15 @@ func (c sentryCase) capture(t *testing.T) []*sentry.Event {
|
||||
// router mirrors the one ordering these tests depend on, over the two
|
||||
// route patterns they need: a recovering middleware outside, then the
|
||||
// sentryhttp handler registered with Use and Repanic set, exactly as
|
||||
// routes.go orders the two. The bare recover stands in for
|
||||
// Middleware.Recoverer, which holds that outer slot in production; it
|
||||
// is here only to keep panic stacks out of the test output. That the
|
||||
// production one really does catch what sentryhttp re-raises is
|
||||
// pinned separately, by TestSentryStillSeesAPanic.
|
||||
// routes.go orders the two. It leaves out nameSentryTransaction,
|
||||
// which routes.go installs inside the sentryhttp handler, so a
|
||||
// transaction captured here keeps the SDK's name and shows what the
|
||||
// hook does with a transaction that missed the rename. The bare
|
||||
// recover stands in for Middleware.Recoverer, which holds that outer
|
||||
// slot in production; it is here only to keep panic stacks out of the
|
||||
// test output. That the production one really does catch what
|
||||
// sentryhttp re-raises is pinned separately, by
|
||||
// TestSentryStillSeesAPanic.
|
||||
func (c sentryCase) router() http.Handler {
|
||||
handler := func(_ http.ResponseWriter, r *http.Request) {
|
||||
// This call is what drains the body tee and fills the
|
||||
@@ -353,8 +358,9 @@ func TestSentryScrub_KeepsTheRoutingContext(t *testing.T) {
|
||||
// Span.doFinish captures with a nil hint, so BeforeSendTransaction
|
||||
// gets one with no context and no request: the route pattern is out of
|
||||
// reach and both the URL and the SDK-built transaction name have to
|
||||
// fall back. Tracing is off in this service, so no transaction event
|
||||
// is produced today; the hook is a floor against that changing.
|
||||
// fall back. This router has no nameSentryTransaction, so this is the
|
||||
// floor under a transaction that missed the rename;
|
||||
// TestSentryTransaction_NamedByRoute covers the routes as they ship.
|
||||
func TestSentryScrub_RedactsTheTransactionDispatch(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -376,6 +382,83 @@ func TestSentryScrub_RedactsTheTransactionDispatch(t *testing.T) {
|
||||
assert.Equal(t, "POST /(redacted)", event.Transaction)
|
||||
}
|
||||
|
||||
// TestSentryTransaction_NamedByRoute serves requests through the real
|
||||
// route tree with tracing on, which the service leaves off. A routed
|
||||
// request's transaction is named after its route pattern, so Sentry
|
||||
// can group transactions by route, and the entrypoint UUID in the
|
||||
// receiver path appears nowhere in the event. A path that matches
|
||||
// nothing at the top level has no pattern, and its name is still
|
||||
// withheld; one that matches no route inside a route group is named
|
||||
// after the group's pattern.
|
||||
func TestSentryTransaction_NamedByRoute(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// newTestEnv leaves the receiver rate limit at zero, which
|
||||
// refuses every request before it reaches the receiver.
|
||||
env := newTestEnvWithConfig(t, &config.Config{
|
||||
DataDir: t.TempDir(),
|
||||
Environment: config.EnvironmentDev,
|
||||
ReceiverRateLimit: 10,
|
||||
})
|
||||
|
||||
router := server.NewRouterWithSentryForTest(
|
||||
t, env.log, env.cfg, env.mw, env.hnd,
|
||||
)
|
||||
|
||||
for _, tc := range []struct {
|
||||
method, path, want string
|
||||
}{
|
||||
{
|
||||
http.MethodPost,
|
||||
"/h/" + sentryReceiverUUID,
|
||||
"POST /h/{uuid}",
|
||||
},
|
||||
// An admin page route group, which installs its own
|
||||
// sentryhttp handler.
|
||||
{http.MethodGet, "/pages/login", "GET /pages/login"},
|
||||
{http.MethodGet, "/pages/unrouted", "GET /pages/*"},
|
||||
{
|
||||
http.MethodPost,
|
||||
"/h/" + sentryReceiverUUID + "/unrouted",
|
||||
"POST /(redacted)",
|
||||
},
|
||||
} {
|
||||
t.Run(tc.want, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
transport := &captureTransport{}
|
||||
|
||||
opts := server.SentryClientOptionsForTest(
|
||||
"https://public@sentry.invalid/1",
|
||||
"webhooker-test",
|
||||
)
|
||||
opts.Transport = transport
|
||||
opts.EnableTracing = true
|
||||
opts.TracesSampleRate = 1.0
|
||||
|
||||
client, err := sentry.NewClient(opts)
|
||||
require.NoError(t, err)
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
sentry.SetHubOnContext(
|
||||
context.Background(),
|
||||
sentry.NewHub(client, sentry.NewScope()),
|
||||
),
|
||||
tc.method, tc.path, nil,
|
||||
)
|
||||
router.ServeHTTP(httptest.NewRecorder(), req)
|
||||
|
||||
event := onlyEvent(t, transport.events)
|
||||
require.Equal(t, "transaction", event.Type)
|
||||
|
||||
assert.Equal(t, tc.want, event.Transaction)
|
||||
assert.NotContains(
|
||||
t, marshalEvent(t, event), sentryReceiverUUID,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSentryScrub_TransactionDispatchIsUnscrubbedWithoutTheHook pins
|
||||
// that dispatch's premise the same way, since it is the one the
|
||||
// service does not exercise today.
|
||||
|
||||
Reference in New Issue
Block a user