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

Warn about gauge re-registration #5688

Conversation

jonatan-ivanov
Copy link
Member

@jonatan-ivanov jonatan-ivanov commented Nov 16, 2024

See #5616 and #5617

I added the warning log entries and benchmarks since the extra logging is on the hot path.
The benchmarks don't test the case where the Gauge re-registration happens indirectly (as the result of a MeterFilter transformation) but since the direct and the indirect cases call the same method, I think testing only the direct case is ok.

I ran the benchmarks a couple of times but they were not as stable on my laptop as I would have liked, the difference between them was sometimes bigger between two identical executions (e.g. without logging) than between the two different ones (with vs. without logging).

So based on this either the benchmarks are wrong or the "noise" is bigger than the difference between the two scenarios I tested.

Before logging

Benchmark                                                   Mode      Cnt     Score   Error  Units
GaugeBenchmark.baseline                                   sample  1874878     0.065 ± 0.003  us/op
GaugeBenchmark.baseline:p0.00                             sample              0.001          us/op
GaugeBenchmark.baseline:p0.50                             sample              0.041          us/op
GaugeBenchmark.baseline:p0.90                             sample              0.048          us/op
GaugeBenchmark.baseline:p0.95                             sample              0.049          us/op
GaugeBenchmark.baseline:p0.99                             sample              0.084          us/op
GaugeBenchmark.baseline:p0.999                            sample              2.295          us/op
GaugeBenchmark.baseline:p0.9999                           sample             32.624          us/op
GaugeBenchmark.baseline:p1.00                             sample           1249.280          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder             sample  1314563     0.085 ± 0.007  us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.00       sample              0.001          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.50       sample              0.046          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.90       sample              0.054          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.95       sample              0.056          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.99       sample              0.082          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.999      sample             15.680          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.9999     sample             51.722          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p1.00       sample           1472.512          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder          sample  1871879     0.081 ± 0.007  us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.00    sample              0.004          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.50    sample              0.053          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.90    sample              0.062          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.95    sample              0.065          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.99    sample              0.110          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.999   sample              4.201          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.9999  sample             37.748          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p1.00    sample           3993.600          us/op

After logging

Benchmark                                                   Mode      Cnt     Score   Error  Units
GaugeBenchmark.baseline                                   sample  1901391     0.062 ± 0.002  us/op
GaugeBenchmark.baseline:p0.00                             sample              0.001          us/op
GaugeBenchmark.baseline:p0.50                             sample              0.040          us/op
GaugeBenchmark.baseline:p0.90                             sample              0.044          us/op
GaugeBenchmark.baseline:p0.95                             sample              0.047          us/op
GaugeBenchmark.baseline:p0.99                             sample              0.066          us/op
GaugeBenchmark.baseline:p0.999                            sample              1.829          us/op
GaugeBenchmark.baseline:p0.9999                           sample             32.220          us/op
GaugeBenchmark.baseline:p1.00                             sample            114.432          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder             sample  1568405     0.090 ± 0.003  us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.00       sample              0.008          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.50       sample              0.057          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.90       sample              0.066          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.95       sample              0.069          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.99       sample              0.093          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.999      sample             14.704          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p0.9999     sample             42.598          us/op
GaugeBenchmark.gaugeReRegistrationWithBuilder:p1.00       sample            192.000          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder          sample  1667600     0.083 ± 0.004  us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.00    sample              0.010          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.50    sample              0.055          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.90    sample              0.064          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.95    sample              0.067          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.99    sample              0.086          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.999   sample             14.080          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p0.9999  sample             32.783          us/op
GaugeBenchmark.gaugeReRegistrationWithoutBuilder:p1.00    sample           1423.360          us/op

/cc: @keith-turner

@jonatan-ivanov jonatan-ivanov added this to the 1.13.9 milestone Nov 16, 2024
@jonatan-ivanov jonatan-ivanov added the bug A general bug label Nov 16, 2024
@jonatan-ivanov jonatan-ivanov changed the base branch from main to 1.12.x November 16, 2024 01:10
@jonatan-ivanov
Copy link
Member Author

I added benchmarks to see the overhead if the registered Meter is not a Counter (basically, we are measuring the cost of meter instanceof Gauge).

I ran the tests quite a few times but on my machine it seems the "noise" is bigger than the difference between the two scenarios I tested.

Before instanceof

Benchmark                            Mode      Cnt     Score   Error  Units
CounterBenchmark.baseline          sample  3265779     0.057 ± 0.002  us/op
CounterBenchmark.baseline:p0.00    sample              0.001          us/op
CounterBenchmark.baseline:p0.50    sample              0.034          us/op
CounterBenchmark.baseline:p0.90    sample              0.039          us/op
CounterBenchmark.baseline:p0.95    sample              0.040          us/op
CounterBenchmark.baseline:p0.99    sample              0.089          us/op
CounterBenchmark.baseline:p0.999   sample              4.778          us/op
CounterBenchmark.baseline:p0.9999  sample             30.048          us/op
CounterBenchmark.baseline:p1.00    sample           1058.816          us/op
CounterBenchmark.countSum          sample  3226782     0.069 ± 0.003  us/op
CounterBenchmark.countSum:p0.00    sample              0.001          us/op
CounterBenchmark.countSum:p0.50    sample              0.041          us/op
CounterBenchmark.countSum:p0.90    sample              0.047          us/op
CounterBenchmark.countSum:p0.95    sample              0.048          us/op
CounterBenchmark.countSum:p0.99    sample              0.087          us/op
CounterBenchmark.countSum:p0.999   sample             14.307          us/op
CounterBenchmark.countSum:p0.9999  sample             33.109          us/op
CounterBenchmark.countSum:p1.00    sample           1986.560          us/op
CounterBenchmark.retrieve          sample  3461976     0.067 ± 0.002  us/op
CounterBenchmark.retrieve:p0.00    sample              0.001          us/op
CounterBenchmark.retrieve:p0.50    sample              0.042          us/op
CounterBenchmark.retrieve:p0.90    sample              0.050          us/op
CounterBenchmark.retrieve:p0.95    sample              0.057          us/op
CounterBenchmark.retrieve:p0.99    sample              0.089          us/op
CounterBenchmark.retrieve:p0.999   sample              6.712          us/op
CounterBenchmark.retrieve:p0.9999  sample             31.328          us/op
CounterBenchmark.retrieve:p1.00    sample           1437.696          us/op

After instanceof

Benchmark                            Mode      Cnt      Score   Error  Units
CounterBenchmark.baseline          sample  3223518      0.063 ± 0.003  us/op
CounterBenchmark.baseline:p0.00    sample               0.001          us/op
CounterBenchmark.baseline:p0.50    sample               0.035          us/op
CounterBenchmark.baseline:p0.90    sample               0.042          us/op
CounterBenchmark.baseline:p0.95    sample               0.043          us/op
CounterBenchmark.baseline:p0.99    sample               0.078          us/op
CounterBenchmark.baseline:p0.999   sample               9.648          us/op
CounterBenchmark.baseline:p0.9999  sample              34.880          us/op
CounterBenchmark.baseline:p1.00    sample            2015.232          us/op
CounterBenchmark.countSum          sample  3159236      0.077 ± 0.013  us/op
CounterBenchmark.countSum:p0.00    sample               0.001          us/op
CounterBenchmark.countSum:p0.50    sample               0.042          us/op
CounterBenchmark.countSum:p0.90    sample               0.049          us/op
CounterBenchmark.countSum:p0.95    sample               0.050          us/op
CounterBenchmark.countSum:p0.99    sample               0.081          us/op
CounterBenchmark.countSum:p0.999   sample              13.872          us/op
CounterBenchmark.countSum:p0.9999  sample              35.840          us/op
CounterBenchmark.countSum:p1.00    sample           10076.160          us/op
CounterBenchmark.retrieve          sample  3418847      0.071 ± 0.002  us/op
CounterBenchmark.retrieve:p0.00    sample               0.001          us/op
CounterBenchmark.retrieve:p0.50    sample               0.043          us/op
CounterBenchmark.retrieve:p0.90    sample               0.049          us/op
CounterBenchmark.retrieve:p0.95    sample               0.051          us/op
CounterBenchmark.retrieve:p0.99    sample               0.085          us/op
CounterBenchmark.retrieve:p0.999   sample               9.232          us/op
CounterBenchmark.retrieve:p0.9999  sample              34.496          us/op
CounterBenchmark.retrieve:p1.00    sample            1359.872          us/op

Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine merging since we didn't detect a consistent, significant increase in time taken via benchmarks.

@jonatan-ivanov jonatan-ivanov merged commit 12d3072 into micrometer-metrics:1.12.x Dec 4, 2024
6 checks passed
@jonatan-ivanov jonatan-ivanov deleted the warn-about-gauge-re-registration branch December 4, 2024 21:10
izeye added a commit to izeye/micrometer that referenced this pull request Dec 22, 2024
@izeye izeye mentioned this pull request Dec 22, 2024
shakuzen pushed a commit that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants