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

Move SamplerResult to SDK #574

Closed
MikeGoldsmith opened this issue Apr 7, 2020 · 3 comments · Fixed by #591
Closed

Move SamplerResult to SDK #574

MikeGoldsmith opened this issue Apr 7, 2020 · 3 comments · Fixed by #591
Labels
pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Comments

@MikeGoldsmith
Copy link
Member

Samplers and associated classes / structs are meant part of the SDK, not the API. SamplerResult is currently in OpenTelemetry.Api/Trace.

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#sampling

@MikeGoldsmith MikeGoldsmith added pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package labels Apr 7, 2020
@MikeGoldsmith MikeGoldsmith added this to the Beta milestone Apr 7, 2020
@eddynaka
Copy link
Contributor

@MikeGoldsmith , can I take this issue? If yes, the idea is to move the file OpenTelemetry.API/Trace/SamplingResult.cs to OpenTelemetry/Trace/SamplingResult.cs and the classes that that struct uses? What other classes should i move?
Thanks

@cijothomas
Copy link
Member

Other classes using SamplingResult are already in the SDK itself, so only SamplingResult would require movement.

@eddynaka
Copy link
Contributor

eddynaka commented Apr 10, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants