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

Request for comments #1

Open
Kralizek opened this issue Mar 2, 2017 · 6 comments
Open

Request for comments #1

Kralizek opened this issue Mar 2, 2017 · 6 comments

Comments

@Kralizek
Copy link
Owner

Kralizek commented Mar 2, 2017

@normj what do you think? I put it down in a couple of hours, it can obviously be improved.

The idea is having a well-structured template so that developers know what to add where. Also, it should enforce best-practices like DI.

@normj
Copy link

normj commented Mar 3, 2017

Interesting to have a light weight abstraction for using the DI.

I think ConfigureServices should be called as part of the constructor or on first invoke so that you aren't setting up the DI for every Lambda invocation. Also a lot of the logic in RequestResponseFunction's FunctionHandlerAsync seems like what you do inside ConfigureServices.

ConfigureExecution seems oddly specific for setting up the logging factory. What is the full intent of that override.

You might want a default implementation of Configure(IConfigurationBuilder builder) which pulls in the environment variables and appsettings.json as optional. That would reduce what users have to implement.

I do like where you are going with this.

@Kralizek
Copy link
Owner Author

Kralizek commented Mar 4, 2017

Thank you for your feedback. I agree with your post although I have some thoughts about few bullets.

First of all, I already had moved the call to ConfigureServices to the constructor (see 01c5dda).
As you can see, now there is a dedicated overridable method called ConfigureLogging that receives the ILoggerFactory so that the final class can simply attach the different logging providers.

When it comes to offer a default implementation, both ConfigureLogging(ILoggerFactory) and Configure(IConfigurationbuilder) represent a fork. Should this template try to minimize as much as possible the boiler plate?

I'm thinking of something like

        protected FunctionTemplate()
        {
            ...
            var loggerFactory = ServiceProvider.GetRequiredService<ILoggerFactory>();

            ConfigureLogging(loggerFactory);

            Logger = loggerFactory.CreateLogger("Function");
        }

        protected virtual void ConfigureLogging(ILoggerFactory loggerFactory) 
        {
            loggerFactory.AddLambdaLogger(new LambdaLoggerOptions
            {
                IncludeCategory = true,
                IncludeLogLevel = true,
                IncludeNewline = true
            });
        }

versus something like

        protected FunctionTemplate()
        {
            ...
            var loggerFactory = ServiceProvider.GetRequiredService<ILoggerFactory>();

            loggerFactory.AddLambdaLogger(new LambdaLoggerOptions
            {
                IncludeCategory = true,
                IncludeLogLevel = true,
                IncludeNewline = true
            });

            ConfigureLogging(loggerFactory);

            Logger = loggerFactory.CreateLogger("Function");
        }

        protected virtual void ConfigureLogging(ILoggerFactory loggerFactory) { }

In the first case, the end user can still opt-out from what I think is the default setup (but it will still have the dependencies to, say, the LambdaLogger, Configuration.EnvironmentVariables, Configuration.Json and so on...).
In the second case, the user can't escape my choices. It could even go as far as creating problems if the user attaches the same logger provider again.

To summarize there are three options:

  1. slim template, the user configures everything
  2. weak template, the user can opt-out of the default setup (with dead dependencies)
  3. strong template, the user can only augment the default setup

When it comes to ConfigureExecution, it's kind of an awkward topic. I would like the end user to be able to customize the behavior of its function depending on the ILambdaContext we receive for each execution, but the DI framework is (rightly, imho) built upon separating the construction of the dependency tree from the resolution of dependencies (separation between IServiceCollection and IServiceProvider). The same applies to the configuration subsystem (IConfigurationBuilder vs IConfigurationRoot). This, also prevents the possibility of using ILambdaContext in the handler unless I change the signatures of IRequestResponseHandler and IEventHandler to also receive the context or a type built to include the relevant information it contains.

Last but not least, I can't find a way to avoid passing all the needed types when registering the handler (cfr. services.AddRequestResponseHandler<UpperCaseHandler, string, string>();).
An alternative would be create a convenient method like

protected void RegisterHandler<THandler>(IServiceCollection services) 
    where THandler : class, IRequestResponseHandler<TInput, TOutput>
{
    services.AddRequestResponseHandler<THandler, TInput, TOutput>();
}

to be used like

RegisterHandler<UpperCaseHandler>(services);

But this would break the convention of using extension methods.

@Kralizek
Copy link
Owner Author

Kralizek commented Mar 8, 2017

@normj sorry my total n00bness in GitHub. Do I need to tag you to notify you of a comment? I hope this doesn't look too rude :P

@Compufreak345
Copy link

Hey @Kralizek , thanks for creating this template.

It's a good source for getting an idea on how to create a DI-Pipeline in lambda functions with less overhead than provided by Amazon.Lambda.AspNetCoreServer.

As I am quite new to the topic and only have experience with Azure, I have some "noob"-questions ;)

How exactly does this fit into the Lifecycle of lambda functions?

The constructor gets called once on function init and the DI pipeline stays existent until the function runtime gets recycled, right?

Do you see any option to share the same DI pipeline for multiple functions? This is what we did for our Azure functions and I think it would be more reasonable regarding resource pooling, memory usage, etc.

As far as I understand Lambda functions right now each single function (or to name it differently, each "trigger event") has its own function runtime thus sharing resources is not possible. So if I want to leverage a shared setup I would do so by using inheritance, but it in fact would set up my dependency pipeline once per function implementation.

Did you do any comparison on your approach with using Amazon.Lambda.AspNetCoreServer and stripping it down by overriding CreateHostBuilder like described here and implementing custom function handlers?

Did you try out your template in practice and can share any experiences with it?

Sorry if my thoughts are a bit unsorted, I am currently brainstorming what would be the best approach to rebuild our application with AWS lambda. I am 100% sure we need dependency injection, I am just not sure about the pros and cons between the 2 approaches I described above.

@Kralizek
Copy link
Owner Author

Hi @Compufreak345

This template tries to fit into the normal function lifecycle as much as possible:

  1. The constructor is executed once when the function is launched for the first time. This is when the registration of the services and the configuration happens.
  2. On each execution of the lambda, the handler method is invoked. At this time a new session of the IServiceProvider is created and the request is processed by the handler
  3. In my personal experience, trying to make Lambda functions more complex than they are is never worth the effort. If you want to reuse some of the logic across several functions, I'd suggest you to create a solution with multiple functions, each referring to a shared class library.
  4. Amazon.Lambda.AspNetCoreServer is used to host REST API applications in Lambda. This template is not tackling that issue.
  5. I've been using this template in production at my previous workplace for many years (AFAIK they are still using it and still developing new functions based off it)

Please let me know if you have more questions!

@Compufreak345
Copy link

Thanks for the fast response, I'll try it out during the next couple of days and will report back if there are any more questions or ideas for improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants