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

sdk: Update sampling documentation #807

Merged
merged 1 commit into from
Jul 9, 2022

Conversation

hdost
Copy link
Contributor

@hdost hdost commented Jun 5, 2022

  • Move Sampling header into Sampler
  • Enrich classes to have links in tracing module

Relates #800

Signed-off-by: Harold Dost github@hdost.com

@hdost hdost requested a review from a team as a code owner June 5, 2022 18:46
@hdost
Copy link
Contributor Author

hdost commented Jun 7, 2022

@TommyCpp barring the obvious failures is this a good direction for #800?

@TommyCpp
Copy link
Contributor

TommyCpp commented Jun 8, 2022

@TommyCpp barring the obvious failures is this a good direction for #800?

Yeah I think it's a good start on docs, Thanks! I think we should sample at every span to be consistent with other SDKs.

This has a risk of breaking change as the current behavior will be Parent<SubSampler> but I think it's worth it.

@jtescher
Copy link
Member

@hdost hdost force-pushed the doc/800-sampler-tracing-info branch from b853709 to 9b9f56d Compare July 7, 2022 22:13
* Move Sampling header into `Sampler`
* Enrich classes to have links in tracing module

Relates open-telemetry#800

Signed-off-by: Harold Dost <github@hdost.com>
@hdost hdost force-pushed the doc/800-sampler-tracing-info branch from 9b9f56d to 2e5a6c3 Compare July 7, 2022 22:32
@codecov
Copy link

codecov bot commented Jul 7, 2022

Codecov Report

Merging #807 (2e5a6c3) into main (c13a11e) will increase coverage by 0.0%.
The diff coverage is n/a.

@@          Coverage Diff          @@
##            main    #807   +/-   ##
=====================================
  Coverage   68.7%   68.8%           
=====================================
  Files        115     115           
  Lines       9397    9397           
=====================================
+ Hits        6465    6470    +5     
+ Misses      2932    2927    -5     
Impacted Files Coverage Δ
opentelemetry-sdk/src/trace/provider.rs 85.5% <ø> (ø)
opentelemetry-sdk/src/trace/sampler.rs 93.4% <ø> (ø)
opentelemetry-jaeger/src/exporter/mod.rs 53.8% <0.0%> (+2.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c13a11e...2e5a6c3. Read the comment docs.

@hdost hdost mentioned this pull request Jul 7, 2022
Copy link
Contributor

@TommyCpp TommyCpp left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@TommyCpp TommyCpp merged commit 2bb8eab into open-telemetry:main Jul 9, 2022
@hdost hdost mentioned this pull request Sep 27, 2022
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.

None yet

3 participants