Publishing partial step doesn't work for SignalFx registry
Describe the bug https://github.com/micrometer-metrics/micrometer/pull/3681 fixed the issue of publishing partial values when the application is shut down. The issue was addressed by introducing a package-private StepMeter contract which defines _closingRollover() to achieve this. Unfortunately, SignalFxMeterRegistry extends AbstractTimer and AbstractDistributionSummary which are not instances of StepMeter and hence this fix is only applied for Counter type meters.
Environment
- Micrometer version - 1.11.0
- Micrometer registry - SignalFx
- Java version: 8+
To Reproduce How to reproduce the bug: Start a micrometer registry SignalFx
- step=60seconds
- and create one timer and one counter. Increment the counter every second and record a random value in the timer every second.
During the app shutdown, the values for the previous step will be reported instead of a closing rollover.
Expected behavior Values accumulated for the current step should be published when the app is shut down.
I think the fixes we did for OTLP (https://github.com/micrometer-metrics/micrometer/pull/3798) are related. There are some lower level components I think we should fix/redesign first to make these work/better (e.g.: better injection for Histogram, interface for Max)
I think the root of this (at least this issue) is because of the inability to StepMeters in SignalFxRegistry. And the reason that SignalFxTimer/Ds didn't extend StepTimer/DS is that Histogram is not pluggable. I guess we addressed the pluggability of Histogram to an extent in this PR which added the capability to provide a custom implementation for Timers. This can be applied to StepTimers too.
By making the SignalFxTimers as StepTimers, I guess micrometer-core will take care of that.
https://github.com/micrometer-metrics/micrometer/pull/3799 fixes this issue for SignalFxMeterRegistry. But there might be issues with StepBucketHistogram cases where Histogram might not get closed properly.
Do you have a test / sample that would replicate the issues with Histograms?
The "waiting for feedback" label doesn't seem to have worked for some reason.
So can we close this?
I think we can close this one.