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

[Sampler.AWS] Wrapped root sampler with ParentBasedSampler #2188

Merged
merged 14 commits into from
Nov 18, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
OpenTelemetry.Sampler.AWS.AWSXRayRemoteSampler
OpenTelemetry.Sampler.AWS.AWSXRayRemoteSampler.Dispose() -> void
OpenTelemetry.Sampler.AWS.AWSXRayRemoteSamplerBuilder
OpenTelemetry.Sampler.AWS.AWSXRayRemoteSamplerBuilder.Build() -> OpenTelemetry.Sampler.AWS.AWSXRayRemoteSampler!
OpenTelemetry.Sampler.AWS.AWSXRayRemoteSamplerBuilder.Build() -> OpenTelemetry.Trace.Sampler!
OpenTelemetry.Sampler.AWS.AWSXRayRemoteSamplerBuilder.SetEndpoint(string! endpoint) -> OpenTelemetry.Sampler.AWS.AWSXRayRemoteSamplerBuilder!
OpenTelemetry.Sampler.AWS.AWSXRayRemoteSamplerBuilder.SetPollingInterval(System.TimeSpan pollingInterval) -> OpenTelemetry.Sampler.AWS.AWSXRayRemoteSamplerBuilder!
override OpenTelemetry.Sampler.AWS.AWSXRayRemoteSampler.ShouldSample(in OpenTelemetry.Trace.SamplingParameters samplingParameters) -> OpenTelemetry.Trace.SamplingResult
Expand Down
14 changes: 12 additions & 2 deletions src/OpenTelemetry.Sampler.AWS/AWSXRayRemoteSamplerBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

using OpenTelemetry.Resources;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Sampler.AWS;

Expand Down Expand Up @@ -66,8 +67,17 @@ public AWSXRayRemoteSamplerBuilder SetEndpoint(string endpoint)
/// <summary>
/// Returns a <see cref="AWSXRayRemoteSampler"/> with configuration of this builder.
/// </summary>
/// <returns>an instance of <see cref="AWSXRayRemoteSampler"/>.</returns>
public AWSXRayRemoteSampler Build()
/// <returns>an instance of <see cref="Trace.Sampler"/>.</returns>
public Trace.Sampler Build()
{
using var rootSampler = this.BuildXraySampler();
AsakerMohd marked this conversation as resolved.
Show resolved Hide resolved
return new ParentBasedSampler(rootSampler);
}

// This is intended for testing to check that the XRayRemoteSampler is built with the correct attributes
// Should not be exposed to public as the public build method should return the sampler wrapped inside
// ParentBasedSampler.
internal AWSXRayRemoteSampler BuildXraySampler()
AsakerMohd marked this conversation as resolved.
Show resolved Hide resolved
{
return new AWSXRayRemoteSampler(this.resource, this.pollingInterval, this.endpoint, this.clock);
}
Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Sampler.AWS/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* Updated OpenTelemetry core component version(s) to `1.10.0`.
([#2317](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2317))

* `AWSXRayRemoteSamplerBuilder.Build()` now returns `ParentBasedSampler` which is of type `Trace.Sampler` instead of `AWSXRayRemoteSampler`

Check failure on line 20 in src/OpenTelemetry.Sampler.AWS/CHANGELOG.md

View workflow job for this annotation

GitHub Actions / lint-md / run-markdownlint

Line length

src/OpenTelemetry.Sampler.AWS/CHANGELOG.md:20:81 MD013/line-length Line length [Expected: 80; Actual: 139] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md013.md

## 0.1.0-alpha.2

Released 2024-Sep-09
Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry.Sampler.AWS/FallbackSampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ internal class FallbackSampler : Trace.Sampler

public FallbackSampler(Clock clock)
{
this.reservoirSampler = new ParentBasedSampler(new RateLimitingSampler(1, clock));
this.fixedRateSampler = new ParentBasedSampler(new TraceIdRatioBasedSampler(0.05));
this.reservoirSampler = new RateLimitingSampler(1, clock);
this.fixedRateSampler = new TraceIdRatioBasedSampler(0.05);
}

public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
Expand Down
8 changes: 4 additions & 4 deletions src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public SamplingRuleApplier(string clientId, Clock clock, SamplingRule rule, Stat
{
// Until calling GetSamplingTargets, the default is to borrow 1/s if reservoir size is
// positive.
this.ReservoirSampler = new ParentBasedSampler(new RateLimitingSampler(1, this.Clock));
this.ReservoirSampler = new RateLimitingSampler(1, this.Clock);
this.Borrowing = true;
}
else
Expand All @@ -30,7 +30,7 @@ public SamplingRuleApplier(string clientId, Clock clock, SamplingRule rule, Stat
this.Borrowing = false;
}

this.FixedRateSampler = new ParentBasedSampler(new TraceIdRatioBasedSampler(rule.FixedRate));
this.FixedRateSampler = new TraceIdRatioBasedSampler(rule.FixedRate);

// We either have no reservoir sampling or borrow until we get a quota so have no end time.
this.ReservoirEndTime = DateTimeOffset.MaxValue;
Expand Down Expand Up @@ -191,15 +191,15 @@ public SamplingStatisticsDocument Snapshot(DateTimeOffset now)
public SamplingRuleApplier WithTarget(SamplingTargetDocument target, DateTimeOffset now)
{
var newFixedRateSampler = target.FixedRate != null
? new ParentBasedSampler(new TraceIdRatioBasedSampler(target.FixedRate.Value))
? new TraceIdRatioBasedSampler(target.FixedRate.Value)
: this.FixedRateSampler;

Trace.Sampler newReservoirSampler = new AlwaysOffSampler();
var newReservoirEndTime = DateTimeOffset.MaxValue;
if (target.ReservoirQuota != null && target.ReservoirQuotaTTL != null)
{
newReservoirSampler = target.ReservoirQuota > 0
? new ParentBasedSampler(new RateLimitingSampler(target.ReservoirQuota.Value, this.Clock))
? new RateLimitingSampler(target.ReservoirQuota.Value, this.Clock)
: new AlwaysOffSampler();

newReservoirEndTime = this.Clock.ToDateTime(target.ReservoirQuotaTTL.Value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public void TestSamplerWithConfiguration()
var sampler = AWSXRayRemoteSampler.Builder(ResourceBuilder.CreateEmpty().Build())
.SetPollingInterval(pollingInterval)
.SetEndpoint(endpoint)
.Build();
.BuildXraySampler();

Assert.Equal(pollingInterval, sampler.PollingInterval);
Assert.Equal(endpoint, sampler.Endpoint);
Expand All @@ -33,7 +33,7 @@ public void TestSamplerWithConfiguration()
[Fact]
public void TestSamplerWithDefaults()
{
var sampler = AWSXRayRemoteSampler.Builder(ResourceBuilder.CreateEmpty().Build()).Build();
var sampler = AWSXRayRemoteSampler.Builder(ResourceBuilder.CreateEmpty().Build()).BuildXraySampler();

Assert.Equal(TimeSpan.FromMinutes(5), sampler.PollingInterval);
Assert.Equal("http://localhost:2000", sampler.Endpoint);
Expand All @@ -53,7 +53,7 @@ public void TestSamplerUpdateAndSample()
.SetPollingInterval(TimeSpan.FromMilliseconds(10))
.SetEndpoint(mockServer.Url!)
.SetClock(clock)
.Build();
.BuildXraySampler();

// the sampler will use fallback sampler until rules are fetched.
Assert.Equal(SamplingDecision.RecordAndSample, this.DoSample(sampler, "cat-service"));
Expand Down
Loading