micrometer-metrics
Removing registry from CompositeMeterRegistry does not remove the meters it added
The add(MeterRegistry registry) and remove(MeterRegistry registry) calls on CompositeMeterRegistry are not symmetric in that while adding a registry to a composite results in adding the meters from the composite, removing it does not remove the meters.
For example:
CompositeMeterRegistrycomposite1 = new CompositeMeterRegistry();
composite1.counter("test");
SimpleMeterRegistry registry = new SimpleMeterRegistry();
composite1.add(registry);
assert registry.getMeters().size() == 1;
composite1.remove(registry);
assert registry.getMeters.size() == 0;
The final assert will fail since the counter remains even after the remove. The meter in registry won't see any more updates from the composite, but there is also no way to tell that it is in this state.
hi @shakuzen
I create the branch from 1.5.x line. In case that we would like to merge this bugfix to 1.1.x line, we should create a new PR, right?
For my knowledge, could users affected by this share their use case for dynamically removing child registries from a composite? The more common use case that I'm aware of would be adding child registries on startup and never removing them over the application's lifecycle. What is the situation in which you are wanting to remove a child registry?
@shakuzen -- I can certainly try. Users of our product can define their own applications and we use micrometer to gather performance data for these applications. Each application has a dedicated meter registry and we add them to a composite registry for reporting purposes. The applications may come and go which leads to us wanting to both add and later potentially remove registries from the composite.
Currently we have code in the remove case to work around the fact that the meters aren't removed, but we'd obviously prefer it if that wasn't necessary.
Each application has a dedicated meter registry and we add them to a composite registry for reporting purposes. The applications may come and go which leads to us wanting to both add and later potentially remove registries from the composite.
Are the registries for the applications potentially added back again later? If not, it seems like you no longer need the registry and can just close it - meaning any leftover meters from the composite aren't an issue. Or am I missing something?
A side effect of fixing this would be that whereas now the case of re-adding a child registry to a composite allows the reference to the (composite) meter from the child registry to be used again (after re-add), but if the meter is removed and re-added when re-adding the child registry, this wouldn't be possible anymore. You would need to retrieve the meter from the child registry to get the newly registered meter. I'm not sure this is a use case for anyone - I would think they would use references to the meter from the composite registry if that's how the meter was added in the first place.
Each application has a dedicated meter registry and we add them to a composite registry for reporting purposes. The applications may come and go which leads to us wanting to both add and later potentially remove registries from the composite.
Are the registries for the applications potentially added back again later? If not, it seems like you no longer need the registry and can just close it - meaning any leftover meters from the composite aren't an issue. Or am I missing something?
A side effect of fixing this would be that whereas now the case of re-adding a child registry to a composite allows the reference to the (composite) meter from the child registry to be used again (after re-add), but if the meter is removed and re-added when re-adding the child registry, this wouldn't be possible anymore. You would need to retrieve the meter from the child registry to get the newly registered meter. I'm not sure this is a use case for anyone - I would think they would use references to the meter from the composite registry if that's how the meter was added in the first place.
Taking it into consideration, what would be the best solution? My proposal is the following:
Once a child registry is removed, all the links between composite meters (parents) and childs will be removed. So, in case that the child registry goes again to be added to composite, a new relationship will be created between parents and children metrics.
Are you agree @shakuzen and @sfitts ?