If tracing is ever enabled, every Sentry transaction will collapse into one bucket named POST /(redacted) #185

New Issue

clawbot · 2026-08-18T02:34:32+02:00

clawbot commented

2026-08-18 02:34:32 +02:00

Raised by the independent review of #181. Deliberately NOT milestoned 1.0.0 — tracing is off in this service, so nothing is broken today.

#179 stopped the receiver's capability UUID reaching Sentry by substituting the chi route pattern. The pattern is reachable on the error dispatch, via the request the SDK carries on hint.Context. It is not reachable on the transaction dispatch: Span.doFinish calls hub.CaptureEvent(event) with a nil hint, which client.go:620-622 replaces with an empty &EventHint{} — non-nil, Context nil, no request. So BeforeSendTransaction correctly falls back.

The consequence, which is correct behaviour and still a problem: the moment someone enables tracing, every transaction gets the name POST /(redacted) and the URL scheme://host/(redacted). Sentry groups transactions by name, so the entire service becomes one bucket and the performance data is worthless.

That is not a defect in #181 — the fallback has to be safe, and safe means withholding. It is a trap laid for whoever turns tracing on, who will see one meaningless bucket and may well "fix" it by reverting the redaction and putting the capability back.

The fix, for whoever enables tracing

A middleware registered after routing that sets the transaction name from chi.RouteContext(r.Context()).RoutePattern() — at which point the name is correct at source and the hook's fallback never fires for it. sentry.TransactionFromContext(r.Context()) gives the span to rename.

Definition of done

Only actionable once tracing is wanted; until then this is a documented trap.

Enabling tracing yields transaction names carrying the route pattern, not (redacted) and not the concrete path.
The BeforeSendTransaction fallback stays as the floor — the new middleware must be an improvement on it, not a replacement for it, so a dispatch that somehow misses the middleware still withholds.
A test asserting a transaction event carries the pattern and no capability UUID.
The README's Sentry section stops describing transaction names as always redacted, once they are not.

Two smaller notes from the same review, recorded here rather than lost

sentryTransactionName cuts on the first space, so a name that is not METHOD /path and contains a space becomes firstWord /(redacted). Safe, and unreachable while the SDK is the only thing setting the name — but a hand-set name would meet it.
The URL rebuild in #181 also strips userinfo, as an unclaimed side effect. Worth keeping deliberately if anyone touches that code.

Implementation requirements

Branch from next, PR based on next, single commit, title ending (closes #N).
Do not modify TODO.md (see #112).
Run make bootstrap in a fresh clone before gating — browser assets are fetched at build time, and make lint needs Docker.
Gate on make check plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.

Raised by the independent review of https://git.eeqj.de/sneak/webhooker/pulls/181. Deliberately NOT milestoned 1.0.0 — tracing is off in this service, so nothing is broken today. https://git.eeqj.de/sneak/webhooker/issues/179 stopped the receiver's capability UUID reaching Sentry by substituting the chi route pattern. The pattern is reachable on the **error** dispatch, via the request the SDK carries on `hint.Context`. It is **not** reachable on the **transaction** dispatch: `Span.doFinish` calls `hub.CaptureEvent(event)` with a nil hint, which `client.go:620-622` replaces with an empty `&EventHint{}` — non-nil, `Context` nil, no request. So `BeforeSendTransaction` correctly falls back. The consequence, which is correct behaviour and still a problem: the moment someone enables tracing, **every** transaction gets the name `POST /(redacted)` and the URL `scheme://host/(redacted)`. Sentry groups transactions by name, so the entire service becomes one bucket and the performance data is worthless. That is not a defect in https://git.eeqj.de/sneak/webhooker/pulls/181 — the fallback has to be safe, and safe means withholding. It is a trap laid for whoever turns tracing on, who will see one meaningless bucket and may well "fix" it by reverting the redaction and putting the capability back. ## The fix, for whoever enables tracing A middleware registered **after** routing that sets the transaction name from `chi.RouteContext(r.Context()).RoutePattern()` — at which point the name is correct at source and the hook's fallback never fires for it. `sentry.TransactionFromContext(r.Context())` gives the span to rename. ## Definition of done Only actionable once tracing is wanted; until then this is a documented trap. - Enabling tracing yields transaction names carrying the route pattern, not `(redacted)` and not the concrete path. - The `BeforeSendTransaction` fallback stays as the floor — the new middleware must be an improvement on it, not a replacement for it, so a dispatch that somehow misses the middleware still withholds. - A test asserting a transaction event carries the pattern and no capability UUID. - The README's Sentry section stops describing transaction names as always redacted, once they are not. ## Two smaller notes from the same review, recorded here rather than lost - `sentryTransactionName` cuts on the first space, so a name that is not `METHOD /path` and contains a space becomes `firstWord /(redacted)`. Safe, and unreachable while the SDK is the only thing setting the name — but a hand-set name would meet it. - The URL rebuild in https://git.eeqj.de/sneak/webhooker/pulls/181 also strips userinfo, as an unclaimed side effect. Worth keeping deliberately if anyone touches that code. ## Implementation requirements - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Do not modify `TODO.md` (see https://git.eeqj.de/sneak/webhooker/issues/112). - Run `make bootstrap` in a fresh clone before gating — browser assets are fetched at build time, and `make lint` needs Docker. - Gate on `make check` plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.

clawbot self-assigned this 2026-08-18 02:34:32 +02:00

clawbot referenced this issue from a commit

2026-08-18 02:42:59 +02:00

Send the chi route pattern to Sentry, not the concrete path (closes #179)

clawbot referenced this issue

2026-08-18 10:50:50 +02:00

Milestone 1.0.0: internet-facing readiness #111

Sign in to join this conversation.