-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[receiver/prometheus] speed up tests by setting skipOffsetting #32341
Conversation
how much of a speed increase did it result in for you locally? |
I had 2 successive runs without errors (one failed on a different issue). |
3 runs without failures. |
I think I see overall a 20s reduction over the ~5minutes tests take. Given that we remove a random offset, it makes tests timing better. That said, I didn't manage to find why we wait 5s between starting the receiver and the first scrape. |
I didn't get a chance to confirm in this receiver, but go's ticker will first tick after your defined duration, by default. Users have to implement their own functionality to have a tick at start time. I assume that's what's going on here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any timing improvement here is helpful, as commented in the issue it looks like other-architecture builds are barely completing tests before the timeout 👍
Also, can you please add a reference to #32298 here? If the tests are consistently passing after this we'll be able to mark it resolved.
Co-authored-by: David Ashpole <dashpole@google.com>
Yes, this is not a great improvement, but we can squeak by.
Done |
4th run, no errors. |
5th run, no errors. |
Since the goal here in my mind is to reduce timeout errors, I'd prefer this go into |
Fixes #32298
This PR modifies prometheusreceiver to allow to set the skipOffsetting option on its scrape option config. This option is private, so it can only be set by reflection.
This removes the random offset to start added to scraping prometheus metrics, so we may get faster CI builds.