API Review Feedback (#1506)

bench/Polly.Core.Benchmarks/BridgeBenchmark.cs

@@ -9,7 +9,7 @@ public class BridgeBenchmark
     public void Setup()
     {
         _policy = Policy.NoOpAsync<string>();
-        _policyWrapped = ResiliencePipeline<string>.Null.AsAsyncPolicy();
+        _policyWrapped = ResiliencePipeline<string>.Empty.AsAsyncPolicy();
     }
 
     [Benchmark(Baseline = true)]

bench/Polly.Core.Benchmarks/ResiliencePipelineBenchmark.cs

@@ -10,48 +10,48 @@ public class ResiliencePipelineBenchmark
     public async ValueTask ExecuteOutcomeAsync()
     {
         var context = ResilienceContextPool.Shared.Get();
-        await ResiliencePipeline.Null.ExecuteOutcomeAsync((_, _) => Outcome.FromResultAsTask("dummy"), context, "state").ConfigureAwait(false);
+        await ResiliencePipeline.Empty.ExecuteOutcomeAsync((_, _) => Outcome.FromResultAsTask("dummy"), context, "state").ConfigureAwait(false);
         ResilienceContextPool.Shared.Return(context);
     }
 
     [Benchmark]
     public async ValueTask ExecuteAsync_ResilienceContextAndState()
     {
         var context = ResilienceContextPool.Shared.Get();
-        await ResiliencePipeline.Null.ExecuteAsync((_, _) => new ValueTask<string>("dummy"), context, "state").ConfigureAwait(false);
+        await ResiliencePipeline.Empty.ExecuteAsync((_, _) => new ValueTask<string>("dummy"), context, "state").ConfigureAwait(false);
         ResilienceContextPool.Shared.Return(context);
     }
 
     [Benchmark]
     public async ValueTask ExecuteAsync_CancellationToken()
     {
-        await ResiliencePipeline.Null.ExecuteAsync(_ => new ValueTask<string>("dummy"), CancellationToken.None).ConfigureAwait(false);
+        await ResiliencePipeline.Empty.ExecuteAsync(_ => new ValueTask<string>("dummy"), CancellationToken.None).ConfigureAwait(false);
     }
 
     [Benchmark]
     public async ValueTask ExecuteAsync_GenericStrategy_CancellationToken()
     {
-        await ResiliencePipeline<string>.Null.ExecuteAsync(_ => new ValueTask<string>("dummy"), CancellationToken.None).ConfigureAwait(false);
+        await ResiliencePipeline<string>.Empty.ExecuteAsync(_ => new ValueTask<string>("dummy"), CancellationToken.None).ConfigureAwait(false);
     }
 
     [Benchmark]
     public void Execute_ResilienceContextAndState()
     {
         var context = ResilienceContextPool.Shared.Get();
-        ResiliencePipeline.Null.Execute((_, _) => "dummy", context, "state");
+        ResiliencePipeline.Empty.Execute((_, _) => "dummy", context, "state");
         ResilienceContextPool.Shared.Return(context);
     }
 
     [Benchmark]
     public void Execute_CancellationToken()
     {
-        ResiliencePipeline.Null.Execute(_ => "dummy", CancellationToken.None);
+        ResiliencePipeline.Empty.Execute(_ => "dummy", CancellationToken.None);
     }
 
     [Benchmark]
     public void Execute_GenericStrategy_CancellationToken()
     {
-        ResiliencePipeline<string>.Null.Execute(_ => "dummy", CancellationToken.None);
+        ResiliencePipeline<string>.Empty.Execute(_ => "dummy", CancellationToken.None);
     }
 
     public class NonGenericStrategy

src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs

@@ -207,14 +207,6 @@ private void UpdateOriginalContext()
         if (Tasks.FirstOrDefault(static t => t.IsAccepted) is TaskExecution<T> acceptedExecution)
         {
             originalContext.Properties.Replace(acceptedExecution.Properties);
-
-            if (acceptedExecution.Type == HedgedTaskType.Secondary)
-            {
-                foreach (var @event in acceptedExecution.Context.ResilienceEvents)
-                {
-                    originalContext.AddResilienceEvent(@event);
-                }
-            }
         }
     }
 

src/Polly.Core/Outcome.TResult.cs

@@ -44,12 +44,12 @@ private Outcome(ExceptionDispatchInfo exceptionDispatchInfo)
     /// <remarks>
     /// Returns <see langword="true"/> even if the result is void. Use <see cref="IsVoidResult"/> to check for void results.
     /// </remarks>
-    public bool HasResult => ExceptionDispatchInfo is null;
+    internal bool HasResult => ExceptionDispatchInfo is null;
 
     /// <summary>
     /// Gets a value indicating whether the operation produced a void result.
     /// </summary>
-    public bool IsVoidResult => Result is VoidResult;
+    internal bool IsVoidResult => Result is VoidResult;
 
     /// <summary>
     /// Throws an exception if the operation produced an exception.
@@ -64,7 +64,7 @@ private Outcome(ExceptionDispatchInfo exceptionDispatchInfo)
     /// </summary>
     /// <param name="result">Output parameter for the result.</param>
     /// <returns><see langword="true"/> if the result is available; <see langword="false"/> otherwise.</returns>
-    public bool TryGetResult(out TResult? result)
+    internal bool TryGetResult(out TResult? result)
     {
         if (HasResult && !IsVoidResult)
         {

src/Polly.Core/PublicAPI.Unshipped.txt

@@ -134,11 +134,8 @@ Polly.Outcome
 Polly.Outcome<TResult>
 Polly.Outcome<TResult>.EnsureSuccess() -> void
 Polly.Outcome<TResult>.Exception.get -> System.Exception?
-Polly.Outcome<TResult>.HasResult.get -> bool
-Polly.Outcome<TResult>.IsVoidResult.get -> bool
 Polly.Outcome<TResult>.Outcome() -> void
 Polly.Outcome<TResult>.Result.get -> TResult?
-Polly.Outcome<TResult>.TryGetResult(out TResult? result) -> bool
 Polly.OutcomeArguments<TResult, TArgs>
 Polly.OutcomeArguments<TResult, TArgs>.Arguments.get -> TArgs
 Polly.OutcomeArguments<TResult, TArgs>.Context.get -> Polly.ResilienceContext!
@@ -193,7 +190,6 @@ Polly.ResilienceContext.CancellationToken.get -> System.Threading.CancellationTo
 Polly.ResilienceContext.ContinueOnCapturedContext.get -> bool
 Polly.ResilienceContext.OperationKey.get -> string?
 Polly.ResilienceContext.Properties.get -> Polly.ResilienceProperties!
-Polly.ResilienceContext.ResilienceEvents.get -> System.Collections.Generic.IReadOnlyList<Polly.Telemetry.ResilienceEvent>!
 Polly.ResilienceContextCreationArguments
 Polly.ResilienceContextCreationArguments.CancellationToken.get -> System.Threading.CancellationToken
 Polly.ResilienceContextCreationArguments.ContinueOnCapturedContext.get -> bool?
@@ -406,7 +402,7 @@ static Polly.RetryResiliencePipelineBuilderExtensions.AddRetry(this Polly.Resili
 static Polly.RetryResiliencePipelineBuilderExtensions.AddRetry<TResult>(this Polly.ResiliencePipelineBuilder<TResult>! builder, Polly.Retry.RetryStrategyOptions<TResult>! options) -> Polly.ResiliencePipelineBuilder<TResult>!
 static Polly.TimeoutResiliencePipelineBuilderExtensions.AddTimeout<TBuilder>(this TBuilder! builder, Polly.Timeout.TimeoutStrategyOptions! options) -> TBuilder!
 static Polly.TimeoutResiliencePipelineBuilderExtensions.AddTimeout<TBuilder>(this TBuilder! builder, System.TimeSpan timeout) -> TBuilder!
-static readonly Polly.ResiliencePipeline.Null -> Polly.ResiliencePipeline!
-static readonly Polly.ResiliencePipeline<T>.Null -> Polly.ResiliencePipeline<T>!
+static readonly Polly.ResiliencePipeline.Empty -> Polly.ResiliencePipeline!
+static readonly Polly.ResiliencePipeline<T>.Empty -> Polly.ResiliencePipeline<T>!
 virtual Polly.Registry.ResiliencePipelineProvider<TKey>.GetPipeline(TKey key) -> Polly.ResiliencePipeline!
 virtual Polly.Registry.ResiliencePipelineProvider<TKey>.GetPipeline<TResult>(TKey key) -> Polly.ResiliencePipeline<TResult>!

src/Polly.Core/ResilienceContext.cs

@@ -1,5 +1,4 @@
 using System.Diagnostics.CodeAnalysis;
-using Polly.Telemetry;
 
 namespace Polly;
 
@@ -14,8 +13,6 @@ namespace Polly;
 /// </remarks>
 public sealed class ResilienceContext
 {
-    private readonly List<ResilienceEvent> _resilienceEvents = new();
-
     internal ResilienceContext()
     {
     }
@@ -66,24 +63,13 @@ internal ResilienceContext()
     /// </summary>
     public ResilienceProperties Properties { get; internal set; } = new();
 
-    /// <summary>
-    /// Gets the collection of resilience events that occurred while executing the resilience strategy.
-    /// </summary>
-    /// <remarks>
-    /// If the number of resilience events with severity greater than <see cref="ResilienceEventSeverity.Information"/> is greater than zero it's an indication that the execution was unhealthy.
-    /// Note that the number of reported events depends on whether telemetry is enabled for the pipeline or not.
-    /// </remarks>
-    public IReadOnlyList<ResilienceEvent> ResilienceEvents => _resilienceEvents;
-
     internal void InitializeFrom(ResilienceContext context)
     {
         OperationKey = context.OperationKey;
         ResultType = context.ResultType;
         IsSynchronous = context.IsSynchronous;
         CancellationToken = context.CancellationToken;
         ContinueOnCapturedContext = context.ContinueOnCapturedContext;
-        _resilienceEvents.Clear();
-        _resilienceEvents.AddRange(context.ResilienceEvents);
     }
 
     [ExcludeFromCodeCoverage]
@@ -101,11 +87,6 @@ internal ResilienceContext Initialize<TResult>(bool isSynchronous)
         return this;
     }
 
-    internal void AddResilienceEvent(ResilienceEvent @event)
-    {
-        _resilienceEvents.Add(@event);
-    }
-
     internal bool Reset()
     {
         OperationKey = null;
@@ -114,7 +95,6 @@ internal bool Reset()
         ContinueOnCapturedContext = false;
         CancellationToken = default;
         Properties.Options.Clear();
-        _resilienceEvents.Clear();
         return true;
     }
 

src/Polly.Core/ResiliencePipeline.cs

@@ -14,7 +14,7 @@ public sealed partial class ResiliencePipeline
     /// <summary>
     /// Resilience pipeline that executes the user-provided callback without any additional logic.
     /// </summary>
-    public static readonly ResiliencePipeline Null = new(PipelineComponent.Null, DisposeBehavior.Ignore);
+    public static readonly ResiliencePipeline Empty = new(PipelineComponent.Empty, DisposeBehavior.Ignore);
 
     internal ResiliencePipeline(PipelineComponent component, DisposeBehavior disposeBehavior)
     {

src/Polly.Core/ResiliencePipelineBuilderBase.cs

@@ -114,7 +114,7 @@ internal PipelineComponent BuildPipelineComponent()
 
         if (components.Count == 0)
         {
-            return PipelineComponent.Null;
+            return PipelineComponent.Empty;
         }
 
         var source = new ResilienceTelemetrySource(Name, InstanceName, null);

src/Polly.Core/ResiliencePipelineT.cs

@@ -15,7 +15,7 @@ public sealed partial class ResiliencePipeline<T>
     /// <summary>
     /// Resilience pipeline that executes the user-provided callback without any additional logic.
     /// </summary>
-    public static readonly ResiliencePipeline<T> Null = new(PipelineComponent.Null, DisposeBehavior.Ignore);
+    public static readonly ResiliencePipeline<T> Empty = new(PipelineComponent.Empty, DisposeBehavior.Ignore);
 
     internal ResiliencePipeline(PipelineComponent component, DisposeBehavior disposeBehavior)
     {

src/Polly.Core/Telemetry/ResilienceStrategyTelemetry.cs

@@ -33,8 +33,6 @@ public void Report<TArgs>(ResilienceEvent resilienceEvent, ResilienceContext con
     {
         Guard.NotNull(context);
 
-        context.AddResilienceEvent(resilienceEvent);
-
         if (Listener is null || resilienceEvent.Severity == ResilienceEventSeverity.None)
         {
             return;
@@ -52,8 +50,6 @@ public void Report<TArgs>(ResilienceEvent resilienceEvent, ResilienceContext con
     /// <param name="args">The event arguments.</param>
     public void Report<TArgs, TResult>(ResilienceEvent resilienceEvent, OutcomeArguments<TResult, TArgs> args)
     {
-        args.Context.AddResilienceEvent(resilienceEvent);
-
         if (Listener is null || resilienceEvent.Severity == ResilienceEventSeverity.None)
         {
             return;

src/Polly.Core/Utils/Pipeline/PipelineComponent.cs

@@ -8,7 +8,7 @@
 /// </remarks>
 internal abstract class PipelineComponent : IDisposable, IAsyncDisposable
 {
-    public static PipelineComponent Null { get; } = new NullComponent();
+    public static PipelineComponent Empty { get; } = new NullComponent();
 
     internal ResilienceStrategyOptions? Options { get; set; }
 

src/Polly.Extensions/README.md

@@ -130,7 +130,6 @@ Dimensions:
 |`pipeline-instance`| The instance name of the pipeline corresponding to the resilience pipeline.|
 |`operation-key`| The operation key associated with the call site. |
 |`exception-name`| The full name of the exception assigned to the execution result (`System.InvalidOperationException`). |
-|`execution-health`| Indicates whether the execution was healthy or not (`Healthy`, `Unhealthy`). |
 
 ### Logs
 

src/Polly.Extensions/Telemetry/Log.cs

@@ -46,7 +46,6 @@ public static partial void PipelineExecuting(
             "Source: '{PipelineName}/{PipelineInstance}', " +
             "Operation Key: '{OperationKey}', " +
             "Result: '{Result}', " +
-            "Execution Health: '{ExecutionHealth}', " +
             "Execution Time: {ExecutionTime}ms",
         EventName = "StrategyExecuted")]
     public static partial void PipelineExecuted(
@@ -56,7 +55,6 @@ public static partial void PipelineExecuted(
         string pipelineInstance,
         string? operationKey,
         object? result,
-        string executionHealth,
         double executionTime,
         Exception? exception);
 

src/Polly.Extensions/Telemetry/ResilienceTelemetryTags.cs

@@ -16,8 +16,6 @@ internal class ResilienceTelemetryTags
 
     public const string ExceptionName = "exception-name";
 
-    public const string ExecutionHealth = "execution-health";
-
     public const string AttemptNumber = "attempt-number";
 
     public const string AttemptHandled = "attempt-handled";

src/Polly.Extensions/Telemetry/TelemetryListenerImpl.cs

@@ -109,7 +109,6 @@ private void MeterEvent<TResult, TArgs>(in TelemetryEventArguments<TResult, TArg
             var tags = TagsList.Get();
             var context = new EnrichmentContext<TResult, TArgs>(in args, tags.Tags);
             UpdateEnrichmentContext(in context);
-            tags.Tags.Add(new(ResilienceTelemetryTags.ExecutionHealth, args.Context.GetExecutionHealth()));
             ExecutionDuration.Record(executionFinished.Duration.TotalMilliseconds, tags.TagsSpan);
             TagsList.Return(tags);
         }
@@ -165,15 +164,12 @@ private void LogEvent<TResult, TArgs>(in TelemetryEventArguments<TResult, TArgs>
         }
         else if (GetArgs<TArgs, PipelineExecutedArguments>(args.Arguments, out var pipelineExecuted))
         {
-            var logLevel = args.Context.IsExecutionHealthy() ? LogLevel.Debug : LogLevel.Warning;
-
             _logger.PipelineExecuted(
-                logLevel,
+                LogLevel.Debug,
                 args.Source.PipelineName.GetValueOrPlaceholder(),
                 args.Source.PipelineInstanceName.GetValueOrPlaceholder(),
                 args.Context.OperationKey,
                 GetResult(args.Context, args.Outcome),
-                args.Context.GetExecutionHealth(),
                 pipelineExecuted.Duration.TotalMilliseconds,
                 args.Outcome?.Exception);
         }

src/Polly.RateLimiting/OnRateLimiterRejectedArguments.cs

@@ -17,12 +17,10 @@ public readonly struct OnRateLimiterRejectedArguments
     /// </summary>
     /// <param name="context">The context associated with the execution of a user-provided callback.</param>
     /// <param name="lease">The lease that has no permits and was rejected by the rate limiter.</param>
-    /// <param name="retryAfter">The amount of time to wait before retrying again. </param>
-    public OnRateLimiterRejectedArguments(ResilienceContext context, RateLimitLease lease, TimeSpan? retryAfter)
+    public OnRateLimiterRejectedArguments(ResilienceContext context, RateLimitLease lease)
     {
         Context = context;
         Lease = lease;
-        RetryAfter = retryAfter;
     }
 
     /// <summary>
@@ -34,12 +32,4 @@ public OnRateLimiterRejectedArguments(ResilienceContext context, RateLimitLease
     /// Gets the lease that has no permits and was rejected by the rate limiter.
     /// </summary>
     public RateLimitLease Lease { get; }
-
-    /// <summary>
-    /// Gets the amount of time to wait before retrying again.
-    /// </summary>
-    /// <remarks>
-    /// This value is retrieved from the <see cref="Lease"/> by reading the <see cref="MetadataName.RetryAfter"/>.
-    /// </remarks>
-    public TimeSpan? RetryAfter { get; }
 }

src/Polly.RateLimiting/PublicAPI.Unshipped.txt

@@ -4,8 +4,7 @@ Polly.RateLimiting.OnRateLimiterRejectedArguments
 Polly.RateLimiting.OnRateLimiterRejectedArguments.Context.get -> Polly.ResilienceContext!
 Polly.RateLimiting.OnRateLimiterRejectedArguments.Lease.get -> System.Threading.RateLimiting.RateLimitLease!
 Polly.RateLimiting.OnRateLimiterRejectedArguments.OnRateLimiterRejectedArguments() -> void
-Polly.RateLimiting.OnRateLimiterRejectedArguments.OnRateLimiterRejectedArguments(Polly.ResilienceContext! context, System.Threading.RateLimiting.RateLimitLease! lease, System.TimeSpan? retryAfter) -> void
-Polly.RateLimiting.OnRateLimiterRejectedArguments.RetryAfter.get -> System.TimeSpan?
+Polly.RateLimiting.OnRateLimiterRejectedArguments.OnRateLimiterRejectedArguments(Polly.ResilienceContext! context, System.Threading.RateLimiting.RateLimitLease! lease) -> void
 Polly.RateLimiting.RateLimiterRejectedException
 Polly.RateLimiting.RateLimiterRejectedException.RateLimiterRejectedException() -> void
 Polly.RateLimiting.RateLimiterRejectedException.RateLimiterRejectedException(string! message) -> void

src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs

@@ -45,12 +45,12 @@ protected override async ValueTask<Outcome<TResult>> ExecuteCore<TResult, TState
             retryAfter = retryAfterValue;
         }
 
-        var args = new OnRateLimiterRejectedArguments(context, lease, retryAfter);
+        var args = new OnRateLimiterRejectedArguments(context, lease);
         _telemetry.Report(new(ResilienceEventSeverity.Error, RateLimiterConstants.OnRateLimiterRejectedEvent), context, args);
 
         if (OnLeaseRejected != null)
         {
-            await OnLeaseRejected(new OnRateLimiterRejectedArguments(context, lease, retryAfter)).ConfigureAwait(context.ContinueOnCapturedContext);
+            await OnLeaseRejected(new OnRateLimiterRejectedArguments(context, lease)).ConfigureAwait(context.ContinueOnCapturedContext);
         }
 
         var exception = retryAfter.HasValue ? new RateLimiterRejectedException(retryAfter.Value) : new RateLimiterRejectedException();