-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Spec Conformance Review: Traces/Sampling #1596
Spec Conformance Review: Traces/Sampling #1596
Comments
Sampler
opentelemetry-go/sdk/trace/sampling.go Lines 25 to 29 in 90bd4ab
Arguments are passes as a SamplingParameter struct to not have an excessively long method signature and ensure future extensibility.
opentelemetry-go/sdk/trace/sampling.go Line 33 in 90bd4ab
opentelemetry-go/sdk/trace/sampling.go Line 34 in 90bd4ab
opentelemetry-go/sdk/trace/span.go Lines 533 to 534 in 90bd4ab
opentelemetry-go/sdk/trace/sampling.go Line 35 in 90bd4ab
opentelemetry-go/sdk/trace/sampling.go Line 37 in 90bd4ab
opentelemetry-go/sdk/trace/sampling.go Line 38 in 90bd4ab
opentelemetry-go/sdk/trace/sampling.go Line 39 in 90bd4ab
Sampling results are returned as a SamplingResult struct.
opentelemetry-go/sdk/trace/sampling.go Line 61 in 90bd4ab
opentelemetry-go/sdk/trace/sampling.go Lines 43 to 57 in 90bd4ab
opentelemetry-go/sdk/trace/sampling.go Line 62 in 90bd4ab
This is not a normative requirement based on the lack of capitalization of "must", but for what it's worth, we do not return attributes in any of the included Samplers.
opentelemetry-go/sdk/trace/sampling.go Line 63 in 90bd4ab
Addressed in #1432
opentelemetry-go/sdk/trace/sampling.go Line 28 in 90bd4ab
|
I noticed that the |
Sampling
If
This is a restatement of the above recommendation.
Based on the way the sampling decision enums are structured in combination with the fact that |
opentelemetry-go/sdk/trace/sampling.go Line 33 in 90bd4ab
This is not technically correct. A |
Tracking here: #1727 |
Traces/Sampling
section of the Spec Compliance Matrix.The text was updated successfully, but these errors were encountered: