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

Renaming SamplingDecision options #1297

Merged
merged 2 commits into from
Sep 21, 2020
Merged
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
2 changes: 1 addition & 1 deletion docs/trace/extending-the-sdk/MySampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ internal class MySampler : Sampler
public override SamplingResult ShouldSample(in SamplingParameters param)
{
Console.WriteLine($"MySampler.ShouldSample({param.Name})");
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
}
}
3 changes: 3 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
boolean value
([#1282](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1282))
([#1285](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1285))
* Renamed `SamplingDecision` options (`NotRecord` to `Drop`, `Record` to
`RecordOnly`, and `RecordAndSampled` to `RecordAndSample`)
([#1297](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1297))

## 0.6.0-beta.1

Expand Down
8 changes: 4 additions & 4 deletions src/OpenTelemetry/Trace/ActivitySourceAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,19 +163,19 @@ private void RunGetRequestedDataOtherSampler(Activity activity)

switch (samplingResult.Decision)
{
case SamplingDecision.NotRecord:
case SamplingDecision.Drop:
activity.IsAllDataRequested = false;
break;
case SamplingDecision.Record:
case SamplingDecision.RecordOnly:
activity.IsAllDataRequested = true;
break;
case SamplingDecision.RecordAndSampled:
case SamplingDecision.RecordAndSample:
activity.IsAllDataRequested = true;
activity.ActivityTraceFlags |= ActivityTraceFlags.Recorded;
break;
}

if (samplingResult.Decision != SamplingDecision.NotRecord)
if (samplingResult.Decision != SamplingDecision.Drop)
{
foreach (var att in samplingResult.Attributes)
{
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Trace/AlwaysOffSampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public sealed class AlwaysOffSampler : Sampler
/// <inheritdoc />
public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
{
return new SamplingResult(SamplingDecision.NotRecord);
return new SamplingResult(SamplingDecision.Drop);
}
}
}
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Trace/AlwaysOnSampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public sealed class AlwaysOnSampler : Sampler
/// <inheritdoc />
public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
{
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
}
}
}
6 changes: 3 additions & 3 deletions src/OpenTelemetry/Trace/ParentBasedSampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public override SamplingResult ShouldSample(in SamplingParameters samplingParame
// If the parent is sampled keep the sampling decision.
if ((parentContext.TraceFlags & ActivityTraceFlags.Recorded) != 0)
{
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
}

if (samplingParameters.Links != null)
Expand All @@ -60,13 +60,13 @@ public override SamplingResult ShouldSample(in SamplingParameters samplingParame
{
if ((parentLink.Context.TraceFlags & ActivityTraceFlags.Recorded) != 0)
{
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
}
}
}

// If parent was not sampled, do not sample.
return new SamplingResult(SamplingDecision.NotRecord);
return new SamplingResult(SamplingDecision.Drop);
}
}
}
6 changes: 3 additions & 3 deletions src/OpenTelemetry/Trace/SamplingDecision.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ public enum SamplingDecision
/// The activity object needs to be created. It will have Name, Source, Id and Baggage.
/// Other properties will be ignored.
/// </summary>
NotRecord,
Drop,

/// <summary>
/// The activity object should be populated with all the propagation info and also all other
/// properties such as Links, Tags, and Events. Activity.IsAllDataRequested will return true.
/// </summary>
Record,
RecordOnly,

/// <summary>
/// The activity object should be populated with all the propagation info and also all other
/// properties such as Links, Tags, and Events.
/// Both Activity.IsAllDataRequested and Activity.IsRecorded will return true.
/// </summary>
RecordAndSampled,
RecordAndSample,
}
}
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Trace/SamplingResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public SamplingResult(SamplingDecision decision)
/// <param name="isSampled"> True if sampled, false otherwise.</param>
public SamplingResult(bool isSampled)
{
this.Decision = isSampled ? SamplingDecision.RecordAndSampled : SamplingDecision.NotRecord;
this.Decision = isSampled ? SamplingDecision.RecordAndSample : SamplingDecision.Drop;
this.Attributes = Enumerable.Empty<KeyValuePair<string, object>>();
}

Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry/Trace/TracerProviderSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ private static ActivitySamplingResult ComputeActivitySamplingResult(

var activitySamplingResult = shouldSample.Decision switch
{
SamplingDecision.RecordAndSampled => ActivitySamplingResult.AllDataAndRecorded,
SamplingDecision.Record => ActivitySamplingResult.AllData,
SamplingDecision.RecordAndSample => ActivitySamplingResult.AllDataAndRecorded,
SamplingDecision.RecordOnly => ActivitySamplingResult.AllData,
_ => ActivitySamplingResult.PropagationData
};

Expand Down
2 changes: 1 addition & 1 deletion test/OpenTelemetry.Tests/Shared/TestSampler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ internal class TestSampler : Sampler
public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
{
this.LatestSamplingParameters = samplingParameters;
return this.SamplingAction?.Invoke(samplingParameters) ?? new SamplingResult(SamplingDecision.RecordAndSampled);
return this.SamplingAction?.Invoke(samplingParameters) ?? new SamplingResult(SamplingDecision.RecordAndSample);
}
}
}
26 changes: 13 additions & 13 deletions test/OpenTelemetry.Tests/Trace/ActivitySourceAdapterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ public void ActivitySourceAdapterSetsKind(ActivityKind kind)
}

[Theory]
[InlineData(SamplingDecision.NotRecord)]
[InlineData(SamplingDecision.Record)]
[InlineData(SamplingDecision.RecordAndSampled)]
[InlineData(SamplingDecision.Drop)]
[InlineData(SamplingDecision.RecordOnly)]
[InlineData(SamplingDecision.RecordAndSample)]
public void ActivitySourceAdapterCallsStartStopActivityProcessor1(SamplingDecision decision)
{
this.testSampler.SamplingAction = (samplingParameters) =>
Expand All @@ -104,9 +104,9 @@ public void ActivitySourceAdapterCallsStartStopActivityProcessor1(SamplingDecisi

// If start is called, that means activity is sampled,
// and TraceFlag is set to Recorded.
Assert.Equal(decision == SamplingDecision.Record || decision == SamplingDecision.RecordAndSampled, a.IsAllDataRequested);
Assert.Equal(decision == SamplingDecision.RecordAndSampled ? ActivityTraceFlags.Recorded : ActivityTraceFlags.None, a.ActivityTraceFlags);
Assert.Equal(decision == SamplingDecision.RecordAndSampled, a.Recorded);
Assert.Equal(decision == SamplingDecision.RecordOnly || decision == SamplingDecision.RecordAndSample, a.IsAllDataRequested);
Assert.Equal(decision == SamplingDecision.RecordAndSample ? ActivityTraceFlags.Recorded : ActivityTraceFlags.None, a.ActivityTraceFlags);
Assert.Equal(decision == SamplingDecision.RecordAndSample, a.Recorded);
};

this.testProcessor.EndAction =
Expand Down Expand Up @@ -167,9 +167,9 @@ public void ActivitySourceAdapterCallsStartStopActivityProcessor2(bool isSampled
}

[Theory]
[InlineData(SamplingDecision.NotRecord)]
[InlineData(SamplingDecision.Record)]
[InlineData(SamplingDecision.RecordAndSampled)]
[InlineData(SamplingDecision.Drop)]
[InlineData(SamplingDecision.RecordOnly)]
[InlineData(SamplingDecision.RecordAndSample)]
public void ActivitySourceAdapterPopulatesSamplingAttributesToActivity(SamplingDecision sampling)
{
this.testSampler.SamplingAction = (samplingParams) =>
Expand All @@ -182,7 +182,7 @@ public void ActivitySourceAdapterPopulatesSamplingAttributesToActivity(SamplingD
var activity = new Activity("test");
activity.Start();
this.activitySourceAdapter.Start(activity, ActivityKind.Internal);
if (sampling != SamplingDecision.NotRecord)
if (sampling != SamplingDecision.Drop)
{
Assert.Contains(new KeyValuePair<string, object>("tagkeybysampler", "tagvalueaddedbysampler"), activity.TagObjects);
}
Expand All @@ -196,7 +196,7 @@ public void ActivitySourceAdapterPopulatesSamplingParamsCorrectlyForRootActivity
this.testSampler.SamplingAction = (samplingParameters) =>
{
Assert.Equal(default, samplingParameters.ParentContext);
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
};

// Start activity without setting parent. i.e it'll have null parent
Expand Down Expand Up @@ -225,7 +225,7 @@ public void ActivitySourceAdapterPopulatesSamplingParamsCorrectlyForActivityWith
Assert.Equal(parentSpanId, samplingParameters.ParentContext.SpanId);
Assert.Equal(traceFlags, samplingParameters.ParentContext.TraceFlags);
Assert.Equal(tracestate, samplingParameters.ParentContext.TraceState);
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
};

// Create an activity with remote parent id.
Expand Down Expand Up @@ -258,7 +258,7 @@ public void ActivitySourceAdapterPopulatesSamplingParamsCorrectlyForActivityWith
Assert.Equal(activityLocalParent.ActivityTraceFlags, samplingParameters.ParentContext.TraceFlags);
Assert.Equal(tracestate, samplingParameters.ParentContext.TraceState);
Assert.Equal(ActivityKind.Client, samplingParameters.Kind);
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
};

// This activity will have a inproc parent.
Expand Down
12 changes: 6 additions & 6 deletions test/OpenTelemetry.Tests/Trace/ParentBasedSamplerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ public void SampledParent()
{
// No parent, use delegate sampler.
Assert.Equal(
new SamplingResult(SamplingDecision.RecordAndSampled),
new SamplingResult(SamplingDecision.RecordAndSample),
this.parentBasedOnSampler.ShouldSample(default));

// No parent, use delegate sampler.
Assert.Equal(
new SamplingResult(SamplingDecision.NotRecord),
new SamplingResult(SamplingDecision.Drop),
this.parentBasedOffSampler.ShouldSample(default));

// Not sampled parent, don't sample.
Assert.Equal(
new SamplingResult(SamplingDecision.NotRecord),
new SamplingResult(SamplingDecision.Drop),
this.parentBasedOnSampler.ShouldSample(
new SamplingParameters(
parentContext: new ActivityContext(
Expand All @@ -51,7 +51,7 @@ public void SampledParent()

// Sampled parent, sample.
Assert.Equal(
new SamplingResult(SamplingDecision.RecordAndSampled),
new SamplingResult(SamplingDecision.RecordAndSample),
this.parentBasedOffSampler.ShouldSample(
new SamplingParameters(
parentContext: new ActivityContext(
Expand Down Expand Up @@ -91,7 +91,7 @@ public void SampledParentLink()

// Not sampled link, don't sample.
Assert.Equal(
new SamplingResult(SamplingDecision.NotRecord),
new SamplingResult(SamplingDecision.Drop),
this.parentBasedOnSampler.ShouldSample(
new SamplingParameters(
parentContext: notSampledParent,
Expand All @@ -102,7 +102,7 @@ public void SampledParentLink()

// Sampled link, sample.
Assert.Equal(
new SamplingResult(SamplingDecision.RecordAndSampled),
new SamplingResult(SamplingDecision.RecordAndSample),
this.parentBasedOffSampler.ShouldSample(
new SamplingParameters(
parentContext: notSampledParent,
Expand Down
4 changes: 2 additions & 2 deletions test/OpenTelemetry.Tests/Trace/SamplersTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void AlwaysOnSampler_AlwaysReturnTrue(ActivityTraceFlags flags)
var link = new ActivityLink(parentContext);

Assert.Equal(
SamplingDecision.RecordAndSampled,
SamplingDecision.RecordAndSample,
new AlwaysOnSampler().ShouldSample(new SamplingParameters(parentContext, this.traceId, "Another name", ActivityKindServer, null, new List<ActivityLink> { link })).Decision);
}

Expand All @@ -61,7 +61,7 @@ public void AlwaysOffSampler_AlwaysReturnFalse(ActivityTraceFlags flags)
var link = new ActivityLink(parentContext);

Assert.Equal(
SamplingDecision.NotRecord,
SamplingDecision.Drop,
new AlwaysOffSampler().ShouldSample(new SamplingParameters(parentContext, this.traceId, "Another name", ActivityKindServer, null, new List<ActivityLink> { link })).Decision);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void SampleBasedOnTraceId()
0,
});
Assert.Equal(
SamplingDecision.NotRecord,
SamplingDecision.Drop,
defaultProbability.ShouldSample(new SamplingParameters(default, notSampledtraceId, ActivityDisplayName, ActivityKindServer, null, null)).Decision);

// This traceId will be sampled by the TraceIdRatioBasedSampler because the first 8 bytes as long
Expand All @@ -92,7 +92,7 @@ public void SampleBasedOnTraceId()
0,
});
Assert.Equal(
SamplingDecision.RecordAndSampled,
SamplingDecision.RecordAndSample,
defaultProbability.ShouldSample(new SamplingParameters(default, sampledtraceId, ActivityDisplayName, ActivityKindServer, null, null)).Decision);
}

Expand Down
16 changes: 8 additions & 8 deletions test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ public void TracerProviderSdkInvokesSamplingWithCorrectParameters()
}

[Theory]
[InlineData(SamplingDecision.NotRecord)]
[InlineData(SamplingDecision.Record)]
[InlineData(SamplingDecision.RecordAndSampled)]
[InlineData(SamplingDecision.Drop)]
[InlineData(SamplingDecision.RecordOnly)]
[InlineData(SamplingDecision.RecordAndSample)]
public void TracerProviderSdkSamplerAttributesAreAppliedToActivity(SamplingDecision sampling)
{
var testSampler = new TestSampler();
Expand All @@ -127,7 +127,7 @@ public void TracerProviderSdkSamplerAttributesAreAppliedToActivity(SamplingDecis
{
Assert.NotNull(rootActivity);
Assert.Equal(rootActivity.TraceId, testSampler.LatestSamplingParameters.TraceId);
if (sampling != SamplingDecision.NotRecord)
if (sampling != SamplingDecision.Drop)
{
Assert.Contains(new KeyValuePair<string, object>("tagkeybysampler", "tagvalueaddedbysampler"), rootActivity.TagObjects);
}
Expand All @@ -146,7 +146,7 @@ public void TracerSdkSetsActivitySamplingResultBasedOnSamplingDecision()

testSampler.SamplingAction = (samplingParameters) =>
{
return new SamplingResult(SamplingDecision.RecordAndSampled);
return new SamplingResult(SamplingDecision.RecordAndSample);
};

using (var activity = activitySource.StartActivity("root"))
Expand All @@ -158,7 +158,7 @@ public void TracerSdkSetsActivitySamplingResultBasedOnSamplingDecision()

testSampler.SamplingAction = (samplingParameters) =>
{
return new SamplingResult(SamplingDecision.Record);
return new SamplingResult(SamplingDecision.RecordOnly);
};

using (var activity = activitySource.StartActivity("root"))
Expand All @@ -172,7 +172,7 @@ public void TracerSdkSetsActivitySamplingResultBasedOnSamplingDecision()

testSampler.SamplingAction = (samplingParameters) =>
{
return new SamplingResult(SamplingDecision.NotRecord);
return new SamplingResult(SamplingDecision.Drop);
};

using (var activity = activitySource.StartActivity("root"))
Expand Down Expand Up @@ -239,7 +239,7 @@ public void ProcessorDoesNotReceiveNotRecordDecisionSpan()

testSampler.SamplingAction = (samplingParameters) =>
{
return new SamplingResult(SamplingDecision.NotRecord);
return new SamplingResult(SamplingDecision.Drop);
};

using ActivitySource source = new ActivitySource("random");
Expand Down