fix(handler): group captured log messages by call site, not message content#164
fix(handler): group captured log messages by call site, not message content#164cat-ph wants to merge 2 commits into
Conversation
posthog-elixir Compliance ReportDate: 2026-07-09 19:59:27 UTC ✅ All Tests Passed!46/46 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
|
That's an interesting topic! To my knowledge, there's no clear cut solution to tracking log messages which can potentially contain dynamic values. It's always a bit of a trade off between undergrouping and overgrouping. Using call site for fingerprinting will work great for very broad messages like Initially I decided against using call site for fingerprinting because I though undergrouping is easier to addres:
My understanding is that overgrouping would be much harder to address:
WDYT? |
|
thanks for looking @martosaur! yeah, I think it's a bit tricky because we've seen (recently at least) a lot of I was thinking I wonder if there's also the idea of having a |
|
oooh, if you have access to the aggregate data, is there a way to check if those unique fingerprints have any large clusters? The thing is, most people I know learn fairly quickly to not interpolate dynamic values in logs, but there is a couple of very popular libraries that are guilty of that. For example I would expect |
|
I think we should avoid putting the message on the exception type, imo the grouping decision should be handled server side and behavior between SDKs should be aligned. If users want to control grouping on the SDK they can still rely on the I also agree that overgrouping should be avoided, we released a new fingerprint algorithm that better take the exception value into account (and strip dynamic fields) when there is no stacktrace. It's not bullet proof but that should help as the message is the only thing we can use to split two error inside a single function. |
💡 Motivation and Context
Exception events built from plain log messages (
capture_levelcaptures without acrash_reason) currently use the formatted message as the exceptiontype. Log messages routinely interpolate dynamic values — ids, URLs, inspected terms — so every distinct message produces a distinct type. Error tracking fingerprints the exception type verbatim, which means a singleLogger.error("Failed to crawl #{url}")call site creates a separate issue per event instead of one issue for the call site. In production data this makes captured-log exception events group dramatically worse than crash-derived ones: the large majority of such events end up with a unique fingerprint, and server-side masking can't help because the volatile parts are often plain words (URLs, entity names), not just numbers.This PR keys the type on the logging call site instead:
Logger error (MyApp.Crawler.update_stats/2), derived from themfalog metadata (the same metadata #152 uses for the synthetic stacktrace frame). The full message is still sent invalue, so nothing is lost from the issue description or event detail. Whenmfametadata is absent (e.g.Logger.bare_log/3), behavior is unchanged.The crash-reporter entry of chained exceptions (a log message with an attached
crash_reason) intentionally keeps its message-derived type: that entry names the issue, and for e.g. Bandit/Cowboy request crashes the banner (** (RuntimeError) oops) is a much better issue title than the logging call site inside the HTTP server would be. The marker that gates this never leaves the handler.💚 How did you test it?
mix test(316 passed),mix format,mix credo --strict(no new findings).📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
$exception_fingerprint(bypasses server grouping rules), and applying the call-site type to crash-reporter chain entries (would regress issue titles for request crashes).typefor message captures, so existing issues for such events will re-key once — those groups are the ones currently splitting per event, so the practical cost is nil.