Ignore noop children when polling composite meters

[jonatan-ivanov]

When a CompositeMeterRegistry is used and one of its CompositeMeters is polled (its value is fetched), the returned value can depend on the order of the registries inside of the composite if the composite contains a registry that has any NoopMeters.

Example: a CompositeMeterRegistry contains two registries, A and B. We create a counter in the composite and increment it once. After this both A and B contain one counter but lets say that in A the counter is ignored so it will be noop. When the count called on CompositeCounter, it can return either 0 (if NoopCounter was used) or 1 (if the non-noop counter was used).

In order to fix this, we can ignore the NoopMeters when Meters are polled in a composite registry.

Closes gh-1441

[stefano-salmaso]

You are merging this into micrometer-metrics:1.9.x, why not in main?

[jonatan-ivanov]

You are merging this into micrometer-metrics:1.9.x, why not in main?

Because I think we should consider this as a bug not a new feature. If we would merge this in main only 1.12.x and newer versions will have this fix. If we merge this into 1.9.x, then that and newer versions will have the fix (1.9, 1.10, 1.11, 1.12, ...)

[klau-nagy-cs]

Hi, Are there any news on this PR? Do you know when this fix will be merged and released? We have the same problem using version 1.11 and this fix would solve our issue. Thank you!

[jkemming]

Hi @jonatan-ivanov,

I've implemented a solution on top of your PR that addresses the "hot path" issue, please have a look. Feel free to incorporate the changes here, or if you want I can open a separate PR.

Also, there are two alternative solutions I have considered. If any of those sound better to you, please let me know:

• If atomic updates to children and firstNonNoopChild aren't required, the inner class could essentially be inlined.
• Alternatively, we could keep the calculation on the "hot path" and cache the result, clearing the cache when children is updated.

[jonatan-ivanov]

@klau-nagy-cs

Are there any news on this PR? Do you know when this fix will be merged and released? We have the same problem using version 1.11 and this fix would solve our issue.

Szia, No news, we need to review and discuss it. Adding 👍🏼 to the issue could help with prioritizing.

@jkemming

I've implemented a solution on top of your PR that addresses the "hot path" issue, please have a look. Feel free to incorporate the changes here, or if you want I can open a separate PR.

Hi, I'm not sure I understand, My comment about the "hot path" is saying that what I'm doing in the PR should not be on the hot path so it should not have a hot path issue. That part of the code should only called once per every publishing but depending on the setup add/remove registries could be even less frequent, we should definitely consider this option too.

[jkemming]

I'm afraid I misunderstood your comment then, sorry for that... Should there be anything I can do to help, please let me know.

[benkeil]

Can we merge this?

[hoa2clj]

Hello! Do you have any updates on this PR? Or do you know when it might be merged and released? Thank you in advance!