feat(ourlogs): Preliminary breadcrumb to log conversion #4479
Conversation
This converts breadcrumbs in a fairly simplistic way into our log format, behind a sample-rate to mitigate load when rolling this out (and also the existing enable flag).
k-fish
force-pushed
the
feat/ourlogs/convert-breadcrumbs-to-logs
branch
from
e07396f to
b678b22
Compare
There was a problem hiding this comment.
This will generate an almost infinite amount of duplicate log entries.
Breadcrumbs are not cleared for event emissions, every event can and will contain the same log lines.
Worst case on every error you receive, you extract the same 100 log lines. That's a pretty bad amplification vector.
Assuming this were to be enabled in prod, yes. Fortunately we're only enabling it for a specific project on s4s at this time. (project flag incoming). More generally, deduplication will be handled by an experimental sdk flag that send the events here. Noted though, we don't want a flood in the off chance of a bad sdk setup, so we should guard against others sending here so I'll update it to block if a context value isn't found. |
| #[metastructure(tag = "sentry_logs")] | ||
| OurLogs(Box<OurLogsContext>), |
There was a problem hiding this comment.
Are these hard to remove later or is the deprecation sticky? If they are I'm probably going to delay this a week until Abhi is back and we can make sure everyone is fine with the context on the sdk side.
There was a problem hiding this comment.
Removing the variant from the code is not a problem. After removing it, relay will still forward the context as if it were a user-defined context.
| #[metastructure(tag = "sentry_logs")] | ||
| OurLogs(Box<OurLogsContext>), |
There was a problem hiding this comment.
Removing the variant from the code is not a problem. After removing it, relay will still forward the context as if it were a user-defined context.
Summary
This converts breadcrumbs in a fairly simplistic way into our log format, behind a sample-rate to mitigate load when rolling this out (and also the existing enable flag).
Other:
Needs:
Related: