Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor configured step duration when building Wavefront senders #2173

Merged
merged 1 commit into from
Jul 3, 2020

Conversation

jchambers
Copy link
Contributor

This fixes an issue with the Wavefront registry where, no matter what is set for a step duration in WavefrontConfig, the Wavefront sender will always emit metrics once per second. I'm fairly confident the once-per-second behavior is not intended because it seems to go against the documentation for WavefrontConfig#step() and also against Wavefront's own example code, which shows providing a step of Duration.ofSeconds(10) with an explanation that:

The following code fragment creates a Wavefront registry that emits data every 10 seconds…

Cheers!

@shakuzen shakuzen added bug A general bug registry: wavefront A Wavefront (Tanzu Observability) Registry related issue labels Jul 2, 2020
@shakuzen shakuzen added this to the 1.5.3 milestone Jul 2, 2020
@shakuzen shakuzen changed the base branch from master to 1.5.x July 3, 2020 03:02
@shakuzen
Copy link
Member

shakuzen commented Jul 3, 2020

Thank you for reporting this and providing a fix. Sorry for the issue - it looks like we missed this when switching from using our own HttpSender abstraction to Wavefront's SDK for sending metrics. Unfortunately I don't see a good way to test that this is having the affect we want, but we'll get the fix merged and I'll add a (brittle) test asserting the internal value is what we expect on the default builder. Thanks again!

@shakuzen shakuzen merged commit a353a55 into micrometer-metrics:1.5.x Jul 3, 2020
shakuzen added a commit that referenced this pull request Jul 3, 2020
Switches from setting the flush interval in seconds to milliseconds for added precision, though such a level of granularity is not usually important. Also adds tests to ensure the flush interval is set according to the configured step interval.

See #2173
@jchambers
Copy link
Contributor Author

Yeah—I had the same struggle with testing, but think what you've come up with is reasonable. Cheers, and thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug registry: wavefront A Wavefront (Tanzu Observability) Registry related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants