Autofac icon indicating copy to clipboard operation
Autofac copied to clipboard

Posibility of tracing auto activated instances.

Open nicholastcs opened this issue 4 years ago • 13 comments

builder.RegisterType<AuditService>()
      .AsSelf()
      .SingleInstance()
      .AutoActivate();

May I know if there is any possibilities that to trace auto activated instances like example above. As far I know this tooling only traces .Resolve() operations.

By the way, a big thanks for team developed useful tooling for Autofac. Really appreciate that.

nicholastcs avatar Oct 03 '20 16:10 nicholastcs

Diagnostics are exposed by the core container so I'm going to move this to the main Autofac repo. The DotGraph mechanism is just one way of visualizing the core diagnostics.

Have you actually tried tracing auto-activate components? What worked, what didn't? Auto activate goes through a resolve request just like anything else, IIRC.

tillig avatar Oct 03 '20 19:10 tillig

Thinking about this, diagnostics are attached after the container is built, however AutoActivated components are resolved before the Build method returns. So I can see the problem.

To trace them, you may need to add tracing inside a container build callback? Does that fire before auto-activated components?

alistairjevans avatar Oct 03 '20 20:10 alistairjevans

I've tried using container build callback, but it only trace the graph for scope.Resolve<T>() inside the using statement of container.BeginLifetimeScope() (See snippet below).

Below is what working (workaround) To trace both auto activated components and scope.Resolve<T>() components. I uniquely mark them with Guid.NewGuid() for the .dot files generated inside the tracer.OperationCompleted callback.

builder.RegisterType<AuditService>()
      .AsSelf()
      .SingleInstance();
// removed .AutoActivate()
using (var scope = container.BeginLifeTimeScope())
{
     var paymentClient = scope.Resolve<IPaymentClient>();
     _ = Task.Run(scope.Resolve<AuditService>().Inspect);
}

image

nicholastcs avatar Oct 04 '20 05:10 nicholastcs

Ouch, that workaround seems painful. I've got an idea in my head of a simpler workaround that means you can still use AutoActivate; I'll put it down on paper (so to speak) and come back to you.

alistairjevans avatar Oct 05 '20 08:10 alistairjevans

Ok, so I have a workaround; it's not ideal, but it'll work consistently while keeping the AutoActivate behaviour.

Basically, you need this method somewhere:


private void AddAutoActivatingTracer(ContainerBuilder builder, DiagnosticTracerBase tracer)
{
    builder.Register(ctxt => "trace-attach")
            .SingleInstance()
            .AutoActivate()
            .ConfigurePipeline(cfg =>
            {
                var isAttached = false;
                cfg.Use(PipelinePhase.RegistrationPipelineStart, (ctxt, next) =>
                {
                    next(ctxt);

                    if (!isAttached)
                    {
                        isAttached = true;
                        ctxt.DiagnosticSource.Subscribe(tracer, tracer.IsEnabled);
                    }
                });
            });
}

Then, call this method last thing before you call Build on your container. It should cause any AutoActivate registrations to get traced.

This obviously isn't ideal, and is probably one for us to think about for a future version.

alistairjevans avatar Oct 05 '20 16:10 alistairjevans

I wonder if the build callbacks should run before startables. Like, right now the callbacks are running after...

            var result = new Container(componentRegistry);
            if ((options & ContainerBuildOptions.IgnoreStartableComponents) == ContainerBuildOptions.None)
            {
                StartableManager.StartStartableComponents(Properties, result);
            }

            // Run any build callbacks.
            BuildCallbackManager.RunBuildCallbacks(result);

But, like, if there are build callbacks that need to do something for startables, like this, they'd need to run before that. I also think that the "build" process, which includes those callbacks, should probably logically conclude before anything is resolved/activated from the container.

Admittedly, I don't recall off the top of my head if things were happening in this order intentionally or if it just sort of... got made that way.

tillig avatar Oct 05 '20 16:10 tillig

There's probably an argument for it either way; regardless it would be a breaking change to change the point at which the event fires.

Perhaps a better option is just to add something to the ContainerBuilder that allows tracers to be attached before Build time.

alistairjevans avatar Oct 05 '20 16:10 alistairjevans

Ok, so I have a workaround; it's not ideal, but it'll work consistently while keeping the AutoActivate behaviour.

Basically, you need this method somewhere:

private void AddAutoActivatingTracer(ContainerBuilder builder, DiagnosticTracerBase tracer)
{
    builder.Register(ctxt => "trace-attach")
            .SingleInstance()
            .AutoActivate()
            .ConfigurePipeline(cfg =>
            {
                var isAttached = false;
                cfg.Use(PipelinePhase.RegistrationPipelineStart, (ctxt, next) =>
                {
                    next(ctxt);

                    if (!isAttached)
                    {
                        isAttached = true;
                        ctxt.DiagnosticSource.Subscribe(tracer, tracer.IsEnabled);
                    }
                });
            });
}

Then, call this method last thing before you call Build on your container. It should cause any AutoActivate registrations to get traced.

This obviously isn't ideal, and is probably one for us to think about for a future version.

Just an update, the code snippet works, able to capture all .Resolve<T>(); and .AutoActivate() instances.

Once again, thanks @tillig and @alistairjevans for the help. I'll close the issue for now.

nicholastcs avatar Oct 08 '20 05:10 nicholastcs

I'm going to reopen it so we don't forget to try to add something to ContainerBuilder to allow for a better approach here.

tillig avatar Oct 08 '20 13:10 tillig

Alright, looking forward for the next update.

nicholastcs avatar Oct 10 '20 03:10 nicholastcs

Took a look at possible changes to the ContainerBuilder this morning.

Two possible options as I see them; @tillig, would be good to get your opinion.

  1. Define a list of DiagnosticTracerBase inside ContainerBuilder; add SubscribeToDiagnostics methods to ContainerBuilder. When we create the container, immediately subscribe all tracers in the list to the newly created Container.

  2. Define the DiagnosticListener that we will use for the container we are about to create in the ContainerBuilder class, and then pass it into the Container constructor, instead of creating it inside the root lifetime scope. Add SubscribeToDiagnostics methods to work against ContainerBuilder's DiagnosticSource.

The first option is simpler, but prevents a developer from wiring up a custom IObserver implementation in the ContainerBuilder. I also kind of like the idea of us having a DiagnosticSource available while we're building? Might be useful in future to actually write diagnostic events for the container build process?

alistairjevans avatar Oct 14 '20 07:10 alistairjevans

Yeah, I think option 2 is where my brain was headed while I was noodling on this myself.

I also thought about the overlap of build callbacks, this problem, and the recent issues around wanting to intercept more events during a registration pipeline. There was a very vague, unformed thought bubbling around in my brain about how to tie those things together but when I try to nail it down it comes out really complex and overengineered for the issue at hand.

I'd vote option 2 - add the DiagnosticListener to the builder and allow it to be passed into the container.

tillig avatar Oct 14 '20 14:10 tillig

Took a look at this this morning; all was going well, until Modules came into play. Modules get their own ContainerBuilder built from the IComponentRegistryBuilder. If we want the Module's ContainerBuilders to all point to the eventual Container DiagnosticListener, we'd have to change the signature for Module, which I am absolutely loathe to do.

I'd put the DiagnosticListener in the IComponentRegistryBuilder, but it feels like the wrong place for it. I'll give it some more thought.

alistairjevans avatar Oct 31 '20 09:10 alistairjevans

This issue has been open for quite some time without any traction. Much as we understand the challenges presented here, we also get very few substantial contributions and we don't have time to really dig into non-trivial issues like this. As such, we're closing this issue. If someone feels like they really want this, we would love to see a PR where this gets addressed.

tillig avatar Nov 06 '23 17:11 tillig