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

TimeWindowMax throws ArithmeticException with a rotateFrequency of 0 #3068

Closed
Noodle-Head opened this issue Mar 11, 2022 · 4 comments
Closed
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Milestone

Comments

@Noodle-Head
Copy link

Noodle-Head commented Mar 11, 2022

Describe the bug
We are experiencing a bug caused by a modulus operation introduced in the TimeWindowMax.java class.
java.lang.ArithmeticException: / by zero is thrown when running our application tests.

Environment
When running integration tests locally and as a part of gitlab build

  • Micrometer version [1.8.1] (it works on [1.6.4])
  • Micrometer registry [Prometheus]
  • OS: [Windows/Linux]
  • Java version: ["11.0.12" 2021-07-20 LTS]

To Reproduce
How to reproduce the bug:
I am not sure how to minimalistic reproduction of the bug. Our application was working fine on Micrometer version 1.6.4. It fails when upgrading to version 1.8.1, see details below:

The bug is caused by line 139 in the TimeWindowMax.java class.

lastRotateTimestampMillis = wallTime - timeSinceLastRotateMillis % durationBetweenRotatesMillis;

This line will throw java.lang.ArithmeticException: / by zero whenever durationBetweenRoatesMillis is 0. I assume this is a bug that should be fixed.

Expected behavior
I expect the calculation to work even though durationBetweenRotatesMillis is set to 0.

@twicksell
Copy link

I just ran into this one when someone set the step size for a registry to 0 as a way of "disabling" metrics reporting. Surprising error that came from SimpleMeterRegistry passing the step size forward through DistributionStatisticConfig.

        DistributionStatisticConfig merged = distributionStatisticConfig
                .merge(DistributionStatisticConfig.builder().expiry(config.step()).build());

Maybe a simple validation check somewhere along this flow would be warranted.

@Noodle-Head
Copy link
Author

I am closing this issue as the cause was an errendous mock of the PromotheusConfig in the test-environment.

Validation could be considered to provide better feedback when the issue is presented.

@shakuzen shakuzen added this to the 1.8.x milestone Jun 2, 2022
@shakuzen shakuzen changed the title Bug introduced in TimeWindowMax.java TimeWindowMax throws ArithmeticException with a rotateFrequency of 0 Jun 2, 2022
@shakuzen
Copy link
Member

shakuzen commented Jun 2, 2022

I'm reopening this to improve the validation.

@shakuzen shakuzen reopened this Jun 2, 2022
@shakuzen shakuzen added module: micrometer-core An issue that is related to our core module bug A general bug labels Jun 2, 2022
@jonatan-ivanov jonatan-ivanov modified the milestones: 1.8.x, 1.9.x Jan 12, 2023
marcingrzejszczak added a commit that referenced this issue Dec 22, 2023
without this change you can get an arithmetic exception if you pass a 0. Negative numbers don't make sense

fixes gh-3068
@marcingrzejszczak
Copy link
Contributor

This PR should fix things

@shakuzen shakuzen modified the milestones: 1.9.x, 1.10.14 Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug module: micrometer-core An issue that is related to our core module
Projects
None yet
Development

No branches or pull requests

5 participants