Allow configuration of collector registry used

[raphw]

Currently, the registry is hard-coded to the default registry. With this change, this would no longer be the case without changing the default behavior where no custom registry is registered.

[brian-brazil]

Can you explain your use case for this? Custom registries are only generally needed for certain advanced forms of exporters, which isn't something you'd write in Spring (or in Java usually for that matter).

[raphw]

The default registry makes it impossible to refresh a Spring context where all beans are discarded and reinitialized. If this initialization registrers its metrics from beans with fixed names, the refresh will cause a naming conflict.

[brian-brazil]

If this initialization registrers its metrics from beans with fixed names, the refresh will cause a naming conflict.

That's the expected behaviour, metrics with a given name should only be defined once. I don't see how what you're proposing here helps with that.

[raphw]

I register the CollectorRegistry as a bean such that it lives and dies with my application context. This is not true for a VM-global singleton which lives as long as the VM. Often, the VM's and the application's life-cycle are considered to be equal what is not true for my use case. A VM might live to see multiple starts and stops of my Spring Boot application.

[brian-brazil]

That would give incorrect semantics from a metrics standpoint, once a metric is registered it should stay registered until the process terminates.

[raphw]

This is not a problem. The application goes offline and online, why would the metrics endpoint bother that it is run in the same OS process? The endpoint does not even know about the process. This is just a deployment detail.

[brian-brazil]

From a prometheus standpoint, that's not a deployment detail that's an important architectural consideration.
What I'm hearing is that your application is in fact a scheduling system, similar to say kubernetes, as it's starting and stopping applications inside of it. Accordingly the way you do that is hook into service discovery, and have Prometheus scrape a separate target for each of these applications while it's alive.

[raphw]

No, it is a testing harness. But neither does it matter. There is no problem on my end here. It should be possible to refresh a Spring context when using Prometheus, that is my argument here.

[brian-brazil]

This is a dupe of #279. A custom registry is not the right solution here.

[raphw]

I never said that it is a Spring Boot test. It is a custom testing harness which is starting and stopping services which sometimes are a Spring Boot application. Besides this one Prometheus-Spring-Boot-integration, this does not cause any problems.

It is of course your right to reject this PR but this is a valid use case and I offered a solution. Also, if Prometheus offers custom registries, why should the Spring integration not support this at all?

[rolandjohann]

@brian-brazil Are there any arguments to reject this PR? I have the issue that integration tests reuse the JVM and the static default registry will live across application contexts.

The right solution in this case is to specify the registry via dependency injection and require it from elsewhere via dependency injection, not static definition - which is an anti pattern.

[codefromthecrypt]

@brian-brazil we recently switched to using this client due to its spring boot integration. When folks who really know what they are doing are told they are doing it wrong, and PRs are rejected which can avoid ugly hacks.. well this behavior without any viable alternative makes me reconsider if it was a wise choice to use this library. If you can, please take a step back and listen.. you have some experts, for example @raphw we do actually know what we are talking about

[geoand]

@brian-brazil It would be great for users of the library if you could point us in the direction of the way you propose the issue of the static registry should be dealt with.
I am asking because this PR proposes a solution that is in-line with the Spring philosophy, but has been rejected as not being in accordance to Prometheus' architecture, all the while leaving us users without an indication of what a valid solution might look like.

[brian-brazil]

if you could point us in the direction of the way you propose the issue of the static registry should be dealt with.

Only register a metric once, and then use that metric for the lifetime of the process.

[geoand]

@brian-brazil Thank you for your reply.

I understand that the metric should only be used once under normal circumstances in order to get semantically correct metrics, but in other situations (like the one @raphw is trying to solve) it is a solid blocker that people will have to work around anyway.

[brian-brazil]

For testing there are methods to clear the registry and remove metrics from it.

These are not things that should be exposed to general instrumentation though, as there are many users who misunderstand how a library like this should be used (largely thinking of a different type of library) and which would lead them down the wrong path.

[geoand]

@brian-brazil Understood, thanks

[brian-brazil]

So thinking a little: Allowing users to select a custom registry in instrumentation would be incorrect, that's not what that feature is for. However if test setups could somehow under the covers have the normal registration use a custom registry, that'd be fine.

[geoand]

@brian-brazil Do you have any specific mechanism in mind?

A simple (albeit terrible solution) would be to have JUnit 4 Rule that would clear the metrics upon completion of each test.
A perhaps more viable solution for Spring tests, would be to have an implementation of TestExecutionListener that clears the metrics

[brian-brazil]

I'm not a Spring expert, I don't know what sort of capabilities it has. I'm imagining something that'd cause metrics from annotations to be registered with a custom registry only for test cases.

[geoand]

@brian-brazil Thanks!

[raphw]

If it helps anybody: we have migrated everything to Micrometer which offers integration that is compatible to Spring's way of doing things. This is also what the Spring team recommends. No problems eversince.