Skip to content

Add experimental support for decoupling tracer error and debug logs#718

Closed
ericmustin wants to merge 1 commit intoDataDog:masterfrom
ericmustin:559_decouple_error_and_debug_logs
Closed

Add experimental support for decoupling tracer error and debug logs#718
ericmustin wants to merge 1 commit intoDataDog:masterfrom
ericmustin:559_decouple_error_and_debug_logs

Conversation

@ericmustin
Copy link
Contributor

What does this PR do?

This PR addresses the use case introduced in Issue 599 for when a user wants to forward tracer errors to their logger but ignore debug messages. Currently the logger requires debug and error keys for the logger object and will always compute the debug message even if a custom logger is passed with debug set to a noop function. This PR adds experimental support for an onlyErrors configuration option which disables the debug message computation and output, if both the tracer's logger is enabled and experimental.onlyErrors is set to true

Motivation

It has been hanging around in the Open Issues for awhile and it bothered me that the logging wasn't as flexible as it ought to be. I wanted to avoid changing the format of user's existing custom loggers, so adding this via the experimental options felt like a low friction way to help resolve the issue for users going forward while maintaining backward compatibility for existing implementations.

Additional Notes

Thanks for taking a look! Let me know what y'all think or if it's worth the hassle of adding, happy to address any feedback. Not terribly familiar with Typescript and a bit rusty with Node in general, so apologies if I've missed something here. That being said, seems to help a few users out there who want to reduce the noise in their logs while still forwarding relevant tracer errors, and seemed like low hanging fruit, so figured I'd give it a shot.

@rochdev
Copy link
Member

rochdev commented Oct 22, 2019

Thanks for the PR!

The idea makes sense in general, but I'd prefer to make it more flexible as we plan to add new log levels in the future. Maybe we could add a logLevel option, with a corresponding DD_LOG_LEVEL or DD_TRACE_LOG_LEVEL env var? This would make it possible to control which log levels generate log records, as opposed to the the existing log level of the logger that only filters them out after the fact.

What do you think?

@ericmustin
Copy link
Contributor Author

@rochdev thanks for taking a look. Yup i think that's a better approach and allows for more flexibility going forward. I can work on putting that together, will close this branch and push up a new one when I have something ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants