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

[Sampler.Aws] perf improvements for AWS Xray sampler #2046

Merged
merged 4 commits into from
Sep 9, 2024

Conversation

birojnayak
Copy link
Contributor

@birojnayak birojnayak commented Sep 8, 2024

Fixes #
This fixes the issue mentioned here #2045 , the dotnet runtime issue is created [here] (dotnet/runtime#107505)

Changes

The changes are to write informational event to event source only when fallback sampler is called first time instead of calling repeatedly. We saw an improvement in 4 times in TPS after this change in our load testing.

Screenshot 2024-09-07 at 6 00 48 PM

Merge requirement checklist

  • [x ] CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • [x ] Changes in public API reviewed (if applicable)

@birojnayak birojnayak requested a review from a team September 8, 2024 01:04
@github-actions github-actions bot added the comp:sampler.aws Things related to OpenTelemetry.Samplers.AWS label Sep 8, 2024
@github-actions github-actions bot requested review from ppittle and srprash September 8, 2024 01:05
@Kielek Kielek changed the title perf improvements for AWS Xray sampler [Sampler.Aws] perf improvements for AWS Xray sampler Sep 9, 2024
@Kielek
Copy link
Contributor

Kielek commented Sep 9, 2024

I think that, the perf. improvements should be documented in the CHANGELOG.md.
EDIT: Please fix also dotnet format issues.

@birojnayak
Copy link
Contributor Author

birojnayak commented Sep 9, 2024

Issue #2049 #2046 tagged.. #2049 is to release next alpha version.
@ppittle @vastin please give a comment/thumbs up saying we need a release. @srprash is in PTO, so @vastin is covering on his behalf. @Kielek FYI..

Copy link

@vastin vastin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Kielek Kielek left a comment

Choose a reason for hiding this comment

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

Approving, based on component owners needs.

@Kielek Kielek merged commit 336a89d into open-telemetry:main Sep 9, 2024
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:sampler.aws Things related to OpenTelemetry.Samplers.AWS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants