fix: group integration webhook 500s in Sentry and raise level to warning#5087
Draft
cursor[bot] wants to merge 1 commit into
Draft
fix: group integration webhook 500s in Sentry and raise level to warning#5087cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
The Sentry HTTP error capture used the raw URL path as the issue title,
so every integration webhook 500 created a new Sentry issue keyed on the
integration UUID (e.g. HTTP 500
/api/v1/integrations/e931a088-90c5-4fa2-9b30-fc7d9d586723/webhook).
This both fragmented the data and reported the error at the default Info
level, which hid real 5xx incidents in Sentry's default filters.
Redact UUID-shaped path segments before forming the Sentry message so
that all integration webhook 500s collapse into a single issue (HTTP 500
/api/v1/integrations/{id}/webhook), set the event level to Warning so
5xx events surface alongside other server warnings, and attach the
original path as an http.path tag so we can still drill into specific
failing integrations.
|
👋 Commands for maintainers:
|
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
1 participant
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.
Context
A Sentry issue (
HTTP 500 /api/v1/integrations/e931a088-90c5-4fa2-9b30-fc7d9d586723/webhook, link) was reported atinfolevel for a single integration UUID. Inspectingpkg/public/middleware/logging.goshowed two problems with how server errors get reported to Sentry:captureHTTPErroruses the concreter.URL.Pathas the Sentry message, so every distinct integration UUID produces a new Sentry issue. The/api/v1/integrations/{integrationID}/webhookendpoint (used by GitHub and Dash0 integrations) generates one Sentry issue per failing integration, fragmenting the data and making aggregate trends invisible.hub.CaptureMessage(...)is called without setting a level, so 5xx events come in asinfo. They get filtered out of Sentry's default error views, which explains why this regression slipped in as a low-signal event.Change
HTTP 500 /api/v1/integrations/{id}/webhook. Other endpoints (e.g./api/v1/canvases/{id}/nodes/{id}) benefit from the same grouping.Warningfor captured 5xx responses so they surface in the standard error filters.http.pathtag (on both the error and panic capture paths) so engineers can still drill into which specific integration / canvas / node was failing without losing aggregation.This is purely an observability fix; behavior of the integration webhook handlers themselves is unchanged. Once landed, future 500s on
/api/v1/integrations/{integrationID}/webhookwill aggregate into one warning-level Sentry issue with the specific UUIDs visible in the tags/request context, which will make it much easier to diagnose the underlying integration handler bug.Test plan
pkg/public/middleware/logging_test.gocoverredactPathIDs(canonical UUIDs, multiple UUIDs in one path, uppercase UUIDs, paths without UUIDs) andgroupingPathForRequest(the integration webhook scenario and a non-UUID path).TestShouldCaptureHTTPErrorcases still pass.go test ./pkg/public/middleware/...passes locally for the targeted tests.