-
Notifications
You must be signed in to change notification settings - Fork 839
Closed
Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-telemetry
Description
Background and motivation
The use of AddProbabilisticSampler()
extension methods was approved here, but later we found out that the Probabilistic
word might collide with some OpenTelemetry .NET API. Therefore, we propose to rename everything Probabilistic
to RandomProbabilistic
.
API Proposal
namespace Microsoft.Extensions.Logging
{
public static class SamplingLoggerBuilderExtensions
{
- public static ILoggingBuilder AddProbabilisticSampler(this ILoggingBuilder builder, IConfiguration configuration);
- public static ILoggingBuilder AddProbabilisticSampler(this ILoggingBuilder builder, Action<ProbabilisticSamplerOptions> configure);
- public static ILoggingBuilder AddProbabilisticSampler(this ILoggingBuilder builder, double probability, LogLevel? level = null);
+ public static ILoggingBuilder AddRandomProbabilisticSampler(this ILoggingBuilder builder, IConfiguration configuration);
+ public static ILoggingBuilder AddRandomProbabilisticSampler(this ILoggingBuilder builder, Action<RandomProbabilisticSamplerOptions> configure);
+ public static ILoggingBuilder AddRandomProbabilisticSampler(this ILoggingBuilder builder, double probability, LogLevel? level = null);
}
}
namespace Microsoft.Extensions.Diagnostics.Sampling
{
- public class ProbabilisticSamplerOptions
+ public class RandomProbabilisticSamplerOptions
{
- public IList<ProbabilisticSamplerFilterRule> Rules { get; set; } = [];
+ public IList<RandomProbabilisticSamplerFilterRule> Rules { get; set; } = [];
}
- public class ProbabilisticSamplerFilterRule
+ public class RandomProbabilisticSamplerFilterRule
{
- public ProbabilisticSamplerFilterRule(
+ public RandomProbabilisticSamplerFilterRule(
double probability,
string? categoryName = null,
LogLevel? logLevel = null,
int? eventId = null,
string? eventName = null)
{
Probability = probability;
CategoryName = categoryName;
LogLevel = logLevel;
EventId = eventId;
EventName = eventName;
}
public double Probability { get; }
public string? CategoryName { get; }
public LogLevel? LogLevel { get; }
public int? EventId { get; }
public string? EventName { get; }
}
}
API Usage
-loggingBuilder.AddProbabilisticSampler(0.1, LogLevel.Information);
+loggingBuilder.AddRandomProbabilisticSampler(0.1, LogLevel.Information);
Alternative Designs
Use the Random
word instead of the Probabilistic
, but this is probably very generic and does not provide enough information about what the API is doing:
namespace Microsoft.Extensions.Logging
{
public static class SamplingLoggerBuilderExtensions
{
- public static ILoggingBuilder AddProbabilisticSampler(this ILoggingBuilder builder, IConfiguration configuration);
- public static ILoggingBuilder AddProbabilisticSampler(this ILoggingBuilder builder, Action<ProbabilisticSamplerOptions> configure);
- public static ILoggingBuilder AddProbabilisticSampler(this ILoggingBuilder builder, double probability, LogLevel? level = null);
+ public static ILoggingBuilder AddRandomSampler(this ILoggingBuilder builder, IConfiguration configuration);
+ public static ILoggingBuilder AddRandomSampler(this ILoggingBuilder builder, Action<RandomSamplerOptions> configure);
+ public static ILoggingBuilder AddRandomSampler(this ILoggingBuilder builder, double probability, LogLevel? level = null);
}
}
Risks
No risks, the API is brand new.
In fact, with the proposal, we are trying to avoid the naming collision risk with OpenTelemetry .NET
Metadata
Metadata
Assignees
Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-telemetry