count bound and enable stats #2381
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before contributing, please read our contributing guidelines and code of conduct.
Overview
newrelic-java-agent/newrelic-agent/src/main/java/com/newrelic/agent/tracers/metricname/MetricNameFormats.java
Lines 21 to 23 in 4b53e4f
private static final Cache<MNFKey, MetricNameFormat> cmsToMnf = Caffeine.newBuilder()
.executor(Runnable::run)
.build();
If a customer were to create enough segments with extremely dynamic names, this cache could grow to an unreasonable size and appear to be a memory leak.
Related Github Issue
#1825
Considerations
This could optionally be bound by some weighting. However, seems like a simple count bound would probably work for this cache. Somewhat arbitrarily around 10K? Or do you have metrics showing distribution of common counts? This is essentially just every tracer right?
Would you want this configurable?
Any interest in recording stats for this cache? If not, will just remove
recordStats()
and/or logging spikes?
if (cmsToMnf.stats().evictionCount() > threshold) {
logger.warn("High MetricNameFormat cache eviction rate detected!");
}
Testing
Dependent on complete changes
Checks