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 custom dependency injection #1579

Closed
brettsam opened this issue Jun 6, 2017 · 19 comments
Closed

Allow custom dependency injection #1579

brettsam opened this issue Jun 6, 2017 · 19 comments
Assignees
Milestone

Comments

@brettsam
Copy link
Member

@brettsam brettsam commented Jun 6, 2017

Customers may want to plug in their own logging pipelines and replace ours (i.e. ILogger -> Serilog -> AppInsights). Need to investigate how we can make this possible.

@channeladam
Copy link

@channeladam channeladam commented Jun 6, 2017

We have large enterprise integrations and want the flexibility to use structured logging libraries such as Serilog control how our app's business logic logs are sent to Application Insights.

Why?

Hopefully this doesn't really need to be asked! The output of a log message / trace / event that is written through Serilog is customisable and can be formatted better and easier for support teams to see the information that is relevant for them to perform monitoring and resolve issues.

What are we doing now?

Right now we have Serilog configured with the AI Sink for Serilog so that we get the nice structure logs/trace in AI & OMS.

If we were to use a special Serilog sink that allows us to write out Serilog messages into the provided Function's ILogger, then AI & OMS will have duplicate logs (e.g. of exceptions)... and enterprises don't want to see duplicate exceptions in AI & OMS... and don't want to be alerted more than once for the same issue!

Discussion

Because of duplicate logs/exceptions/alerts issue, we aren't writing to the Function App ILogger, and that means that the Function App portal is effectively useless to us (and our enterprise clients). It would be nice though to be able to use the Function App portal to see the logs and exceptions in more real-time instead of always having to wait for the logs to make it into AI & OMS (minutes). Especially in testing phases!

Category filters won't help with this.
And we would still like the function host to be using AI and writing out metrics there - so we still do want to provide an AI key for the function!
But we don't want to be given an ILogger that also writes to AI!

If our function was given a LoggerFactory, we could hook Serilog into it and get the desired structured logging... BUT even if you did this, there still could be an issue because we wouldn't necessarily have the current configurable mechanism to sink into AI in the way we want to (e.g. log as trace vs custom events).

Proposed Solution

So the only thing I can think of that give us the flexibility is the option to be given something like an IFunctionAppLogLogger *Trademark :) that is disconnected from AI but will log to the Function App logs and thus be visible in the Function App portal.

Thanks!

@justinyoo
Copy link

@justinyoo justinyoo commented Jun 17, 2017

Here's my two cents:

If we can define an assembly reference for ILogger implementation in host.json just like those properties - scriptFile and entryPoint at function.json, I guess we might be able to add our own/custom loggers including serilog.

@brettsam
Copy link
Member Author

@brettsam brettsam commented Jun 19, 2017

The goal will likely be to expose a method for dependency injection. With that, you can supply your own ILoggerFactoryBuilder via a precompiled assembly (this is the interface we use to set up the LoggerFactory). I have a proof-of-concept working that does this but it needs some refining. Since we'll need to support it long-term, we want to make sure we're exposing an extensibility pattern that is more generally useful and not just for logging.

Using a function won't work (easily) as the LoggerFactory needs to be set up before the Host is created, and a Host is needed to run a function.

@brettsam
Copy link
Member Author

@brettsam brettsam commented Jun 19, 2017

Renaming this as it's become more general purpose.

@tjrobinson
Copy link

@tjrobinson tjrobinson commented Jun 20, 2017

I don't think this solves your problem but for anyone coming across this thread because they want to log from WebJobs/Azure Functions to Serilog, check this out: https://github.com/StarRez/Serilog.Sinks.AzureWebJobsTraceWriter

@ScottHolden
Copy link

@ScottHolden ScottHolden commented Oct 3, 2017

Another nice way is via BYOB. I've got a small sample up at: https://github.com/ScottHolden/WebJobs.ContextResolver

@tjrobinson
Copy link

@tjrobinson tjrobinson commented Oct 4, 2017

BYOB = Bring Your Own Binder?

@tjrobinson
Copy link

@tjrobinson tjrobinson commented Oct 4, 2017

@ScottHolden Would your ContextResolver work where the value you need to resolve is a parameter of an attribute? For example the ServiceBusTriggerAttribute takes in a number of parameters, e.g. topicName and subscriptionName. I needed to resolve those from configuration at runtime.

See: Azure/Azure-Functions#345

@MV10
Copy link

@MV10 MV10 commented Feb 12, 2018

Although a replacement logger is important, in the mean time I'll tag this Azure-Functions #299 discussion. Easy to inject something like Serilog this way. You just don't get portal integration, obviously.

I sure wish we could just use normal DI in Functions (but that would require abandoning this odd fixation on static classes... feels like it's 2001 all over again...).

@m1nkeh
Copy link

@m1nkeh m1nkeh commented Mar 10, 2018

Has there been any movement on this issue at all recently... i can see no "proper" updates for a few months now, and we are just facing this issue.

We need / want to store all of our Keys in KeyVault (CosmosDb, Blob Storage, AppInsights Etc.), and therefore ILogger is a bit too restrictive within Functions, and doing it completely ourselves and having to scatter all the built in telemetry you get with ILogger out of the box is a bit galling...

Need a way to do middle ground really....

@tsahi
Copy link

@tsahi tsahi commented Mar 19, 2018

While considering this, please also consider the use case of Bot Framework bots based on Functions. In bots, it is common to instantiate the root dialog from the function, and then instantiate more dialogs from the root dialog, depending on the conversation flow. each such dialog may depend on services that need to be injected as well.

@Mike-E-angelo
Copy link

@Mike-E-angelo Mike-E-angelo commented Mar 29, 2018

but that would require abandoning this odd fixation on static classes... feels like it's 2001 all over again..

100% my thinking @MV10 ... I'm super pumped over the vision of Azure Functions, but the static, (SLOW!) reflection-based voodoo of the Run function has got to be addressed! 😆 It would be nice to have a POCO-based design with strongly-typed interfaces that we can use so we don't have so many messy attributes acting as configuration. This of course would make it easier for DI and be more congruent with ASP.NET Core, etc.

I understand the history here in wanting to ensure ease-of-use for the script kiddies, but now you've got all the superduper hotshot .net devs who want to get into the action too, as this issue clearly demonstrates.

Oh well, maybe for v3. 😄

@aderderian
Copy link

@aderderian aderderian commented Aug 9, 2018

Any updates on this one. I would love to be able to use a LoggerFactory so I can inject ILogger into my service layers.

@paulbatum
Copy link
Member

@paulbatum paulbatum commented Aug 9, 2018

No relevant updates on this one right now. We're focused on functions V2 GA and this issue is beyond the scope of that release.

@hiraldesai
Copy link

@hiraldesai hiraldesai commented Oct 3, 2018

Can someone advise how long before full DI support will be available?

Also - will the DI support work with third party IoC containers like Autofac the same way how dotnet core apps support it?

@ins4n333
Copy link

@ins4n333 ins4n333 commented Oct 20, 2018

Hi @brettsam.
I tried to use microsoft/ApplicationInsights-aspnetcore#759 (comment) for adding custom TelemetryInitializers.
But it still doest not work for me. Debugger also doesnt reach Initialize method. It looks as telemetryconfiguration with all telemetryInitializers are configured before my custom Configure(IWebJobsBuilder builder).
In my func project I use next packages:
Microsoft.ApplicationInsights.AspNetCore" Version="2.5.0"
"Microsoft.Azure.WebJobs" Version="3.0.1"
"Microsoft.Azure.WebJobs.Extensions.DurableTask" Version="1.6.2"
"Microsoft.NET.Sdk.Functions" Version="1.0.23"

May be there is another way to add TelemetryInitializers?

@ins4n333
Copy link

@ins4n333 ins4n333 commented Oct 20, 2018

I have just found a hack)
I can "replace" an existing TelemetryClient (service) in serviceCollection ( IWebJobsBuilder ) to my own implementation. When I did it my custom telemetryinitializer stated working..

@philn5d
Copy link

@philn5d philn5d commented Jan 3, 2019

Maybe it's just me, but it seems like it would be good to handle it just a bit differently. Surface the log stream in a way that it can be subscribed to outside the function hosting environment. This way, ops or devs can tap into the log stream without having to change any function code. It should still write to the file system but pop the events onto a queue too. Or at least have that option. What is a log but a record of a series of events anyway?

@MisinformedDNA
Copy link

@MisinformedDNA MisinformedDNA commented Jun 18, 2020

Hasn't this been completed?

@brettsam Can this be closed?

@brettsam brettsam closed this Aug 18, 2020
@msftbot msftbot bot locked as resolved and limited conversation to collaborators Sep 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet