Skip to content

Conversation

@harry671003
Copy link
Contributor

@harry671003 harry671003 commented Jun 23, 2025

Changes

This PR mainly enables native histogram fuzz tests. It also addresses some of the issue identified while testing.

In addition it makes the following changes:

  • Excluding duplicate label set errors from test comparison
    • Duplicate label check in Thanos engine doesn't always work similar to Prometheus engine.
  • Removing known optimizations from sample stats checks
    • Thanos engine will output less samples
  • Fixing the buckets for NH load to avoid zero buckets.
    • Histogram.Equals() doesn't equate histograms with zero values.
  • Compare histograms approximately.
  • Allow fractional errors while comparing floats

Excluding known edge cases

Excluding some known edge cases from native histgram fuzz tests such as:

  • sort() and other family functions. Thanos engine doesn't filter out NH samples. Prometheus does.
  • predict_linear with step invariant - Thanos engine is unable to handle this case today.
  • timestamp() - Fix in Implement timestamp() for native histograms #598

Race condition in Prometheus engine

        enginefuzz_test.go:341: case 31 error mismatch.
            new result: {pod="nginx-1"} =>
            {count:66, sum:0, (0.5,1]:1, (1,2]:2, (2,4]:63} @[0]
            {count:66, sum:0, (0.5,1]:1, (1,2]:2, (2,4]:63} @[4000]
            
        old result: {pod="nginx-1"} =>
            {count:66, sum:0} @[0]
            {count:66, sum:0} @[4000]
            {count:66, sum:0} @[8000]    

Fix: prometheus/prometheus#16879

@harry671003 harry671003 force-pushed the fix_native_histogram_fuzz branch 2 times, most recently from 954a373 to cb42d73 Compare June 23, 2025 19:01
@harry671003 harry671003 marked this pull request as ready for review June 23, 2025 19:28
@harry671003 harry671003 force-pushed the fix_native_histogram_fuzz branch 2 times, most recently from 42d726c to 98bb185 Compare July 11, 2025 22:28
@harry671003 harry671003 changed the title Test: Exclude known edge cases from native histogram fuzz tests Test: Enable fuzz tests for native histograms Jul 11, 2025
@harry671003 harry671003 force-pushed the fix_native_histogram_fuzz branch from 98bb185 to 1b31e3b Compare July 15, 2025 19:11
@harry671003 harry671003 requested a review from yeya24 July 15, 2025 19:11
@harry671003 harry671003 force-pushed the fix_native_histogram_fuzz branch 2 times, most recently from fcc8b90 to 02c9313 Compare July 21, 2025 16:13
@harry671003
Copy link
Contributor Author

Waiting for #609

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
@harry671003 harry671003 force-pushed the fix_native_histogram_fuzz branch from 02c9313 to ac6640d Compare July 22, 2025 20:17
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Thanks. This is great work.
I have some small questions and nothing blocks this PR

@yeya24
Copy link
Contributor

yeya24 commented Jul 24, 2025

Let's enable the fuzz tests and see how it goes. We can always adjust things if needed

@yeya24 yeya24 merged commit c8ce33a into thanos-io:main Jul 24, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants