Avoid calling naming convention on scrape #5288
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shakuzen
added
bug
The naming convention was being called on each scrape for each metric to create the MetricMetadata. This is unnecessary because we already have the computed convention name specifically to avoid needing to call the convention again.
jonatan-ivanov
approved these changes
Jul 8, 2024
| String name = config().namingConvention().name(id.getName(), id.getType(), id.getBaseUnit()) + suffix; | ||
| String help = prometheusConfig.descriptions() ? Optional.ofNullable(id.getDescription()).orElse(" ") : " "; | ||
| private MetricMetadata getMetadata(String name, @Nullable String description) { | ||
| String help = prometheusConfig.descriptions() ? Optional.ofNullable(description).orElse(" ") : " "; |
There was a problem hiding this comment.
This was a copy-paste form the 0.x client code but eliminating the Optional might also have some positive impact on performance.
There was a problem hiding this comment.
Agreed and it stood out to me too when making these changes, but I wanted to keep the changes here focused to one issue. Let's take it up in a separate change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The naming convention was being called on each scrape for each metric to create the MetricMetadata. This is unnecessary because we already have the computed convention name specifically to avoid needing to call the convention again.
See #5229
The same test has been added to the micrometer-registry-prometheus module and the simpleclient module as well. The test is passing without changes to the simpleclient module, but the changes made in main code were needed for this to pass in micrometer-registry-prometheus, demonstrating the difference in behavior we want to ensure does not regress in the future.