getsentry
fix: logging when not in debug mode
Originally I noticed that the condition in SentryLog short circuits if the SDK is not in debug mode, so we will never print even error logs in a production app.
Then I noticed that the statically assigned value of diagnosticLevel in SentryLog is always overwritten by the hardcoded default in SentryOptions. This PR also centralizes the default, modifies the log level when SentryOptions.debug is modified, removes the separate tracking of debug state in SentryLog and thus simplifies the check for whether to log, since it now only needs to look at log level.
#skip-changelog
Hmm just occurred to me that setting debug to false after setting diagnosticLevel will overwrite it to error again. That needs to be fixed. diagnosticLevel probably shouldn’t even be exposed publicly, but that’s a different story.
@brustolin @bruno-garcia I just wanted to confirm that this change in behavior is desired. I was fixing the broken tests and noticed that one is testConfigureWithoutDebug_PrintsNothing, which tells me that this was deliberate. Do we really not want to print any warning or error messages in production? How do we get logs from customers today to help debug issues?
@brustolin @bruno-garcia I just wanted to confirm that this change in behavior is desired. I was fixing the broken tests and noticed that one is
testConfigureWithoutDebug_PrintsNothing, which tells me that this was deliberate. Do we really not want to print any warning or error messages in production? How do we get logs from customers today to help debug issues?
I like this changes for debug in development purpose, I don't see how this logs in production would help, is not like the final user would dump the device log and sent it to the developer. AFAIK (dont quote me on this 😉) this logs are not send automatically to ituneConnect app report thing.
You have a point about end users, that would require a more integrated approach. This would be immediately helpful for any other build that doesn't set debug to true, that could be test/profile builds or TestFlight betas.
@brustolin I updated the doc comment in SentryOptions, but that led me to make a few other changes after thinking about things some more. I deprecated diagnosticLevel so most of the changes in the latest commit deal with that. Let me know what you think.
@brustolin I updated the doc comment in SentryOptions, but that led me to make a few other changes after thinking about things some more. I deprecated
diagnosticLevelso most of the changes in the latest commit deal with that. Let me know what you think.
I had a conversation about this with @bruno-garcia last week and if I understood it right we really want diagnosticLevel. I kind like it, sometimes during debug having a very verbose SDK is a pain. The power to change granularity is nice.
I think this should be closed now, now that we've discussed the philosophy behind how we do logging. Nothing is actually broken there, so shouldn't be messed with.