Improvements to EventHub/ServiceBus error handling (#1784, #1760) #1788
Conversation
sample/SampleHost/Program.cs
Outdated
| config.Queues.VisibilityTimeout = TimeSpan.FromSeconds(15); | ||
| config.Queues.MaxDequeueCount = 3; | ||
| config.LoggerFactory = new LoggerFactory().AddConsole(); | ||
| while (true) |
There was a problem hiding this comment.
Will undo these changes - was using this create/dispose loop to verify my changes. Before the changes, each restart resulted in ServiceBus error logs being written, now the host restarts are all clean.
| { | ||
| internal static class LogLevelExtensions | ||
| { | ||
| internal static TraceLevel ToTraceLevel(this LogLevel logLevel) |
There was a problem hiding this comment.
Copied this from the functions runtime codebase
| // get the services we need to construct our binding providers | ||
| INameResolver nameResolver = context.Config.GetService<INameResolver>(); | ||
| IExtensionRegistry extensions = context.Config.GetService<IExtensionRegistry>(); | ||
|
|
||
| // register the background exception handler | ||
| var exceptionHandler = MessagingExceptionHandler.Subscribe(Config.MessageOptions, context.Trace, context.Config.LoggerFactory); |
There was a problem hiding this comment.
The key for ServiceBus issue fix was to capture the exception handler and flow it down to the listener, so it can be unsubscribed BEFORE we abort the messaging factory.
| @@ -444,6 +444,9 @@ void IExtensionConfigProvider.Initialize(ExtensionConfigContext context) | |||
| .AddConverter<byte[], EventData>(ConvertBytes2EventData) | |||
| .AddConverter<EventData, byte[]>(ConvertEventData2Bytes); | |||
|
|
|||
| // register the background exception handler | |||
| MessagingExceptionHandler.Subscribe(_options, context.Trace, context.Config.LoggerFactory); | |||
There was a problem hiding this comment.
We don't need to unsubscribe for EventHubs - the way our listener unregisters the EPH shuts down cleanly. I had flowed it down anyways, however we don't want to unregister before we close the EPH (since any errors happening during graceful shutdown we want to log).
|
|
||
| protected override LogLevel GetLogLevel(Exception ex) | ||
| { | ||
| if (ex is ReceiverDisconnectedException || |
There was a problem hiding this comment.
This is the key change for the EventHub issue fix - overriding the base level determination to make these Info level rather than errors as they are now (generating CRIs)
|
|
||
| public EventHubExceptionHandler(EventProcessorOptions options, TraceWriter traceWriter, ILoggerFactory loggerFactory = null) : base("EventProcessorHost", traceWriter, loggerFactory) | ||
| { | ||
| _options = options; |
There was a problem hiding this comment.
Null check the options? Otherwise your subscribe/unsubscribe will NRE.
mathewc
force-pushed
the
logging-fix
branch
2 times, most recently
from
5c25c4a to
c731190
Compare
Fixes for the following issues: