[dynatrace/v2] reduce log verbosity #5306
Conversation
| "Metadata discrepancy detected:\n" + "original metadata:\t%s\n" + "tried to set new:\t%s\n" | ||
| + "Metadata for metric key %s will not be sent.", | ||
| previousMetadataLine, metadataLine, key)); | ||
| logger.info( |
There was a problem hiding this comment.
I'm fine with the change if you think it's best, but I just wanted to point out this means this will be logged at info level every time, whereas before it would be logged at warn level once and then logged at debug level. This feels more verbose to me now.
There was a problem hiding this comment.
Hmm, I rethought it again, and for the metadata one, you're right - it might be too verbose. I changed this one to debug always. If someone is missing metadata, they would turn the log level to DEBUG and would then find out why their metadata is missing.
There was a problem hiding this comment.
Another option would be to make an InfoThenDebugLogger if that will be better for users realizing they have a metadata issue at all. Most users, I imagine, do not have debug logs enabled until they are aware of an issue, but they may not become aware of this issue without enabling debug logs.
There was a problem hiding this comment.
I also considered the InfoThenDebugLogger. However, this is a very specific use case, and I did not want to add a whole new chunk of code just for that. I assume it would live in micrometer-core alongside the WarnThenDebugLogger.
To your point about not turning on debug logging: I planned to add a note to the docs suggesting turning on debug logging if you are missing metadata. My idea was that:
- User sees that metadata is missing for a metric
- User either immediately turns on debug logging or
- User goes to the docs, sees the note about using debug logging when metadata is missing, and then turns it on.
I am not set on this, though. If you think the InfoThenDebugLogger could be useful elsewhere, I am happy to contribute it, but I didn't want to introduce more code to maintain if it only solves a niche problem.
There was a problem hiding this comment.
I would add it as an implementation detail (package private or an inner class) here until we have a need for it elsewhere, and only then we would move it to micrometer-commons. Let me know whether you want to do that or leave it as debug level.
There was a problem hiding this comment.
I think I'd prefer to leave it as debug for now. If we added an InfoThenDebugLogger, it would still only log the first occurrence of metadata being inconsistent (e.g. if there are two separate meters with inconsistent data).
shakuzen
added
enhancement
This PR simplifies the logging in the Dynatrace exporter v2. Most of the loggers' log levels were reduced to INFO. This allows collecting logs (mainly at the INFO level) without drowning out business code WARN and ERROR logs (e.g., when filtering for only ERROR logs).