Repository navigation
Change to DEBUG (from INFO) 'Not re-registering MeterRegistry ...' - #2914
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. 🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
| Step hopefullyNull = meterRegistries.putIfAbsent(meterRegistry, newStep); | ||
| if (hopefullyNull != null) { | ||
| logger.info("Not re-registering MeterRegistry as it is already registered from another compound meter registry: {}", meterRegistry); | ||
| logger.debug("Not re-registering MeterRegistry as it is already registered from another compound meter registry: {}", meterRegistry); |
There was a problem hiding this comment.
If my understanding is correct, this code path is executed basically on every interaction with a meter, which in turn causes the large amounts of logging.
I feel like DEBUG-level is still to much: It would clutter the logs we use for support cases.
I would move it to trace or even remove this statement completely, because I don't know hat it would be used for.
There was a problem hiding this comment.
Can't comment on unchanged code, so adding this here:
if (step >= 0 && step < 1000) {
logger.warn("Not registering unsupported step interval of {} milliseconds (1 seconds is the minimum supported) for Micrometer MeterRegistry: {}", step, meterRegistry);
return;
}
This will also cause a spam of warnings IINM. Maybe we should make sure to log only once per registry?
There was a problem hiding this comment.
And another one I'm not sure about:
logger.warn("Identified Micrometer SimpleConfig: {}", config);
Shouldn't this one be debug?
There was a problem hiding this comment.
yes agree, will move to trace. I don't want to add additional memory cost to track which have been logged, trace is fine. It is useful when there is a problem tracing which meters are being processed.
Also agree that other should be debug
|
@JonasKunz @jackshirazi Hey i am using apm-agent-attach/1.35.0, latest version of apm-agent-attach, still getting above logs undefinetely |
|
@JonasKunz @jackshirazi any idea when this fix will be released? We are currently blocked on upgrading until that time - presumably we will be seeing a |
|
Targeting this month |
What does this PR do?
Change to DEBUG (from INFO) 'Not re-registering MeterRegistry ...' message as it is too verbose