Group reported errors by workflow activity and error type - #3
Merged
Conversation
Errors are captured from the interceptor rather than from the code that failed, so every event carries the same stack trace. Sentry groups on that stack by default, so unrelated failures collapse into one issue: its title describes whichever error fired most recently, and it reopens forever because something in the bundle keeps failing. Report errors with an explicit fingerprint of the event, the workflow or activity that failed and the error type. It is applied before the configured scope, so `WithConfigureSentryScope` can still override it. Panics keep the default grouping, as a recovered panic carries a real stack.
There was a problem hiding this comment.
Pull request overview
This PR changes how Sentry issues are grouped for Temporal errors reported by the interceptor by setting an explicit, deterministic fingerprint (event name + workflow/activity type + error type) to prevent unrelated failures from collapsing into a single Sentry issue due to identical interceptor stack traces.
Changes:
- Add
ReportErrorInput.Fingerprint()to derive a stable Sentry fingerprint usingtemporal.ApplicationError.Type()(with a Go-type fallback). - Apply the default fingerprint in
ReportErrorbefore any user-providedWithConfigureSentryScopeconfiguration (so user scope can override it). - Add focused unit tests to validate fingerprint composition and ensure it reaches the sent Sentry event; document the behavior in the README (including correcting the scope function signature).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| sentry_activities.go | Computes and applies a deterministic Sentry fingerprint for reported errors before user scope configuration. |
| README.md | Documents the new default issue grouping behavior and shows how to override it via WithConfigureSentryScope. |
| fingerprint_test.go | Adds tests covering fingerprint derivation and verifying it is applied to captured Sentry events (and that panics keep default grouping). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This interceptor captures errors from
ExecuteActivity/ExecuteWorkflow, not from the code that failed. Go errors don't carry stack traces of their own, sosentry-gofalls back to the stack of the reporting goroutine — which is always this interceptor. Every event from every consumer therefore carries an identical stack trace.Sentry's default grouping hashes that stack, and the stack takes precedence over the error message (Sentry's own
grouping-inforeports the message as "ignored because stacktrace takes precedence"). So unrelated failures produce the same hash and collapse into one issue. Because Sentry files an event into a group matching any of its hashes, one shared hash bridges two different errors, each pulls in the next, and the group snowballs.We hit this in production on a service using this pattern: a single Sentry issue accumulated 23 distinct grouping hashes spanning six unrelated activities. Two consequences, both bad:
This makes the default a deterministic fingerprint instead:
The error type comes from
temporal.ApplicationError.Type(), falling back to the Go type of the error. Both are low cardinality and stable, so this splits issues without risking issue explosion.Notes on the approach
WithConfigureSentryScopestill overrides it. Users who want different grouping keep full control, and there's no new option to learn.ConfigureSentryScopeFuncdoesn't receive the error, so it can't compute an error-type-aware fingerprint itself — hence setting it inReportError, where the error is in hand, rather than widening the exported function type.ReportPanicis deliberately untouched. A recovered panic carries a real stack trace, so default grouping already works there. Covered by a test so it stays that way.Impacted areas
ReportErroronly. Grouping of already-reported errors is unaffected — Sentry applies fingerprints from the next event onward and does not retroactively split existing issues, so consumers upgrading will see new issues appear alongside the old bundle, which they can then resolve once.Steps to reproduce or test
Development
make test— newfingerprint_test.gocovers the fingerprint for activity errors, workflow errors, the non-ApplicationErrorfallback, anApplicationErrorwith an empty type, and the case where neither info struct is set; that two error types on the same activity and the same error type on two activities produce different keys; and — through a realsentry.Clientwith a recording transport — that the fingerprint reaches the sent event, that a custom scope overrides it, and that panics stay on default grouping.nil.make lintclean (0 issues).make testgreen, including all pre-existing tests.QA
N/A — library change, no runtime surface of its own.
Checklist
Breaking Changeif it applies.ConfigureScopecall per reported error, on a path that is already doing a network capture.Deploy notes
None — no migrations, no config, no new dependencies.
One thing worth flagging for the reviewer: I also corrected the
WithConfigureSentryScopeexample in the README, which was missing theeventName stringparameter thatConfigureSentryScopeFuncactually takes. It's adjacent to my change rather than part of it, but the same example is now the one users are pointed at for overriding the fingerprint, so a wrong signature there would send them straight into a compile error. Happy to split it out if you'd rather keep this PR to the code.