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

Made sampling based on UserId configurable #650

Closed
wants to merge 1 commit 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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This changelog will be used to generate documentation on [release notes page](http://azure.microsoft.com/en-us/documentation/articles/app-insights-release-notes-dotnet/).

## Version 2.5.0-beta2

- Turned off sampling based on UserId by default, made the use of this sample scoring configurable in the AdaptiveSamplingTelemetryProcessor

## Version 2.5.0-beta1
- Method `Sanitize` on classes implementing `ITelemetry` no longer modifies the `TelemetryContext` fields. Serialized event json and ETW event will still have context tags sanitized.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.AdaptiveSamplingTelemetryProcessor.AllowSamplingBasedOnUserId.get -> bool
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.AdaptiveSamplingTelemetryProcessor.AllowSamplingBasedOnUserId.set -> void
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.SamplingTelemetryProcessor.AllowSamplingBasedOnUserId.get -> bool
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.SamplingTelemetryProcessor.AllowSamplingBasedOnUserId.set -> void
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.ServerTelemetryChan
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.ServerTelemetryChannel.Send(Microsoft.ApplicationInsights.Channel.ITelemetry item) -> void
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.ServerTelemetryChannel.ServerTelemetryChannel() -> void
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.ServerTelemetryChannel.StorageFolder.get -> string
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.ServerTelemetryChannel.StorageFolder.set -> void
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.ServerTelemetryChannel.StorageFolder.set -> void
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.AdaptiveSamplingTelemetryProcessor.AllowSamplingBasedOnUserId.get -> bool
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.AdaptiveSamplingTelemetryProcessor.AllowSamplingBasedOnUserId.set -> void
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.SamplingTelemetryProcessor.AllowSamplingBasedOnUserId.get -> bool
Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.SamplingTelemetryProcessor.AllowSamplingBasedOnUserId.set -> void
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ public void SamplingScoreGeneratedUsingUserIdIfPresent()
requestTelemetry.Context.User.Id = userId;
requestTelemetry.Context.Operation.Id = GenerateRandomOperaitonId();

var eventTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(eventTelemetry);
var requestTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(requestTelemetry);
var eventTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(eventTelemetry, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update the name of the test method. IfPresentAndConfigured

var requestTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(requestTelemetry, true);

Assert.AreEqual(eventTelemetrySamplingScore, requestTelemetrySamplingScore, 12);
}
Expand All @@ -44,20 +44,46 @@ public void SamplingScoreGeneratedUsingOperationIdIfPresent()
var requestTelemetry = new RequestTelemetry();
requestTelemetry.Context.Operation.Id = operationId;

var eventTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(eventTelemetry);
var requestTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(requestTelemetry);
var eventTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(eventTelemetry, true);
var requestTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(requestTelemetry, true);

Assert.AreEqual(eventTelemetrySamplingScore, requestTelemetrySamplingScore, 12);
}

[TestMethod]
public void SamplingScoreGeneratedSkipsUserId()
{
string operationId = GenerateRandomOperaitonId();
string userId = GenerateRandomUserId();

var eventTelemetry = new EventTelemetry();
eventTelemetry.Context.Operation.Id = operationId;
eventTelemetry.Context.User.Id = userId;

var requestTelemetry = new RequestTelemetry();
requestTelemetry.Context.Operation.Id = operationId;
requestTelemetry.Context.User.Id = userId;

var eventTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(eventTelemetry, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ever test it with false?

var requestTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(requestTelemetry, true);

Assert.AreEqual(eventTelemetrySamplingScore, requestTelemetrySamplingScore, 12);

var eventTelemetrySamplingScoreNoUserConfig = SamplingScoreGenerator.GetSamplingScore(eventTelemetry, true);
var requestTelemetrySamplingScoreNoUserConfig = SamplingScoreGenerator.GetSamplingScore(requestTelemetry, true);

Assert.AreNotEqual(eventTelemetrySamplingScore, eventTelemetrySamplingScoreNoUserConfig);
Assert.AreNotEqual(requestTelemetrySamplingScore, requestTelemetrySamplingScoreNoUserConfig);
}

[TestMethod]
public void SamplingScoreIsRandomIfUserIdAndOperationIdAreNotPresent()
{
var eventTelemetry = new EventTelemetry();
var traceTelemetry = new TraceTelemetry();

var eventTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(eventTelemetry);
var traceTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(traceTelemetry);
var eventTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(eventTelemetry, true);
var traceTelemetrySamplingScore = SamplingScoreGenerator.GetSamplingScore(traceTelemetry, true);

Assert.AreNotEqual(eventTelemetrySamplingScore, traceTelemetrySamplingScore);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class AdaptiveSamplingTelemetryProcessor : ITelemetryProcessor, IDisposab
/// Fixed-rate sampling telemetry processor.
/// </summary>
private readonly SamplingTelemetryProcessor samplingProcessor;

/// <summary>
/// Sampling percentage estimator settings.
/// </summary>
Expand Down Expand Up @@ -220,6 +220,23 @@ public double MovingAverageRatio
}
}

/// <summary>
/// Gets or sets a value indicating whether to enable sample scoring for telemetry items based on the Context.User.Id flag. Default
/// is false.
/// </summary>
public bool AllowSamplingBasedOnUserId
{
get
{
return this.samplingProcessor.AllowSamplingBasedOnUserId;
}

set
{
this.samplingProcessor.AllowSamplingBasedOnUserId = value;
}
}

/// <summary>
/// Processes telemetry item.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ internal static class SamplingScoreGenerator
/// Generates telemetry sampling score between 0 and 100.
/// </summary>
/// <param name="telemetry">Telemetry item to score.</param>
/// <param name="allowUserId">Sampling score can be determined by using the Context.User.Id information, default is false.</param>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add remark that is can be used for backward compatibility

/// <returns>Item sampling score.</returns>
public static double GetSamplingScore(ITelemetry telemetry)
public static double GetSamplingScore(ITelemetry telemetry, bool allowUserId = false)
{
double samplingScore = 0;

if (telemetry.Context.User.Id != null)
if (allowUserId && telemetry.Context.User.Id != null)
{
samplingScore = (double)telemetry.Context.User.Id.GetSamplingHashCode() / int.MaxValue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public SamplingTelemetryProcessor(ITelemetryProcessor next)
{ RequestTelemetryName, typeof(RequestTelemetry) },
{ TraceTelemetryName, typeof(TraceTelemetry) },
};
this.AllowSamplingBasedOnUserId = false;
}

/// <summary>
Expand Down Expand Up @@ -124,6 +125,17 @@ public string IncludedTypes
}
}

/// <summary>
/// Gets or sets a value indicating whether or not we should calculate sampling based on the Context.User.Id field.
/// Default is false.
///
/// <remarks>
/// Calculation of the sampling score based on user_Id field is disabled by default to ensure successful correlation across many
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why are you switching to Kusto names of the fields in remark?

/// services on operation_Id field.
/// </remarks>
/// </summary>
public bool AllowSamplingBasedOnUserId { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow is not the best verb here. Maybe Calculate like in CalculateSamplingScoreFromUserId or Use like in UseUserIdSamplingScore


/// <summary>
/// Gets or sets data sampling percentage (between 0 and 100) for all <see cref="ITelemetry"/>
/// objects logged in this <see cref="TelemetryClient"/>.
Expand Down Expand Up @@ -187,7 +199,7 @@ public void Process(ITelemetry item)
//// Ok, now we can actually sample:

samplingSupportingTelemetry.SamplingPercentage = samplingPercentage;
bool isSampledIn = SamplingScoreGenerator.GetSamplingScore(item) < samplingPercentage;
bool isSampledIn = SamplingScoreGenerator.GetSamplingScore(item, this.AllowSamplingBasedOnUserId) < samplingPercentage;

if (isSampledIn)
{
Expand Down