DataDog / datadogpy

The Datadog Python library
https://datadoghq.com/
Other
609 stars 302 forks source link

[base] demote noisy log lines to debug level #729

Closed yashsarma closed 2 years ago

yashsarma commented 2 years ago

What does this PR do?

We currently use datadogpy library v0.44.0 and noticed tons of log volume coming from the base class file especially in the flush thread flow, causing log volume spike and masking useful information that would otherwise be gleaned from our logs. This PR downgrades the log level to debug, these lines serve little purpose in my humble opinion and could easily be replaced with a counter.

What inspired you to submit this pull request?

This change isn't a feature or a bug fix, but follows a process of better log hygiene.

Description of the Change

please refer to the questions above.

Alternate Designs

Would be replace these log lines with simple statsd counter incr.

Possible Drawbacks

None

Verification Process

What process did you follow to verify that your change has the desired effects?

Describe the actions you performed (including scripts, commands you ran, etc.), and describe the results you observed.

N/A

Additional Notes

N/A

Release Notes

Reduce log noise caused by repeated info logs in the statsd buffer flush flow.

Review checklist (to be filled by reviewers)

sgnn7 commented 2 years ago

Hey @yashsarma! These logging lines are very informative as they indicate fundamental properties around the way that the DogStatsd library processes the data for all metric/event/etc submissions and are generally intended to be located in extremely low-invocation paths per instance:

The expectation is that you would have only 1-2 of these lines hit for each object which would be critical for high-level understanding of how the DogStatsd instance is operating. If you're seeing a lot of these, it may be good to figure out your use case here and why you are seeing a lot of them before we apply any changes.

sgnn7 commented 2 years ago

@yashsarma I'm also curious about your counter comment - can you elaborate on why/how would a counter replace these log lines?

tiwilliam commented 2 years ago

I find these lines annoying. These were added when buffering was made default in #670, and was intended to signal when you had explicitly turned it off. Now buffering is disabled by default, and these will always be printed on import, even if you create your own instance and enabling buffering, making the log line confusing since you might actually be using a buffered instance.

sgnn7 commented 2 years ago

@tiwilliam

was intended to signal when you had explicitly turned it off

It was added for a display on startup of the current mode (since it was inteded to be changed) and on any explicit toggling of the setting but I do see your point since the default was never switched in the final releases.


@tiwilliam / @yashsarma:

Given that this may have a bit bigger impact than it was anticipated originally or implied from the initial issue description we can merge this change for now. I think if/when buffering is enabled by default again as an option we may need to revisit this decision though.