feat: put sentry_debug! logs behind feature flag
Description
feature/chore to allow for enabling and disabling [sentry] debug logs. I was getting my container and other log systems filled with logs like this:
[sentry] [LogsBatcher] Flushing 31 logs
[sentry] Get response: `{"id":"f42674c3591b41dfa855d216760a9ea8"}`
Totally get having the debug logs there and I understand their use but I wanted the ability to disable them.
Perhaps another approach could be to make them traces at the trace/debug level if tracing flag is enabled that way people could opt in with level filters?
For now I've just feature flagged the macro to a no-op when used if the debug_logs flag is not enabled.
Happy for input or thoughts but I would personally love this.
Issues
N/A
This is basically how it worked before I changed this, when we had the debug-logs feature flag.
I really like the fact that this allows us to have a no-op implementation in case you don't want to log, and I think this is something I overlooked when removing the feature flag.
This way of implementing it would allow us to be much more verbose with logging. Admittedly there've been cases where I wanted to add debug logs for some part of the code but decided not to do it because it was in a "hot path" - without the approach proposed here, you would always need to grab the current Hub -> client -> check if debug is true regardless.
This approach however has 2 downsides:
- more confusing to users: we would have 3 very similar feature flags:
log(thelogcrate integration),logs(enables capturing logs through the batcher logger which runs on its own thread),debug-logs(this feature)- if we decide to do this, we need to document it, and decide what to do with the existing
debugfield ofClientOptions. Either we remove it in favor of this flag or we make it conditional on this flag.
- if we decide to do this, we need to document it, and decide what to do with the existing
- if you want to emit debug logs from an integration crate (which ideally you would, considering it would also be zero-cost in general) then you would also need to define the
debug-logsfeature there and enable it if it's enabled insentry-core(and vice versa maybe)
Yep my bad definitely overlooked the debug:true on my end. Must have missed it in the documentation most likely as I was trying to search for a way to get rid of the logbatcher logs specifically as this was the only debug log our team really noticed. I do stand by the implementation but as you say its mainly now a performance thing and a small one at that. on the whole feature flag being confusing perhaps a name of debug-prints or alike could be better if this was to be deemed as valuable enough?
Cheers for the help and getting onto the review quickly!