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

Parallel and synchronized test execution for loadscraper.TestScrape #28877

Merged
merged 17 commits into from
Mar 30, 2024

Conversation

michalpristas
Copy link
Contributor

Description:
This is not a pretty refactor but I believe this is proper way to fix the test.

I was running into 0 is not lower than 0 error when Processor Queue Length on my system was properly reported as 0.
I added a special case not to fail in this case.

Then the main fix here is making test cases run in parallel as concurrent as possible.
Windows is special case when it comes to reporting CPU metrics as it uses sampling. This sampling is happening every 5 seconds.
While sampling it loads current load from counter and does 1/5/15 minutes average.
As tests are running sequentially, this current load can be different.

To work around this each test case is started in a goroutine while main test goroutine waits for all of them to start.
Each subtest waits for signal to start processing. This signal comes from main goroutine.
This way we achieve as close execution as possible. However there's no guarantee but it should be way more stable than it was.
To make test even more realistic, I changed sequencing frequency to 500 milliseconds in a test and let 3 seconds for scraper to do some actual work and not evaluate on a single scrape.

I ran more than 1200 consecutive executions without failure on my machine.

Link to tracking Issue: #10030

Testing: Unit test

Documentation:

@michalpristas
Copy link
Contributor Author

michalpristas commented Dec 1, 2023

hey @dmitryax pinging just not to let this PR die stale. ay feedback would be appreciated

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 27, 2023
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jan 10, 2024
@dmitryax dmitryax reopened this Mar 26, 2024
@dmitryax dmitryax added Skip Changelog PRs that do not require a CHANGELOG.md entry and removed Stale labels Mar 26, 2024
@dmitryax
Copy link
Member

Hi @michalpristas , I'm sorry I missed the PR. It LGTM. Can you please rebase it?

@michalpristas
Copy link
Contributor Author

michalpristas commented Mar 28, 2024

hey @dmitryax thanks for getting back. will rebase soon

@github-actions github-actions bot requested a review from braydonk March 28, 2024 08:30
@dmitryax dmitryax merged commit 4a71e9d into open-telemetry:main Mar 30, 2024
142 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/hostmetrics Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants