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

[exporter/prometheusremotewrite] Make WAL shutdown wait until goroutine exits #37733

Merged
merged 4 commits into from
Feb 6, 2025

Conversation

pjanotti
Copy link
Contributor

@pjanotti pjanotti commented Feb 5, 2025

Description

When WAL is used the shutdown doesn't wait until the go routine launched to handle WAL terminates. This makes the shutdown non-deterministic and cause the test failure issues linked below.

Although a bug on the component it doesn't seem to deserve a changelog since it was detected via tests and there is no open issue around the shutdown of the component.

Link to tracking issue

Fixes #9124, #37715

Testing

Ran the component tests locally multiple times on Windows (more likely to surface concurrency issues).

Documentation

N/A

@pjanotti pjanotti requested review from dashpole and a team as code owners February 5, 2025 23:45
@pjanotti pjanotti added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 5, 2025
Copy link
Member

@ArthurSens ArthurSens 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 looking at this ❤️

@dashpole dashpole added the ready to merge Code review completed; ready to merge by maintainers label Feb 6, 2025
@songy23 songy23 merged commit c33b2ad into open-telemetry:main Feb 6, 2025
185 of 186 checks passed
@github-actions github-actions bot added this to the next release milestone Feb 6, 2025
@pjanotti pjanotti deleted the fix-TestExportWithWALEnabled branch February 6, 2025 18:15
chengchuanpeng pushed a commit to chengchuanpeng/opentelemetry-collector-contrib that referenced this pull request Feb 8, 2025
…ne exits (open-telemetry#37733)

#### Description

When WAL is used the shutdown doesn't wait until the go routine launched
to handle WAL terminates. This makes the shutdown non-deterministic and
cause the test failure issues linked below.

Although a bug on the component it doesn't seem to deserve a changelog
since it was detected via tests and there is no open issue around the
shutdown of the component.

#### Link to tracking issue

Fixes open-telemetry#9124, open-telemetry#37715

#### Testing

Ran the component tests locally multiple times on Windows (more likely
to surface concurrency issues).

#### Documentation

N/A
songy23 pushed a commit that referenced this pull request Feb 10, 2025
…tch type prweWAL (#37781)

#### Description
While implementing #37733 I noticed that the pointer receiver to the
methods of the `prweWAL` were `prwe`, however, this made harder to
navigate the code since it was the same name of the receiver pointer
used for the type `prweExporter`. Changing it to match the respective
type makes easier to read and navigate the code.

#### Testing
Local lint and make.

#### Documentation
N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/prometheusremotewrite ready to merge Code review completed; ready to merge by maintainers Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky test - prometheusremotewriteexporter - Test_PushMetrics/WAL
5 participants