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

feat(jaeger-remote-sampler): Implement jaeger remote sampler #4589

Conversation

legalimpurity
Copy link
Contributor

@legalimpurity legalimpurity commented Mar 27, 2024

Which problem is this PR solving?

This is an implementation of JaegerRemoteSampler. This follows the specification mentioned here.

Fixes #4534

Short description of the changes

Add a new samper implementation in experimental packages.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Have added full coverage for entire code.

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added

@legalimpurity legalimpurity requested a review from a team March 27, 2024 12:18
Copy link

codecov bot commented Mar 27, 2024

Codecov Report

Attention: Patch coverage is 94.44444% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 92.86%. Comparing base (2610122) to head (7f476b3).
Report is 12 commits behind head on main.

❗ Current head 7f476b3 differs from pull request most recent head b553fbe. Consider uploading reports for the commit b553fbe to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4589      +/-   ##
==========================================
+ Coverage   90.77%   92.86%   +2.08%     
==========================================
  Files          90      331     +241     
  Lines        1930     9571    +7641     
  Branches      417     2054    +1637     
==========================================
+ Hits         1752     8888    +7136     
- Misses        178      683     +505     
Files Coverage Δ
...s/sampler-jaeger-remote/src/JaegerRemoteSampler.ts 100.00% <100.00%> (ø)
...s/sampler-jaeger-remote/src/PerOperationSampler.ts 100.00% <100.00%> (ø)
...mental/packages/sampler-jaeger-remote/src/types.ts 100.00% <100.00%> (ø)
...y-sdk-trace-base/src/sampler/ParentBasedSampler.ts 78.78% <0.00%> (-5.09%) ⬇️
...trace-base/src/sampler/TraceIdRatioBasedSampler.ts 92.00% <0.00%> (-8.00%) ⬇️

... and 244 files with indirect coverage changes

@pichlermarc pichlermarc added this pull request to the merge queue Aug 23, 2024
Merged via the queue into open-telemetry:main with commit c398601 Aug 23, 2024
19 checks passed
@legalimpurity legalimpurity deleted the feature/implement_jaeger_remote_sampler branch August 28, 2024 16:24
Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
…lemetry#4589)

Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
Co-authored-by: Trent Mick <trentm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Jaeger Remote Sampler as defined by the specification
5 participants