Skip to content

Group reported errors by workflow activity and error type - #3

Merged
Kamefrede merged 1 commit into
masterfrom
bugfix/default-error-fingerprint
Aug 17, 2026
Merged

Group reported errors by workflow activity and error type#3
Kamefrede merged 1 commit into
masterfrom
bugfix/default-error-fingerprint

Conversation

@goncalvesnelson

Copy link
Copy Markdown
Contributor

Description

This interceptor captures errors from ExecuteActivity / ExecuteWorkflow, not from the code that failed. Go errors don't carry stack traces of their own, so sentry-go falls 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-info reports 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:

  • The issue title is whatever fired most recently, so the on-call page never describes what is actually broken.
  • The issue can't be closed. Ours was resolved five times over three months and auto-reopened as a regression every time, because a different error in the bundle kept firing.

This makes the default a deterministic fingerprint instead:

<event name> / <workflow or activity type> / <error type>
ActivityExecuteActivity / CreateSchedule              / validation_failed
ActivityExecuteActivity / CreateAutoTradeTransaction  / forbidden
WorkflowExecuteWorkflow / SetupAutomatedTransaction   / unknown

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

  • Applied before the configured scope, so WithConfigureSentryScope still overrides it. Users who want different grouping keep full control, and there's no new option to learn.
  • No API change. ConfigureSentryScopeFunc doesn't receive the error, so it can't compute an error-type-aware fingerprint itself — hence setting it in ReportError, where the error is in hand, rather than widening the exported function type.
  • ReportPanic is 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.
  • The fix does not give you useful stack traces — it only stops the useless one from being the grouping key. The stack is still the interceptor's. Fixing that requires the error to carry frames from where it was created, which is a concern for whatever error library a consumer uses, not for this interceptor.

Impacted areas

ReportError only. 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 — new fingerprint_test.go covers the fingerprint for activity errors, workflow errors, the non-ApplicationError fallback, an ApplicationError with 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 real sentry.Client with a recording transport — that the fingerprint reaches the sent event, that a custom scope overrides it, and that panics stay on default grouping.
  • Written test-first and confirmed failing before the implementation. The event-level test was re-run with the wiring removed to confirm it guards: without it the event's fingerprint is nil.
  • make lint clean (0 issues). make test green, including all pre-existing tests.

QA

N/A — library change, no runtime surface of its own.

Checklist

  • Add label Breaking Change if it applies.
  • Commits are atomic and logically separated.
  • Performance implications have been considered — one extra ConfigureScope call per reported error, on a path that is already doing a network capture.
  • Security implications have been considered — the fingerprint is built from type names and the interceptor event name, never from error messages or request payloads, so no user data enters it.
  • The new and updated code has good coverage.
  • The README file, if required, has been updated.

Deploy notes

None — no migrations, no config, no new dependencies.

One thing worth flagging for the reviewer: I also corrected the WithConfigureSentryScope example in the README, which was missing the eventName string parameter that ConfigureSentryScopeFunc actually 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.

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.
Copilot AI lite review requested due to automatic review settings August 15, 2026 12:59
@goncalvesnelson goncalvesnelson added the bug Something isn't working label Aug 15, 2026
@goncalvesnelson goncalvesnelson self-assigned this Aug 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 using temporal.ApplicationError.Type() (with a Go-type fallback).
  • Apply the default fingerprint in ReportError before any user-provided WithConfigureSentryScope configuration (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.

@Kamefrede Kamefrede left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Kamefrede
Kamefrede merged commit 80b97b7 into master Aug 17, 2026
8 checks passed
@Kamefrede
Kamefrede deleted the bugfix/default-error-fingerprint branch August 17, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants