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

Ensure all XRay Sampler functionality is under ParentBased logic #6205

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Possible nil dereference panic in `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace`. (#5965)
- Non-200 HTTP status codes when retrieving sampling rules in `go.opentelemetry.io/contrib/samplers/aws/xray` now return an error. (#5718)
- Ensure all AWS XRay Remote Sampler logic is under a ParentBased Sampler in `go.opentelemetry.io/contrib/samplers/aws/xray`. (#6205)

### Removed

Expand Down
4 changes: 2 additions & 2 deletions samplers/aws/xray/remote_sampler.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type remoteSampler struct {
// Compile time assertion that remoteSampler implements the Sampler interface.
var _ sdktrace.Sampler = (*remoteSampler)(nil)

// NewRemoteSampler returns a sampler which decides to sample a given request or not
// NewRemoteSampler returns a ParentBased XRay Sampler which decides to sample a given request or not
// based on the sampling rules set by users on AWS X-Ray console. Sampler also periodically polls
// sampling rules and sampling targets.
// NOTE: ctx passed in NewRemoteSampler API is being used in background go routine. Cancellation to this context can kill the background go routine.
Expand Down Expand Up @@ -77,7 +77,7 @@ func NewRemoteSampler(ctx context.Context, serviceName string, cloudPlatform str

remoteSampler.start(ctx)

return remoteSampler, nil
return sdktrace.ParentBased(remoteSampler), nil
}

// ShouldSample matches span attributes with retrieved sampling rules and returns a sampling result.
Expand Down
11 changes: 11 additions & 0 deletions samplers/aws/xray/remote_sampler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
package xray

import (
"context"
"net/url"
"testing"
"time"

"github.com/stretchr/testify/assert"
)
Expand All @@ -16,3 +19,11 @@ func TestRemoteSamplerDescription(t *testing.T) {
s := rs.Description()
assert.Equal(t, "AWSXRayRemoteSampler{remote sampling with AWS X-Ray}", s)
}

func TestNewRemoteSamplerDescription(t *testing.T) {
endpointURL, _ := url.Parse("http://localhost:2000")
rs, _ := NewRemoteSampler(context.Background(), "otel-test", "", WithEndpoint(*endpointURL), WithSamplingRulesPollingInterval(300*time.Second))

s := rs.Description()
assert.Equal(t, "ParentBased{root:AWSXRayRemoteSampler{remote sampling with AWS X-Ray},remoteParentSampled:AlwaysOnSampler,remoteParentNotSampled:AlwaysOffSampler,localParentSampled:AlwaysOnSampler,localParentNotSampled:AlwaysOffSampler}", s)
}
Loading