autofac / Autofac.ServiceFabric

Autofac integration for Azure Service Fabric. Provides service factory implementations for Actors, Stateful Services and Stateless Services.
MIT License
26 stars 27 forks source link

Allow logging exceptions during service instantiation #33

Closed fcastellscha closed 6 years ago

fcastellscha commented 6 years ago

I don't know if there is already a way of doing this, but I need to log the exceptions during service instantiation. Normally these exceptions are Autofac exceptions that cannot resolve a service dependency, but they could also be exceptions during the constructor execution.

Maybe we could pass an Action to RegisterServiceFabricSupport or to the service registration methods? like

builder.RegisterServiceFabricSupport(e=> logger.Error(e, "Exception instantiating service");

or

builder.RegisterStatelessService<MyServiceType>(myServiceTypeName, e=> logger.Error(e, "Exception instantiating MyServiceType");

johnkattenhorn commented 6 years ago

@alexmg, @tillig - any chance of getting this merged and released ?

alexmg commented 6 years ago

Hi guys. Sorry, I've been away for the past few weeks and just got back today.

I assume this is to log the exception to another logging framework in addition to any existing ETW events that SF writes.

I think I prefer the RegisterServiceFabricSupport option to avoid having repeated boilerplate code on all service registrations.

In either case there would be an assumption that the logger implementation is available at the time of service registration (unless exposed using a static API like Serilog's Log class).

johnkattenhorn commented 6 years ago

Yep, I see your point @alexmg, I'm cool with RegisterServiceFabricSupport, I'll submit another PR today if you like.

alexmg commented 6 years ago

That would be awesome @johnkattenhorn. And thanks @fcastellscha and @EspenG for getting this started.

The new parameter should be optional and have a descriptive name like constructorExceptionCallback to indicate its scope of use is fairly limited.

We could also consider providing an options type to the RegisterServiceFabricSupport method with a default implementation that has an empty callback for the ConstructorExceptionCallback property.

alexmg commented 6 years ago

Thinking about it a little, I don't think there will be enough options to warrant a whole new options type.

alexmg commented 6 years ago

I pulled in the PR from @EspenG as a starting point for this and then refactored so that the action is provided to the RegisterServiceFabricSupport instead of each individual service registration.

I also made sure that the lifetime scope is proactively disposed when a constructor exception is thrown because the interceptor won't get the chance to do it.

alexmg commented 6 years ago

These changes are currently on the MyGet feed in any 2.1.0 build if you would like to test them.

https://www.myget.org/F/autofac/api/v3/index.json

RemcoBlok commented 5 years ago

I need a way to resolve my logger in the callback. The callback is specified when configuring the container builder, but before the container is built. Is it possible to replace the Action<Exception> with Action<ILifetimeScope, Exception>. I can then use the ILifetimeScope to resolve my logger. thanks