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

Allow for calling custom code before and after function execution #1041

Closed
Bio2hazard opened this issue Mar 2, 2017 · 9 comments
Closed
Labels
Milestone

Comments

@Bio2hazard
Copy link

Bio2hazard commented Mar 2, 2017

I have been struggling to come up with a clean way to run code before and after a function runs, and I feel that it would be a worthwhile feature to add as it would promote modular, reusable code.

My specific use-case is to wrap my function invocation in a database transaction. Currently, I have to place a await UnitOfWork.SaveChangesAsync(); at the bottom of each function. I would like to streamline this by simply placing a attribute on my function.

A second use-case is to make it easier to implement benchmarking or telemetry such as Application Insights, as it would be very convenient to use DI to inject a telemetry tool, start a stop watch prior to function invocation, and then send it up after the fact.

Right now I can't find a existing extensibility point for functions. The IJobActivator interface lets me create the class, but has no knowledge of the function being executed.

Custom processors have before / after methods, but again do not have any knowledge of the actual function being executed. Additionally, the instances have already been disposed of at this point.

To solve this issue, I would propose adding a interface that defines before / after functionality. In my proof of concept, I built it out like this:

    /// <summary>
    /// Interface defining functionality for executing code before and after function invocation.
    /// </summary>
    public interface IExtendableFunction
    {
        /// <summary>
        /// Executes before function invocation.
        /// </summary>
        /// <param name="info">The <see cref="MethodInfo"/> for the function invocation.</param>
        /// <param name="arguments">The list of arguments passed to the function.</param>
        /// <returns>A <see cref="Task"/> for the operation.</returns>
        Task BeforeFunctionExecutionAsync(MethodInfo info, object[] arguments);

        /// <summary>
        /// Executes after function invocation.
        /// </summary>
        /// <param name="info">The <see cref="MethodInfo"/> for the function invocation.</param>
        /// <param name="arguments">The list of arguments passed to the function.</param>
        /// <returns>A <see cref="Task"/> for the operation.</returns>
        Task AfterFunctionExecutionAsync(MethodInfo info, object[] arguments);
    }

Then, a simple tweak to FunctionInvoker<TReflected>:

  • Adding a private readonly MethodInfo to the class and instanciating it from the constructor
  • Updating InvokeAsync:
        public async Task InvokeAsync(object[] arguments)
        {
            // Return a task immediately in case the method is not async.
            await Task.Yield();

            TReflected instance = _instanceFactory.Create();

            using (instance as IDisposable)
            {
                if (instance is IExtendableFunction)
                {
                    await((IExtendableFunction)instance).BeforeFunctionExecutionAsync(_methodInfo, arguments);
                }

                await _methodInvoker.InvokeAsync(instance, arguments);

                if (instance is IExtendableFunction)
                {
                    await((IExtendableFunction)instance).AfterFunctionExecutionAsync(_methodInfo, arguments);
                }
            }
        }

If you are willing to add this functionality, I would be more than happy to submit a pull request with my changes.

@abatishchev
Copy link

Also #734 and #980

@Bio2hazard
Copy link
Author

I saw #980 , it's very close to this, but I hope they will consider adding both options, as they work differently.

The difference, and one of the big issues I have when it comes to working with Web Jobs, is scope.

To get dependency injection to work, I explicitly start a isolated scope on my DI container in the IJobActivator implementation. I then pass the scope identifier into my function instance, so that I can dispose of the scope in the dispose method of my function instance.

So far, all extensibility points that Web Jobs offer lie outside of the scope of the function. Sure, the Trace lets me know there's a message to process, but that happens before IJobActivator creates the instance. Trace also lets me know when a message is done processing, but that happens after the instance is already disposed of.

My concern with the proposed solution in #980 is that there is no indication that i'll be able to access my scoped DI container from the filter.

Web Api gets around that by providing a thread-safe HttpContext / OwinContext. That, coupled with being able to start a DI container scope first thing a request comes in and being able to dispose of the scope only after everything related to the request is finished, means I can register my container in the HttpContext / OwinContext and access my scope from the web api filters.

In other words, unless we get a extensibility point to create a DI scope before the Queue/Blob/Whatever Processors run, that lasts until after trace / filters finish, and a thread-safe, static FunctionExecutionContext in which we can register objects to retrieve later in the pipeline, #980 will not allow me to do what I need to do.

That's why I propose this solution - It's simple, effective, non-breaking and completely optional.

We work exclusively with IoC and DI as it makes our code testable, allows us to substitute functionality and all the other benefits of using dependency injection, and yet it's been a constant struggle to get anywhere with DI in Web Jobs.

Let me give a few examples:
At the end of a function execution, I want to check the pending database transaction for changes, so that I can flush stale records out of my redis cache.

So I need to place a dbContext.ProcessPendingChanges at the end of every function.

I want to check if the queue message that triggered my Function has a userId element, and if so I want to prep my Exception reporting tool with that userId, so that any exceptions that happen during the function execution can be traced back to that user.

Well, the trace module is the only place where I can take action on a exception, and it happens long after the function instance was disposed of, and it has no knowledge of what triggered the function. So the only option is to make the first line of every function a exceptionHandler.RegisterUser, and then wrapping the whole function in a try/catch.

I want to implement application insights, and ideally pass in a operationId from a website AI request, so that I can track web job executions as a dependency of the web request. There's even a guide on how to do that with Cloud Services ( https://docs.microsoft.com/en-us/azure/application-insights/app-insights-cloudservices#track-requests-from-worker-roles ).

And there is a couple of more things of similar nature - the point being that right now to get anywhere, I have to wrap my functions in more boilerplate code than actual unique function code, and now that this issue is not getting any traction, I'm honestly at my wit's end.

It seems like such a basic thing that I feel like I am missing something obvious, but I've spent hours crawling through the code and came up blank - anything that could be remotely helpful ( such as overriding or customizing the FunctionInvoker ) is internal.

@christopheranderson christopheranderson added this to the Next milestone Mar 13, 2017
@dfaivre
Copy link

dfaivre commented Mar 21, 2017

This would also make my life easier (though I may just not have a deep enough understanding of the WebJobs SDK yet). I'd like my functions invoked with DI with a scope of each invocation. I'd also like to to be able to add "job context information" to our Serilog infrastructure.

@mathewc
Copy link
Member

mathewc commented Jun 21, 2017

@Bio2hazard We've started early work on the Invoke Filters feature, and as we iterate on that PR we want to make sure that your scenarios are addressed. If we can distill your requirements down to a simple set of regression tests, we'll add those as part of this work :) The PR is early/rough but is here: #1189. I think with the changes we're proposing in that feature, we should be able to address your scenarios as well with some minor tweaks.

Take a look at that PR and the context objects we're going to pass into the executing/executed filters and let us know if they'll work for you. Or, you might simply provide us some simplified sample code based on this filter design capturing your scenario and we'll bake it into our tests.

@Bio2hazard
Copy link
Author

@mathewc Thank you addressing this!

I pulled down the branch and tested it locally, but most of my scope concerns are not currently addressed by it.

To distill my requirements down to a minimum: I need a dependency injection scope that is accessible before, during and after the function execution.

Right now the life cycle of a message queue triggered function is such:

  • Queue Processor BeginProcessingMessageAsync
    • OnExecutingAsync Filter
      • JobActivator.CreateInstance
      • Function Class CTOR
        • Function Method
      • Function Class Dispose
    • OnExecutedAsync Filter
  • Queue Processor CompleteProcessingMessageAsync

In order to use constructor based dependency injection, the dependency injection container needs to instantiate the Function Class. The only extensibility point that this can be done in at this time is the JobActivator.

One way to make it work would be to leverage AsyncLocal to have a scoped context in the queue processor's CTOR or BeginProcessingMessageAsync and carry it through to the queue processors dispose or CompleteProcessingMessageAsync.

( Using queue processor as an example here - should be the appropriate processor or a base processor ).

@mathewc
Copy link
Member

mathewc commented Jul 6, 2017

Thanks @Bio2hazard . Recently we've agreed on design changes to enable what you asked for originally (in your IExtendableFunction sketch). We're adding an IInvocationFilter interface which our filter attribute will of course implement, but more importantly you can also implement on your job class as you asked for. We'll then invoke your filters at the right time on your instance. We'll be updating the PR soon with those changes. Once we do we'll let you know so you can try again :) This will enable exactly what you asked for originally.

@mathewc
Copy link
Member

mathewc commented Jul 12, 2017

@Bio2hazard The PR has been updated. Please try again

@Bio2hazard
Copy link
Author

That works beautifully, and even flows nicely through inherited classes. 👍

Thank you so much. Excited to see it in a release in the near future.

@mathewc
Copy link
Member

mathewc commented Aug 8, 2017

Closing this since the parent feature issue is closed. This will be available very soon in the next release.

@mathewc mathewc closed this as completed Aug 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants