Using MeterFilter's and 'enable' properties as described in
https://docs.spring.io/spring-boot/docs/current/reference/html/actuator.html#actuator.metrics.customizing
will not work if an CompositeMeterRegistry is used because I've added an metrics export to your configuration.
There is absolutely no mention to this behaviour in the documentation. It was introduced in this PR:
https://github.com/spring-projects/spring-boot/commit/9f2141300035554aa078fcd4f6daea710ea96d13
The original behaviour of MeterFilters and properties that ware applied to all meter registries was in line with Spring documentation. The current behaviour is not. Either the documentation or the functionality must change.
Comment From: walec51
@wilkinsona @jkschneider
Comment From: wilkinsona
Thanks for the report, @walec51. Rather stating what must change, can you please take a step back and explain the problem that you're facing? Without fully understanding it, we won't be in a position to evaluate your suggestion.
The changes made in 9f21413 should only affect the auto-configured composite. The intent here was for the auto-configuration to apply the filters to each of the composite's delegates so that filtering would work as it did before, with the added benefit that filters only configured on some of the delegates would no longer inadvertently apply everywhere.
From what you've described thus far, I'm not sure if the code isn't behaving as we intended or if you have a different problem. If you would like us to spend some more time investigating, please spend some time providing a complete yet minimal sample that reproduces the problem. You can share it with us by pushing it to a separate repository on GitHub or by zipping it up and attaching it to this issue.
Comment From: spring-projects-issues
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
Comment From: spring-projects-issues
Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.
Comment From: ntinnemeier
To give an example of what problems this might bring - this specific behaviour of not applying the MeterFilters for the composite meter registry caused a memory leak for us.
I a nutshell this is what happened: - we were running an application that used only one meter registry (StatsdMeterRegistry). - we were not exposing http client metrics, i.e. management.metrics.enable.http.client.metrics = false - we added another meter registry to our application (PrometheusMeterRegistry) - the CompositeMeterRegistry (that was added because we now have more than one registry) accumulated a lot of http client meter instances, causing our application to run out of memory.
In our specific case a lot of http client metrics were created because of the specific way we use a URI builder to construct URIs for our WebClient. (See: this post https://rieckpil.de/expose-metrics-of-spring-webclient-using-spring-boot-actuator/ for more background on that.)
Even though this memory leak happened under very specific circumstances, I still believe that storing meters in memory is rather unexpected from a programmer perspective and certainly not ideal, especially because we are explicitly filtering these meters out. I can think of many more scenarios that might cause unexpected rises in memory usage or even memory leaks when only adding an extra meter registry to your application.
My advice is to reopen this issue and considering a way to ensure that these meters do not end up in memory.
Comment From: wilkinsona
@ntinnemeier, thanks for describing your situation. Unfortunately, I'm not sure that I understand the description. For example, it's not clear to me why adding the PrometheusMeterRegistry causes HTTP client metrics to be recorded when you have said that they are disabled. Can you please provide a minimal example that reproduces the behaviour you have described? You can share it with us by pushing it to a separate repository on GitHub or by zipping it up and attaching it to this issue.
Comment From: ntinnemeier
@wilkinsona thanks for the quick response!
"it's not clear to me why adding the PrometheusMeterRegistry causes HTTP client metrics to be recorded when you have said that they are disabled." If by "recorded" you mean "being stored in memory", then yes, we're on the same page. This particular behaviour also defeats my expectations.
Anyway, I hope this little, crude program makes it more insightful: https://github.com/ntinnemeier/spring-boot-issue-28188
You can find instructions about how to reproduce the issue in the readme (and also how to make it go away).
To add on this further, the actual problem seems to be that the MeterFilters are intentionally not applied for the AutoConfiguredCompositeMeterRegistry, cf: https://github.com/spring-projects/spring-boot/commit/9f2141300035554aa078fcd4f6daea710ea96d13#diff-0ad034983817f45a2c03e760424a0c8e6552b0b8c760e3352effadec7025e497R64
Shouldn't we actually check if any delegate MeterRegistry accepts the metric by applying the delegate's MeterFilters instead of simply not applying any MeterFilter at all?
Comment From: wilkinsona
Thanks for the sample and for the additional explanation. I believe that I now understand the problem.
Shouldn't we actually check if any delegate MeterRegistry accepts the metric by applying the delegate's MeterFilters instead of simply not applying any MeterFilter at all?
We cannot do that as it would break the CompositeMeterRegistry contract. A composite allows children to be added at runtime. When a new MeterRegistry is added, every meter known to the composite is added to the child. If filtering was applied as you have suggested, meters that all of the existing children filtered out would be lost, even if a new child's filters would accept them.
Aside from the CompositeMeterRegistry contract, as far as I can tell, Micrometer does not provide a way for us to implement the behaviour that you've described. In other words, this would have to be implemented in Micrometer rather than Boot.
@shakuzen @jonatan-ivanov could you take a look at this please? As far as I can tell, things are working as designed but I'd like to be sure that I haven't missed something.
Comment From: shakuzen
For the specific case of web client metrics, I think setting management.metrics.web.client.request.autotime.enabled=false would solve it by not instrumenting web clients in the first place. It sounds like that's the desired behavior for @ntinnemeier anyway.
I can think of many more scenarios that might cause unexpected rises in memory usage or even memory leaks when only adding an extra meter registry to your application.
Unless I'm missing something, the scenario that's an issue is registering meters with unbounded cardinality and a composite. Unbounded cardinality is always something to be avoided or at least controlled. Web client metrics is one of the well-known places where we want metrics but we may not reliably be able to get a low-cardinality templated URI. We try to limit the cardinality by capping the number of unique tags, but that is done by a filter and filters are not applied to composites because of the potential for a registry to be added later.
Shouldn't we actually check if any delegate MeterRegistry accepts the metric by applying the delegate's MeterFilters instead of simply not applying any MeterFilter at all?
I think the key change you're looking for is CompositeMeterRegistry not remembering meters registered to it if they did not end up getting registered to any current child. This would be a breaking change to behavior that some users may be relying upon. Nonetheless, you could open an issue in Micrometer for us to consider such a behavior change.
As far as I can tell, things are working as designed but I'd like to be sure that I haven't missed something.
I don't think you have. It's far from ideal, but a stop-gap solution the Spring Boot team may want to consider is configuring filters on the composite it auto-configures only if the filter is known to be for limiting cardinality, like the web client metrics MeterFilter.maximumAllowableTags one.
Comment From: wilkinsona
Thanks, @shakuzen. The MeterFilter returned by maxiumumAllowableTags appears to be an anonymous inner class which would prevent us from identifying it when consuming the MeterFilter beans and applying them. We could identify it at the point of creation by decorating it with a new CardinalityLimitingMeterFilter that delegates to the maximumAllowableTags filter. It'd work, but I agree it's far from ideal. I'll flag this one for a team meeting so that we can decide if that's something that we want to do.
Comment From: wilkinsona
We've discussed this today and decided that we don't want to try to identify the nature of a meter filter. If you want to take more control over the composite you can disable CompositeMeterRegistryAutoConfiguration and define your own composite meter registry.