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

Add TelemetrySource to RateLimiterRejectedException #2346

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 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
1 change: 1 addition & 0 deletions src/Polly.Core/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ Polly.CircuitBreaker.BrokenCircuitException.BrokenCircuitException(string! messa
Polly.CircuitBreaker.BrokenCircuitException.BrokenCircuitException(string! message, System.TimeSpan retryAfter, System.Exception! inner) -> void
Polly.CircuitBreaker.BrokenCircuitException.BrokenCircuitException(System.TimeSpan retryAfter) -> void
Polly.CircuitBreaker.BrokenCircuitException.RetryAfter.get -> System.TimeSpan?
Polly.Telemetry.ResilienceStrategyTelemetry.AsTelemetrySourceString() -> string!
12 changes: 12 additions & 0 deletions src/Polly.Core/Telemetry/ResilienceStrategyTelemetry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ internal ResilienceStrategyTelemetry(ResilienceTelemetrySource source, Telemetry

internal ResilienceTelemetrySource TelemetrySource { get; }

/// <summary>
/// Returns a string representation of the source of the telemetry.
/// </summary>
/// <returns>The string representation of the source of the telemetry.</returns>
public string AsTelemetrySourceString()
{
var pipelineName = TelemetrySource?.PipelineName ?? "(null)";
var pipelineInstanceName = TelemetrySource?.PipelineInstanceName ?? "(null)";
var strategyName = TelemetrySource?.StrategyName ?? "(null)";
return $"{pipelineName}/{pipelineInstanceName}/{strategyName}";
}

/// <summary>
/// Reports an event that occurred in a resilience strategy.
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions src/Polly.RateLimiting/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
#nullable enable
Polly.RateLimiting.RateLimiterRejectedException.RateLimiterRejectedException(string! message, string! telemetrySource) -> void
Polly.RateLimiting.RateLimiterRejectedException.RateLimiterRejectedException(string! message, string! telemetrySource, System.Exception! inner) -> void
Polly.RateLimiting.RateLimiterRejectedException.RateLimiterRejectedException(string! message, string! telemetrySource, System.TimeSpan retryAfter) -> void
Polly.RateLimiting.RateLimiterRejectedException.RateLimiterRejectedException(string! message, string! telemetrySource, System.TimeSpan retryAfter, System.Exception! inner) -> void
Polly.RateLimiting.RateLimiterRejectedException.TelemetrySource.get -> string?
73 changes: 60 additions & 13 deletions src/Polly.RateLimiting/RateLimiterRejectedException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,33 @@ public RateLimiterRejectedException(string message)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="RateLimiterRejectedException"/> class.
/// </summary>
/// <param name="message">The message that describes the error.</param>
/// <param name="telemetrySource">The source pipeline and strategy names.</param>
peter-csala marked this conversation as resolved.
Show resolved Hide resolved
public RateLimiterRejectedException(string message, string telemetrySource)
: base(message)
=> TelemetrySource = telemetrySource;

/// <summary>
/// Initializes a new instance of the <see cref="RateLimiterRejectedException"/> class.
/// </summary>
/// <param name="message">The message that describes the error.</param>
/// <param name="retryAfter">The retry after value.</param>
public RateLimiterRejectedException(string message, TimeSpan retryAfter)
: base(message) => RetryAfter = retryAfter;
: base(message)
=> RetryAfter = retryAfter;

/// <summary>
/// Initializes a new instance of the <see cref="RateLimiterRejectedException"/> class.
/// </summary>
/// <param name="message">The message that describes the error.</param>
/// <param name="telemetrySource">The source pipeline and strategy names.</param>
/// <param name="retryAfter">The retry after value.</param>
public RateLimiterRejectedException(string message, string telemetrySource, TimeSpan retryAfter)
: base(message)
=> (TelemetrySource, RetryAfter) = (telemetrySource, retryAfter);

/// <summary>
/// Initializes a new instance of the <see cref="RateLimiterRejectedException"/> class.
Expand All @@ -56,14 +76,36 @@ public RateLimiterRejectedException(string message, Exception inner)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="RateLimiterRejectedException"/> class.
/// </summary>
/// <param name="message">The message that describes the error.</param>
/// <param name="telemetrySource">The source pipeline and strategy names.</param>
/// <param name="inner">The inner exception.</param>
public RateLimiterRejectedException(string message, string telemetrySource, Exception inner)
: base(message, inner)
=> TelemetrySource = telemetrySource;

/// <summary>
/// Initializes a new instance of the <see cref="RateLimiterRejectedException"/> class.
/// </summary>
/// <param name="message">The message that describes the error.</param>
/// <param name="retryAfter">The retry after value.</param>
/// <param name="inner">The inner exception.</param>
public RateLimiterRejectedException(string message, TimeSpan retryAfter, Exception inner)
: base(message, inner) => RetryAfter = retryAfter;
: base(message, inner)
=> RetryAfter = retryAfter;

/// <summary>
/// Initializes a new instance of the <see cref="RateLimiterRejectedException"/> class.
/// </summary>
/// <param name="message">The message that describes the error.</param>
/// <param name="telemetrySource">The source pipeline and strategy names.</param>
/// <param name="retryAfter">The retry after value.</param>
/// <param name="inner">The inner exception.</param>
public RateLimiterRejectedException(string message, string telemetrySource, TimeSpan retryAfter, Exception inner)
: base(message, inner)
=> (TelemetrySource, RetryAfter) = (telemetrySource, retryAfter);

/// <summary>
/// Gets the amount of time to wait before retrying again.
Expand All @@ -74,6 +116,11 @@ public RateLimiterRejectedException(string message, TimeSpan retryAfter, Excepti
/// </remarks>
public TimeSpan? RetryAfter { get; }

/// <summary>
/// Gets the name of the strategy which has thrown the exception.
peter-csala marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public string? TelemetrySource { get; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use telemetry source here? The strategy name is not enough to uniquely identify the strategy.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, added a comment :)


#pragma warning disable RS0016 // Add public types and members to the declared API
#if !NETCOREAPP
/// <summary>
Expand All @@ -84,10 +131,16 @@ public RateLimiterRejectedException(string message, TimeSpan retryAfter, Excepti
private RateLimiterRejectedException(SerializationInfo info, StreamingContext context)
: base(info, context)
{
var value = info.GetDouble("RetryAfter");
if (value >= 0.0)
var retryAfter = info.GetDouble(nameof(RetryAfter));
if (retryAfter >= 0.0)
martincostello marked this conversation as resolved.
Show resolved Hide resolved
{
RetryAfter = TimeSpan.FromSeconds(retryAfter);
}

var telemetrySource = info.GetString(nameof(TelemetrySource));
if (telemetrySource is not null)
{
RetryAfter = TimeSpan.FromSeconds(value);
TelemetrySource = telemetrySource;
}
}

Expand All @@ -96,14 +149,8 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont
{
Guard.NotNull(info);

if (RetryAfter.HasValue)
{
info.AddValue("RetryAfter", RetryAfter.Value.TotalSeconds);
}
else
{
info.AddValue("RetryAfter", -1.0);
}
info.AddValue(nameof(TelemetrySource), TelemetrySource ?? "(null)/(null)/(null)");
info.AddValue(nameof(RetryAfter), RetryAfter.HasValue ? RetryAfter.Value.TotalSeconds : -1.0);
martincostello marked this conversation as resolved.
Show resolved Hide resolved

base.GetObjectData(info, context);
}
Expand Down
7 changes: 6 additions & 1 deletion src/Polly.RateLimiting/RateLimiterResilienceStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ namespace Polly.RateLimiting;

internal sealed class RateLimiterResilienceStrategy : ResilienceStrategy, IDisposable, IAsyncDisposable
{
private const string Message = "The operation could not be executed because it was rejected by the rate limiter.";
peter-csala marked this conversation as resolved.
Show resolved Hide resolved
private readonly ResilienceStrategyTelemetry _telemetry;

public RateLimiterResilienceStrategy(
Expand Down Expand Up @@ -65,7 +66,11 @@ protected override async ValueTask<Outcome<TResult>> ExecuteCore<TResult, TState
await OnLeaseRejected(new OnRateLimiterRejectedArguments(context, lease)).ConfigureAwait(context.ContinueOnCapturedContext);
}

var exception = retryAfter.HasValue ? new RateLimiterRejectedException(retryAfter.Value) : new RateLimiterRejectedException();
var source = _telemetry.AsTelemetrySourceString();

var exception = retryAfter.HasValue
? new RateLimiterRejectedException($"{Message} It can be retried after '{retryAfter.Value}'.", source, retryAfter.Value)
: new RateLimiterRejectedException(Message, source);

return Outcome.FromException<TResult>(exception.TrySetStackTrace());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,27 @@ public void Enabled_Ok()
new ResilienceStrategyTelemetry(_source, null).Enabled.Should().BeFalse();
}

[Fact]
public void AsTelemetrySourceString_Ok()
{
var source = new ResilienceTelemetrySource("builder", "instance", "strategy_name");
new ResilienceStrategyTelemetry(source, null).AsTelemetrySourceString().Should().Be("builder/instance/strategy_name");
}

[Fact]
public void AsTelemetrySourceString_Null_Ok()
{
ResilienceTelemetrySource? source = null;
new ResilienceStrategyTelemetry(source!, null).AsTelemetrySourceString().Should().Be("(null)/(null)/(null)");
}

[Fact]
public void AsTelemetrySourceString_Nulls_Ok()
{
var source = new ResilienceTelemetrySource(null, null, null);
new ResilienceStrategyTelemetry(source, null).AsTelemetrySourceString().Should().Be("(null)/(null)/(null)");
}

[Fact]
public void Report_NoOutcome_OK()
{
Expand Down
110 changes: 101 additions & 9 deletions test/Polly.RateLimiting.Tests/RateLimiterRejectedExceptionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,108 @@ namespace Polly.Core.Tests.Timeout;

public class RateLimiterRejectedExceptionTests
{
private readonly string _telemetrySource = "MyPipeline/MyPipelineInstance/MyRateLimiterStrategy";
private readonly string _message = "dummy";
private readonly TimeSpan _retryAfter = TimeSpan.FromSeconds(4);

[Fact]
public void Ctor_Ok()
{
var retryAfter = TimeSpan.FromSeconds(4);
var exception = new RateLimiterRejectedException();
exception.InnerException.Should().BeNull();
exception.Message.Should().Be("The operation could not be executed because it was rejected by the rate limiter.");
exception.RetryAfter.Should().BeNull();
exception.TelemetrySource.Should().BeNull();
}

[Fact]
public void Ctor_RetryAfter_Ok()
{
var exception = new RateLimiterRejectedException(_retryAfter);
exception.InnerException.Should().BeNull();
exception.Message.Should().Be($"The operation could not be executed because it was rejected by the rate limiter. It can be retried after '00:00:04'.");
exception.RetryAfter.Should().Be(_retryAfter);
exception.TelemetrySource.Should().BeNull();
}

new RateLimiterRejectedException().Message.Should().Be("The operation could not be executed because it was rejected by the rate limiter.");
new RateLimiterRejectedException().RetryAfter.Should().BeNull();
new RateLimiterRejectedException("dummy").Message.Should().Be("dummy");
new RateLimiterRejectedException("dummy", new InvalidOperationException()).Message.Should().Be("dummy");
new RateLimiterRejectedException(retryAfter).RetryAfter.Should().Be(retryAfter);
new RateLimiterRejectedException(retryAfter).Message.Should().Be($"The operation could not be executed because it was rejected by the rate limiter. It can be retried after '{retryAfter}'.");
new RateLimiterRejectedException("dummy", retryAfter).RetryAfter.Should().Be(retryAfter);
new RateLimiterRejectedException("dummy", retryAfter, new InvalidOperationException()).RetryAfter.Should().Be(retryAfter);
[Fact]
public void Ctor_Message_Ok()
{
var exception = new RateLimiterRejectedException(_message);
exception.InnerException.Should().BeNull();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().BeNull();
exception.TelemetrySource.Should().BeNull();
}

[Fact]
public void Ctor_Message_TelemetrySource_Ok()
{
var exception = new RateLimiterRejectedException(_message, _telemetrySource);
exception.InnerException.Should().BeNull();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().BeNull();
exception.TelemetrySource.Should().Be(_telemetrySource);
}

[Fact]
public void Ctor_Message_RetryAfter_Ok()
{
var exception = new RateLimiterRejectedException(_message, _retryAfter);
exception.InnerException.Should().BeNull();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().Be(_retryAfter);
exception.TelemetrySource.Should().BeNull();
}

[Fact]
public void Ctor_Message_TelemetrySource_RetryAfter_Ok()
{
var exception = new RateLimiterRejectedException(_message, _telemetrySource, _retryAfter);
exception.InnerException.Should().BeNull();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().Be(_retryAfter);
exception.TelemetrySource.Should().Be(_telemetrySource);
}

[Fact]
public void Ctor_Message_InnerException_Ok()
{
var exception = new RateLimiterRejectedException(_message, new InvalidOperationException());
exception.InnerException.Should().BeOfType<InvalidOperationException>();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().BeNull();
exception.TelemetrySource.Should().BeNull();
}

[Fact]
public void Ctor_Message_TelemetrySource_InnerException_Ok()
{
var exception = new RateLimiterRejectedException(_message, _telemetrySource, new InvalidOperationException());
exception.InnerException.Should().BeOfType<InvalidOperationException>();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().BeNull();
exception.TelemetrySource.Should().Be(_telemetrySource);
}

[Fact]
public void Ctor_Message_RetryAfter_InnerException_Ok()
{
var exception = new RateLimiterRejectedException(_message, _retryAfter, new InvalidOperationException());
exception.InnerException.Should().BeOfType<InvalidOperationException>();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().Be(_retryAfter);
exception.TelemetrySource.Should().BeNull();
}

[Fact]
public void Ctor_Message_TelemetrySource_RetryAfter_InnerException_Ok()
{
var exception = new RateLimiterRejectedException(_message, _telemetrySource, _retryAfter, new InvalidOperationException());
exception.InnerException.Should().BeOfType<InvalidOperationException>();
exception.Message.Should().Be(_message);
exception.RetryAfter.Should().Be(_retryAfter);
exception.TelemetrySource.Should().Be(_telemetrySource);
}

#if !NETCOREAPP
Expand All @@ -29,6 +118,9 @@ public void BinaryDeserialization_Ok()

result = SerializeAndDeserializeException(new RateLimiterRejectedException());
result.RetryAfter.Should().BeNull();

result = SerializeAndDeserializeException(new RateLimiterRejectedException("message", _telemetrySource));
result.TelemetrySource.Should().Be(_telemetrySource);
}

public static T SerializeAndDeserializeException<T>(T exception)
Expand Down