Skip to content

[chore][processor/remotetap] Fix flaky tests #32970

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 22, 2024
Merged

[chore][processor/remotetap] Fix flaky tests #32970

merged 4 commits into from
May 22, 2024

Conversation

crobert-1
Copy link
Member

Description:

I added more information on the issue, but it seems like the limiter being used is especially flaky with timing in tests, so if too many metrics/traces/logs are being sent, the limit won't get fully hit as expected, meaning data will count as two separate intervals in the test (thus not being limited properly). This is flakiness in testing, not the rate limiter's fault, from what I can tell.

Also, in assert.Equal, the expected value should be first, and the actual value should be second. I've swapped these as the original output of the test failure was causing me confusion.

Link to tracking Issue:
Resolves #32967

@crobert-1 crobert-1 changed the title [chore][processor/remotetap] Fix flaky test [chore][processor/remotetap] Fix flaky tests May 9, 2024
@github-actions github-actions bot requested a review from atoulme May 9, 2024 19:46
@crobert-1 crobert-1 added the Run ARM Enable running ARM tests on a PR label May 9, 2024
@crobert-1 crobert-1 marked this pull request as ready for review May 9, 2024 21:42
@crobert-1 crobert-1 requested a review from a team May 9, 2024 21:42
@crobert-1
Copy link
Member Author

Test is now passing

Running target 'test' in module 'processor/remotetapprocessor' as part of group 'processor-1'
make --no-print-directory -C processor/remotetapprocessor test
cd /home/runner/actions-runner/_work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/internal/tools && go build -o /home/runner/actions-runner/_work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum -trimpath gotest.tools/gotestsum
/home/runner/actions-runner/_work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum --rerun-fails=1 --packages="./..." -- -race -timeout 600s -parallel 4 --tags=""
✓  internal/metadata (1.18s)
✓  internal/metadata (1.092s)
✓  . (1.622s)

DONE [31](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/9023148520/job/24794288757?pr=32970#step:7:32) tests in 32.839s

atoulme
atoulme approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

Thank you for fixing!

@crobert-1 crobert-1 added the ready to merge Code review completed; ready to merge by maintainers label May 17, 2024
@crobert-1
Copy link
Member Author

Approved by approver and code owner, added ready to merge for a test only change.

@atoulme
Copy link
Contributor

atoulme commented May 19, 2024

Test failures are only for ARM and don't seem to relate to the functionality changed by this PR.

codeboten
codeboten approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks @crobert-1

@codeboten codeboten merged commit 6cec2a6 into open-telemetry:main May 22, 2024
174 of 178 checks passed
@github-actions github-actions bot added this to the next release milestone May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atoulme atoulme atoulme approved these changes

@codeboten codeboten codeboten approved these changes

Assignees

@MovieStoreGuy MovieStoreGuy

Labels
processor/remotetap ready to merge Code review completed; ready to merge by maintainers Run ARM Enable running ARM tests on a PR
Projects
None yet
Milestone
v0.102.0
Development

Successfully merging this pull request may close these issues.

[processor/remotetap] TestConsume test failure
4 participants
@crobert-1 @atoulme @codeboten @MovieStoreGuy