[Question]: How to log a retry when using dependency injection
Closed this issue · 7 comments
What are you wanting to achieve?
I am trying to use the outlined samples of Polly with dependency injection hence we register a pipeline via AddResiliencePipeline, later using it resiliencePipeline and log incidents where the retry actually occured.
What code or approach do you have so far?
public static IServiceCollection UseSomething (this IServiceCollection services, Action<ResiliencePipelineBuilder>? options = null)
{
services.AddResiliencePipeline(Worker.ResiliencePipelineName, builder =>
{
builder
.AddRetry(new RetryStrategyOptions()
{
... // This has OnRetry, but we don't have a logger or logger factory at this point in time..
})
.AddTimeout(TimeSpan.FromSeconds(300));
}
});
} Then in our actual service:
public Worker(ResiliencePipelineProvider<string> pipelineProvider, ILogger<Worker> logger)
{
this.logger = logger;
this.resiliencePipeline = pipelineProvider.GetPipeline(ResiliencePipelineName);
}
private async Task DoWorkAsync(CancellationToken cancellationToken)
{
...
// We have the 'right' logger here, but what is the way to know when Retry has happened?
var context = ResilienceContextPool.Shared.Get(cancellationToken);
await resiliencePipeline.ExecuteAsync(async token => await notification.SendAsync(), context);
..
}Additional context
Can someone show me what I fail to see ?
Thanks :)
OnRetry is the correct callback to use for the retry event (docs).
Using the code you've provided in the issue, one possible way to achieve what you want to do is to add the logger to the resilience context, and then extract it in the callback to use it.
The AddResiliencePipeline method has several overloads. They are defined inside the PollyServiceCollectionExtensions class. Some allows you to access only the ResiliencePipelineBuilder while others let you use the AddResiliencePipelineContext.
The AddResiliencePipelineContext exposes the ServiceProvider so, you can fetch for example an ILoggerFactory:
services.AddResiliencePipeline("my-key", (builder, context) =>
{
var loggerFactory = context.ServiceProvider.GetRequiredService<ILoggerFactory>();
var logger = ...
builder
.AddRetry(new RetryStrategyOptions()
{
OnRetry = args =>
{
logger.LogError(...);
return default;
},
...
}
});Here you can read more about this.
Thanks Peter! On that link, the thing I was looking for is described as 👍
services.AddResiliencePipeline("myFavoriteStrategy", static (builder, context) =>
{
builder.AddRetry(new()
{
OnRetry = args =>
{
var logger = context.ServiceProvider.GetService<ILogger>();
// ...
return default;
}
});
});
It is different in the way that the logger if fabricated in the event itself -which means that it will be in the scope of other log entries (i.e. compliant with scoped loggers).
@Manthazar If the suggested solution solved your problem then can we close this issue?
If not then please let us know how can we further assist.
AddResiliencePipeline() includes default logging of all "resilience events":
Resilience event occurred. EventName: '"OnRetry"', Source: '"MyPipeline"/"(null)"/"MyRetry"', Operation Key: 'null', Result: '"Some exception message."'
Given that retries are the number-one use case for this library, it feels like an oversight to me that the "OnRetry" attempt number and delay time are not included in the default logging when other events do have explicit handling.
AddResiliencePipeline() includes default logging of all "resilience events":
Resilience event occurred. EventName: '"OnRetry"', Source: '"MyPipeline"/"(null)"/"MyRetry"', Operation Key: 'null', Result: '"Some exception message."'
Given that retries are the number-one use case for this library, it feels like an oversight to me that the "OnRetry" attempt number and delay time are not included in the default logging when other events do have explicit handling.
The Execution Attempt telemetry event does include the attempt number and the execution time. If you would like to see the attempt number and/or the delay time as part of the OnRetry telemetry event then please file a separate issue with this feature request.
From my point of view. All is well answered Peter.