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

The BuilderName and StrategyName now defaults to null #1251

Merged
merged 2 commits into from
Jun 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void Ctor_Defaults()
options.OnHalfOpened.Should().BeNull();
options.ShouldHandle.Should().BeNull();
options.StrategyType.Should().Be("CircuitBreaker");
options.StrategyName.Should().BeEmpty();
options.StrategyName.Should().BeNull();

// now set to min values
options.FailureThreshold = 0.001;
Expand All @@ -47,7 +47,7 @@ public void Ctor_Generic_Defaults()
options.OnHalfOpened.Should().BeNull();
options.ShouldHandle.Should().BeNull();
options.StrategyType.Should().Be("CircuitBreaker");
options.StrategyName.Should().BeEmpty();
options.StrategyName.Should().BeNull();

// now set to min values
options.FailureThreshold = 0.001;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void Ctor_Defaults()
options.OnHalfOpened.Should().BeNull();
options.ShouldHandle.Should().BeNull();
options.StrategyType.Should().Be("CircuitBreaker");
options.StrategyName.Should().BeEmpty();
options.StrategyName.Should().BeNull();

// now set to min values
options.FailureThreshold = 1;
Expand All @@ -42,7 +42,7 @@ public void Ctor_Generic_Defaults()
options.OnHalfOpened.Should().BeNull();
options.ShouldHandle.Should().BeNull();
options.StrategyType.Should().Be("CircuitBreaker");
options.StrategyName.Should().BeEmpty();
options.StrategyName.Should().BeNull();

// now set to min values
options.FailureThreshold = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class GenericResilienceStrategyBuilderTests
[Fact]
public void Ctor_EnsureDefaults()
{
_builder.BuilderName.Should().Be("");
_builder.BuilderName.Should().BeNull();
_builder.Properties.Should().NotBeNull();
_builder.TimeProvider.Should().Be(TimeProvider.System);
_builder.OnCreatingStrategy.Should().BeNull();
Expand Down
31 changes: 22 additions & 9 deletions src/Polly.Core.Tests/ResilienceStrategyBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public void Ctor_EnsureDefaults()
{
var builder = new ResilienceStrategyBuilder();

builder.BuilderName.Should().Be("");
builder.BuilderName.Should().BeNull();
builder.Properties.Should().NotBeNull();
builder.TimeProvider.Should().Be(TimeProvider.System);
builder.IsGenericBuilder.Should().BeFalse();
Expand Down Expand Up @@ -157,20 +157,17 @@ public void AddStrategy_AfterUsed_Throws()
[Fact]
public void Build_InvalidBuilderOptions_Throw()
{
var builder = new ResilienceStrategyBuilder
{
BuilderName = null!
};
var builder = new InvalidResilienceStrategyBuilder();

builder.Invoking(b => b.Build())
builder.Invoking(b => b.BuildStrategy())
.Should()
.Throw<ValidationException>()
.WithMessage(
"""
The 'ResilienceStrategyBuilder' configuration is invalid.

Validation Errors:
The BuilderName field is required.
The RequiredProperty field is required.
""");
}

Expand All @@ -180,15 +177,15 @@ public void AddStrategy_InvalidOptions_Throws()
var builder = new ResilienceStrategyBuilder();

builder
.Invoking(b => b.AddStrategy(_ => NullResilienceStrategy.Instance, new TestResilienceStrategyOptions { StrategyName = null! }))
.Invoking(b => b.AddStrategy(_ => NullResilienceStrategy.Instance, new InvalidResilienceStrategyOptions()))
.Should()
.Throw<ValidationException>()
.WithMessage(
"""
The 'ResilienceStrategyOptions' options are not valid.

Validation Errors:
The StrategyName field is required.
The RequiredProperty field is required.
""");
}

Expand Down Expand Up @@ -344,4 +341,20 @@ protected internal override async ValueTask<Outcome<TResult>> ExecuteCoreAsync<T
}
}
}

private class InvalidResilienceStrategyOptions : ResilienceStrategyOptions
{
[Required]
public string? RequiredProperty { get; set; }

public override string StrategyType => "Invalid";
}

private class InvalidResilienceStrategyBuilder : ResilienceStrategyBuilderBase
{
[Required]
public string? RequiredProperty { get; set; }

internal override bool IsGenericBuilder => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ public void Ctor_EnsureDefaults()
var options = new TestResilienceStrategyOptions();

options.StrategyType.Should().Be("Test");
options.StrategyName.Should().Be("");
options.StrategyName.Should().BeNull();
}
}
8 changes: 5 additions & 3 deletions src/Polly.Core/Strategy/ResilienceStrategyBuilderBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ private protected ResilienceStrategyBuilderBase(ResilienceStrategyBuilderBase ot
/// <summary>
/// Gets or sets the name of the builder.
/// </summary>
/// <remarks>This property is also included in the telemetry that is produced by the individual resilience strategies.</remarks>
[Required(AllowEmptyStrings = true)]
public string BuilderName { get; set; } = string.Empty;
/// <remarks>
/// This property is also included in the telemetry that is produced by the individual resilience strategies.
/// Defaults to <see langword="null"/>.
/// </remarks>
public string? BuilderName { get; set; }

/// <summary>
/// Gets the custom properties attached to builder options.
Expand Down
8 changes: 4 additions & 4 deletions src/Polly.Core/Strategy/ResilienceStrategyBuilderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ namespace Polly.Strategy;
public sealed class ResilienceStrategyBuilderContext
{
internal ResilienceStrategyBuilderContext(
string builderName,
string? builderName,
ResilienceProperties builderProperties,
string strategyName,
string? strategyName,
string strategyType,
TimeProvider timeProvider,
bool isGenericBuilder)
Expand All @@ -28,7 +28,7 @@ internal ResilienceStrategyBuilderContext(
/// <summary>
/// Gets the name of the builder.
/// </summary>
public string BuilderName { get; }
public string? BuilderName { get; }

/// <summary>
/// Gets the custom properties attached to the builder.
Expand All @@ -38,7 +38,7 @@ internal ResilienceStrategyBuilderContext(
/// <summary>
/// Gets the name of the strategy.
/// </summary>
public string StrategyName { get; }
public string? StrategyName { get; }

/// <summary>
/// Gets the type of the strategy.
Expand Down
7 changes: 2 additions & 5 deletions src/Polly.Core/Strategy/ResilienceStrategyOptions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using System.ComponentModel.DataAnnotations;

namespace Polly.Strategy;

/// <summary>
Expand All @@ -12,10 +10,9 @@ public abstract class ResilienceStrategyOptions
/// </summary>
/// <remarks>
/// This property is also included in the telemetry that is produced by the individual resilience strategies.
/// Defaults to <see cref="string.Empty"/>. This name uniquely identifies particular instance of specific strategy.
/// Defaults to <see langword="null"/>. This name uniquely identifies particular instance of specific strategy.
/// </remarks>
[Required(AllowEmptyStrings = true)]
public string StrategyName { get; set; } = string.Empty;
public string? StrategyName { get; set; }

/// <summary>
/// Gets the strategy type.
Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/Telemetry/ResilienceTelemetrySource.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace Polly.Telemetry;

internal sealed record class ResilienceTelemetrySource(string BuilderName, ResilienceProperties BuilderProperties, string StrategyName, string StrategyType);
internal sealed record class ResilienceTelemetrySource(string? BuilderName, ResilienceProperties BuilderProperties, string? StrategyName, string StrategyType);

2 changes: 1 addition & 1 deletion src/Polly.Core/Telemetry/TelemetryUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ internal static class TelemetryUtil

internal static readonly ResiliencePropertyKey<string> StrategyKey = new("StrategyKey");

public static ResilienceStrategyTelemetry CreateTelemetry(string builderName, ResilienceProperties builderProperties, string strategyName, string strategyType)
public static ResilienceStrategyTelemetry CreateTelemetry(string? builderName, ResilienceProperties builderProperties, string? strategyName, string strategyType)
{
builderProperties.TryGetValue(DiagnosticSourceKey, out var diagnosticSource);

Expand Down
28 changes: 14 additions & 14 deletions src/Polly.Extensions/Telemetry/Log.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,21 @@ internal static class Log
public static partial void ResilienceEvent(
this ILogger logger,
string eventName,
string builderName,
string strategyName,
string? builderName,
string? strategyName,
string strategyType,
string? strategyKey,
object? result,
Exception? exception);
#else
private static readonly Action<ILogger, string, string, string, string, string?, object?, Exception?> ResilienceEventAction =
LoggerMessage.Define<string, string, string, string, string?, object?>(LogLevel.Warning, new EventId(0, "ResilienceEvent"), ResilienceEventMessage);
private static readonly Action<ILogger, string, string?, string?, string, string?, object?, Exception?> ResilienceEventAction =
LoggerMessage.Define<string, string?, string?, string, string?, object?>(LogLevel.Warning, new EventId(0, "ResilienceEvent"), ResilienceEventMessage);

public static void ResilienceEvent(
this ILogger logger,
string eventName,
string builderName,
string strategyName,
string? builderName,
string? strategyName,
string strategyType,
string? strategyKey,
object? result,
Expand All @@ -66,16 +66,16 @@ public static void ResilienceEvent(
[LoggerMessage(1, LogLevel.Debug, StrategyExecutingMessage, EventName = "StrategyExecuting")]
public static partial void ExecutingStrategy(
this ILogger logger,
string builderName,
string? builderName,
string? strategyKey,
string resultType);
#else
private static readonly Action<ILogger, string, string?, string, Exception?> ExecutingStrategyAction =
LoggerMessage.Define<string, string?, string>(LogLevel.Debug, new EventId(1, "StrategyExecuting"), StrategyExecutingMessage);
private static readonly Action<ILogger, string?, string?, string, Exception?> ExecutingStrategyAction =
LoggerMessage.Define<string?, string?, string>(LogLevel.Debug, new EventId(1, "StrategyExecuting"), StrategyExecutingMessage);

public static void ExecutingStrategy(
this ILogger logger,
string builderName,
string? builderName,
string? strategyKey,
string resultType)
{
Expand All @@ -87,20 +87,20 @@ public static void ExecutingStrategy(
[LoggerMessage(2, LogLevel.Debug, StrategyExecutedMessage, EventName = "StrategyExecuted")]
public static partial void StrategyExecuted(
this ILogger logger,
string builderName,
string? builderName,
string? strategyKey,
string resultType,
object? result,
string executionHealth,
double executionTime,
Exception? exception);
#else
private static readonly Action<ILogger, string, string?, string, object?, string, double, Exception?> StrategyExecutedAction =
LoggerMessage.Define<string, string?, string, object?, string, double>(LogLevel.Debug, new EventId(2, "StrategyExecuted"), StrategyExecutedMessage);
private static readonly Action<ILogger, string?, string?, string, object?, string, double, Exception?> StrategyExecutedAction =
LoggerMessage.Define<string?, string?, string, object?, string, double>(LogLevel.Debug, new EventId(2, "StrategyExecuted"), StrategyExecutedMessage);

public static void StrategyExecuted(
this ILogger logger,
string builderName,
string? builderName,
string? strategyKey,
string resultType,
object? result,
Expand Down
4 changes: 2 additions & 2 deletions src/Polly.Extensions/Telemetry/TelemetryResilienceStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Polly.Extensions.Telemetry;
internal sealed class TelemetryResilienceStrategy : ResilienceStrategy
{
private readonly TimeProvider _timeProvider;
private readonly string _builderName;
private readonly string? _builderName;
private readonly string? _strategyKey;
private readonly List<Action<EnrichmentContext>> _enrichers;
private readonly ILogger _logger;
Expand All @@ -28,7 +28,7 @@ public TelemetryResilienceStrategy(

public TelemetryResilienceStrategy(
TimeProvider timeProvider,
string builderName,
string? builderName,
string? strategyKey,
ILoggerFactory loggerFactory,
List<Action<EnrichmentContext>> enrichers)
Expand Down