Skip to content

Commit

Permalink
[Sampler.AWS] Wrapped root sampler with ParentBasedSampler (open-tele…
Browse files Browse the repository at this point in the history
…metry#2188)

Co-authored-by: Piotr Kiełkowicz <[email protected]>
  • Loading branch information
AsakerMohd and Kielek authored Nov 18, 2024
1 parent ad5d037 commit 3e8b607
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 21 deletions.
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
8 changes: 5 additions & 3 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,10 +67,11 @@ 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()
{
return new AWSXRayRemoteSampler(this.resource, this.pollingInterval, this.endpoint, this.clock);
var rootSampler = new AWSXRayRemoteSampler(this.resource, this.pollingInterval, this.endpoint, this.clock);
return new ParentBasedSampler(rootSampler);
}

// This is intended for testing with a mock clock.
Expand Down
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Sampler.AWS/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
* 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
which is of type `Sampler` instead of `AWSXRayRemoteSampler`.
([#2188](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2188))

## 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
30 changes: 19 additions & 11 deletions test/OpenTelemetry.Sampler.AWS.Tests/TestAWSXRayRemoteSampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

using System.Diagnostics;
using System.Reflection;
using OpenTelemetry.Resources;
using OpenTelemetry.Trace;
using WireMock.RequestBuilders;
Expand All @@ -18,27 +19,34 @@ public void TestSamplerWithConfiguration()
{
var pollingInterval = TimeSpan.FromSeconds(5);
var endpoint = "http://localhost:3000";

var sampler = AWSXRayRemoteSampler.Builder(ResourceBuilder.CreateEmpty().Build())
var parentBasedSampler = AWSXRayRemoteSampler.Builder(ResourceBuilder.CreateEmpty().Build())
.SetPollingInterval(pollingInterval)
.SetEndpoint(endpoint)
.Build();

Assert.Equal(pollingInterval, sampler.PollingInterval);
Assert.Equal(endpoint, sampler.Endpoint);
Assert.NotNull(sampler.RulePollerTimer);
Assert.NotNull(sampler.Client);
FieldInfo? rootSamplerFieldInfo = typeof(ParentBasedSampler).GetField("rootSampler", BindingFlags.NonPublic | BindingFlags.Instance);

var xraySampler = (AWSXRayRemoteSampler?)rootSamplerFieldInfo?.GetValue(parentBasedSampler);

Assert.Equal(pollingInterval, xraySampler?.PollingInterval);
Assert.Equal(endpoint, xraySampler?.Endpoint);
Assert.NotNull(xraySampler?.RulePollerTimer);
Assert.NotNull(xraySampler?.Client);
}

[Fact]
public void TestSamplerWithDefaults()
{
var sampler = AWSXRayRemoteSampler.Builder(ResourceBuilder.CreateEmpty().Build()).Build();
var parentBasedSampler = AWSXRayRemoteSampler.Builder(ResourceBuilder.CreateEmpty().Build()).Build();

FieldInfo? rootSamplerFieldInfo = typeof(ParentBasedSampler).GetField("rootSampler", BindingFlags.NonPublic | BindingFlags.Instance);

var xraySampler = (AWSXRayRemoteSampler?)rootSamplerFieldInfo?.GetValue(parentBasedSampler);

Assert.Equal(TimeSpan.FromMinutes(5), sampler.PollingInterval);
Assert.Equal("http://localhost:2000", sampler.Endpoint);
Assert.NotNull(sampler.RulePollerTimer);
Assert.NotNull(sampler.Client);
Assert.Equal(TimeSpan.FromMinutes(5), xraySampler?.PollingInterval);
Assert.Equal("http://localhost:2000", xraySampler?.Endpoint);
Assert.NotNull(xraySampler?.RulePollerTimer);
Assert.NotNull(xraySampler?.Client);
}

[Fact(Skip = "Flaky test. Related issue: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1219")]
Expand Down

0 comments on commit 3e8b607

Please sign in to comment.