Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace usages of io.micrometer.core.instrument.Tags with Iterable<Tag> #1612

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

DanielThomas
Copy link
Member

io.micrometer.core.instrument.Tags turns out to be a poor intermediate representation of Iterable<Tag>. In addition to being immutable and requiring a new instance for every call, it sorts and deduplicates elements on every operation which can lead to some significant CPU overhead on hot paths and/or with large numbers of tags:

https://github.com/micrometer-metrics/micrometer/blob/a56b968ba5b3db9b5e4a4feac813080783a16f5f/micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java#L43-L47

All of the tags builder methods call Tags::and and there's no optimization for an initially empty state or the passed Iterable<? extends Tag> being a Tags instance, so you pay that overhead when finally adding the tags, no matter how the tags are constructed:

This replaces all direct usages of Tags with Iterable<Tag> and avoids calling the tag builder methods more than once.

@DanielThomas DanielThomas force-pushed the dannyt/micrometer-tag-performance branch from fb90913 to e34352c Compare August 28, 2023 06:14
@srinivasankavitha srinivasankavitha merged commit aaa04f8 into master Aug 28, 2023
@DanielThomas DanielThomas deleted the dannyt/micrometer-tag-performance branch August 28, 2023 22:48
@DanielThomas
Copy link
Member Author

Reported upstream too micrometer-metrics/micrometer#4052

@ShivaniSara
Copy link

Is the CPU overhead issue due to Tags::and resolved after this change? I am still seeing this issue with micrometer-registry-dynatrace.version 1.11.5

@DanielThomas
Copy link
Member Author

What release of DGS @ShivaniSara? This that might change after 8.5.0 and the switch to Spring GraphQL under the covers.

@ShivaniSara
Copy link

ShivaniSara commented Apr 4, 2024

I am sorry I am not clear on the DGS release. I was just using 'micrometer-registry-dynatrace.version' to collect metrics and was able to see in Dynatrace that Tags::and is consuming alot. I see in Counter class:

public Builder tags(Iterable tags) {
this.tags = this.tags.and(tags);
return this;
}

@DanielThomas
Copy link
Member Author

Oh I see what you're asking. Yes, this change addressed the overhead, reducing the overhead to a single sort per meter construction. Using this pattern is definitely the way to go until the upstream issue is addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants