Skip to content

fix(logger): wide event source points at StartWideEvent caller + tests#6269

Open
Flo4604 wants to merge 1 commit into
mainfrom
logger-wide-event-source
Open

fix(logger): wide event source points at StartWideEvent caller + tests#6269
Flo4604 wants to merge 1 commit into
mainfrom
logger-wide-event-source

Conversation

@Flo4604
Copy link
Copy Markdown
Member

@Flo4604 Flo4604 commented May 23, 2026

The wide event system emits its accumulated log from End(), which is almost always a deferred call inside middleware. Calling logger.Error / logger.Info from event.go meant slog captured the PC of those lines, so every wide event's source attribute resolved to pkg/logger/event.go:103 which is useless for finding the actual handler.

Capture the PC at StartWideEvent instead, store it on the Event, and build the slog.Record manually in End() with that PC. Source now points at the middleware/handler that opened the event.

Add unit tests:

pkg/logger: faultHandler enrichment (fault errors, plain errors, no errors, multiple errors, slog.Attr-wrapped errors, multi-sink fan-out) and source PC attribution for logger.Error aliases and wide events.
svc/api/internal/middleware: errors middleware emits a 500 log with workspaceId/requestId/code/publicMessage/http.* group, stashes the full internal error chain on the session, and stays silent on 4xx.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dashboard Ready Ready Preview, Comment May 23, 2026 10:15pm
design Ready Ready Preview, Comment May 23, 2026 10:15pm

Request Review

@mintlify
Copy link
Copy Markdown
Contributor

mintlify Bot commented May 23, 2026

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
engineering 🟢 Ready View Preview May 23, 2026, 10:12 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@Flo4604 Flo4604 changed the title Logger wide event source fix(logger): wide event source points at StartWideEvent caller + tests May 23, 2026
The wide event system emits its accumulated log from End(), which is
almost always a deferred call inside middleware. Calling logger.Error /
logger.Info from event.go meant slog captured the PC of those lines, so
every wide event's source attribute resolved to pkg/logger/event.go:103
— useless for finding the actual handler.

Capture the PC at StartWideEvent instead, store it on the Event, and
build the slog.Record manually in End() with that PC. Source now points
at the middleware/handler that opened the event.

Tests:
- pkg/logger/loggertest: shared CaptureHandler + FlatAttrs/PCFrame helpers
  so callers across services can assert on emitted log records without
  redefining a handler in every test file.
- pkg/logger: faultHandler enrichment (fault errors, plain errors, no
  errors, multiple errors, slog.Attr-wrapped errors, multi-sink fan-out)
  and source PC attribution for logger.Error aliases and wide events.
- svc/api/internal/middleware: errors middleware emits a 500 log with
  workspaceId/requestId/code/publicMessage/http.* group, stashes the
  full internal error chain on the session, and stays silent on 4xx.
Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — initial review of the wide-event source fix and its accompanying test infrastructure. The change correctly redirects the emitted log's source attribute at the StartWideEvent caller instead of the internal Handle call site, and the new tests pin down both the PC plumbing and the surrounding fault-enrichment behavior end-to-end.

  • Capture caller PC at StartWideEventruntime.Callers(2, pcs[:]) stores the caller's PC on Event.pc; End() now builds a slog.Record manually so the captured PC survives instead of being overwritten by logger.Error/Info inside event.go.
  • End() rewrite — switched from variadic []any to a typed []slog.Attr, added an explicit Enabled short-circuit, and dispatched via logger.Handler().Handle(ctx, r) to preserve the captured PC. Existing semantics around message ("error" vs supplied) and errors.<n> grouping are preserved.
  • New pkg/logger/loggertest package — shared CaptureHandler plus Install, Last, Find, Snapshot/Since, FlatAttrs, and PCFrame helpers. Lives outside pkg/logger so production code can't depend on it, and the doc is explicit about the global-state lifecycle (no teardown, scope via snapshot indices).
  • pkg/logger unit testsfaultHandler enrichment across fault errors, plain errors, no-error records, multi-error precedence, multi-sink fan-out, and slog.Any-wrapped errors; PC attribution for the logger.Error alias and for wide events; double-End() idempotency.
  • svc/api/internal/middleware unit tests — first coverage for WithErrorHandling: 500 emits the full workspaceId/requestId/code/publicMessage/http.* attribute set with the fault chain attached via the global enrichment handler; the full internal error chain is stashed via SetInternalError; 4xx (key-not-found) is silent.
  • Bazelgo_test targets added in both pkg/logger/BUILD.bazel and svc/api/internal/middleware/BUILD.bazel, plus a new pkg/logger/loggertest/BUILD.bazel.

Verified the PC skip count (2 is correct for a direct caller of StartWideEvent), the faultHandler does not double-enrich wide events because errors live inside the errors group (top-level Record.Attrs walk only), and MultiHandler.Enabled short-circuits don't suppress CaptureHandler (always returns true). The discarded runtime.Callers return value is benign — pcs[0] stays zero on the unlikely empty result, and the wide-event test explicitly asserts NotZero to catch that path.

Pullfrog  | View workflow run | Using Claude Opus𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant