spring-cloud
Enhancement: RefreshScope should rebind MeterRegistry metrics
When using the spring cloud config server and changing a property value registered as Metric in a MeterRegistry, the value is not refreshed automatically.
As a first use case, suppose you want to change the value of a Hikari connections pool size:
from spring.datasource.hikari.maximumPoolSize=5 to spring.datasource.hikari.maximumPoolSize=10
(and adding spring.cloud.refresh.extra-refreshable=javax.sql.DataSource in my personal case),
the new created datasource won't appear as a new metric in the MeterRegistry who keeps the old one.
To circumvent this issue, I'm currently using a BeanPostProcessor
to remove the old datasource metrics and to rebind the new datasource to the registry.
/*
When changing a datasource value at runtime, the Prometheus Metrics registry does not pick up the changes automatically.
We have to remove the old datasource values and rebind the new datasource with the registry.
*/
public class DataSourceMetricsRefresher implements BeanPostProcessor {
private final MeterRegistry registry;
public DataSourceMetricsRefresher(MeterRegistry registry) {
this.registry = registry;
}
@Override
public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
if (DataSource.class.isAssignableFrom(bean.getClass())) {
HikariDataSource hikariDataSource = DataSourceUnwrapper.unwrap((DataSource) bean, HikariDataSource.class);
List<Meter> meters = this.registry.getMeters();
meters.stream().filter(meter -> meter.getId().getName().startsWith("hikaricp")).forEach(registry::remove);
hikariDataSource.setMetricsTrackerFactory(new MicrometerMetricsTrackerFactory(this.registry));
}
return bean;
}
}
As a second use case, suppose you want to enable/disable hibernate stats on the fly:
spring.jpa.properties.hibernate.generate_statistics=false
Here again, you have to do the same dance to rebind the HibernateMetrics to the registry.
This is not ideal and should be provided out of the box like it's done for logging in the LoggingRebinder.
From what you describe it looks like it’s a problem specifically targetted at a refreshed datasource. What would happen with counters? eg. a counter that is tracking the amount of transactions an application has executed? Are they going to be reset? Or will they be recreated with the previous value? (Applies for all meters actually)
So I’m not sure how that would work out in the end. 🤔
@TYsemyn This issue is not specific to datasource metrics, it’s the case for all the registered metrics in the meterregistry. (See second use case for hibernate stats) Regarding the ds, it’s more about changing the connection pool at runtime and see the new values correctly reflected in the registry. (Prometheus in my case) But indeed, when you change the pool size, behind the scene, a new connection pool replaces the old one and it would mean that you will not see previously recorded metrics for the old one anymore, and as you say, same for other metrics. The “old” metrics would already have been exported so you don’t really “loose” them and could trace what happened across time. (Btw, the exact same thing happens today if you restart your application) IMO, these metrics would at least be consistent with your current application state, no?
That's just it, refreshing (part of) the application context or restarting your application are 2 different things from my perspective. Prometheus is tracking your application instance by scraping it every x seconds. So if you visualize it with eg. Grafana in a graph you'll constantly see an increase for your counters over time. When your application instance has been shut down and a new one starts, your graph will also stop on that point in time.
Now imagine you have that same graph but you re-register that counter when you refresh your application then you'll have a sudden drop in that graph. Somebody else who's looking at the dashboard who doesn't know that you refreshed the context will be completely baffled.
Throw in metrics analysis eg. canary analysis and you'll have a useless score, ending up with a possible worse version of your application in production. That said, refreshing your application context while doing such analysis is in general a bad practice imo though.
Valid point taken, but IMO, having discrepancies between the metrics and the real values is still not acceptable and we should find a way to fix it. 🙂
@spencergibb he knows about it since I opened this issue after a chat with him on slack 😉
but I didn't know that. Would have been nice context in the original posting.
I agree with both of you
The main issue here is that refreshing a bean should (ideally) re trigger all the post processing phases that happened the first time. But this does not seems reasonably feasible.
How spring could detect that this method:
@Autowired
public void bindMetricsRegistryToHikariDataSources(
Collection<DataSource> dataSources) {
for (DataSource dataSource : dataSources) {
HikariDataSource hikariDataSource = DataSourceUnwrapper.unwrap(dataSource,
HikariDataSource.class);
if (hikariDataSource != null) {
bindMetricsRegistryToHikariDataSource(hikariDataSource);
}
}
}
that is responsible of registering the metrics registry to all the datasource should be recalled when a DataSource object is refreshed.
I'll be happy if you manage to do that, otherwise, I'll favorite your workaround @ask4gilles :)
If I recall correctly, behind the scenes, there is a check done in hikari to verify that the registry is not already set, so you can’t just rely on refreshing this bean. Furthermore, the ´old’ metrics are kept in the registry and, currently, you have to deregister them yourself. As a reminder, this issue is not just about the datasource case but also for every resource refreshed by spring cloud and registered in the registry.
For context, here is the thread on the Micrometer slack: https://micrometer-metrics.slack.com/archives/C662HUJC9/p1547124725097800
I don't know how this would be generically achieved currently. Metrics can be registered via MeterBinders and also directly, so I don't think Spring Cloud can know how to rebind everything (HikariCP metrics are not registered via a MeterBinder, for instance). I suppose at least Spring Cloud could bind all MeterBinder beans again, but for that to have any effect (in most cases), all of the meters originally registered by a binder would need to be removed first. MeterRegistry isn't aware of what MeterBinder any metric came from, and MeterBinders don't have an unbind concept right now and it would be a breaking change to require it. Furthermore, Spring Boot properties support for configuring metrics also poses a problem, as something like common tags (management.metrics.tags) could be changed by a refresh, which affects all metrics (it's a MeterFilter), but again that would require re-registering all metrics.
Cloud "refresh" support copy some specfic properties from the old instance to the new one?
