-
Notifications
You must be signed in to change notification settings - Fork 72
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
Cannot configure sink using APPINSIGHTS_CONNECTIONSTRING
without relying on TelemetryClient
service
#163
Comments
Hi, anybody?! |
Because of this issue I had to delete this package and use AI directly: services.AddApplicationInsightsTelemetry(options => options.ConnectionString = s); |
Also:
|
You need to manually initialize the Telemetry Configuration:
Unfortunately, this does not work via appsettings.json based configuration because static methods are only called for interface or abstract type parameters. |
Sorry about the delay. Hoping to help catch up on the backlog, here - #168 is tracking. |
Any updates on this? Instrumentation key-based ingestion has now a deprecation date set https://azure.microsoft.com/en-us/updates/technical-support-for-instrumentation-key-based-global-ingestion-in-application-insights-will-end-on-31-march-2025/ |
@shahiddev is this one you'd be interested in shepherding through? |
Hey @nblumhardt I can try and take a look - aware of the deprecation but wasn't aware of new regions not allowing connection strings |
Awesome :-) @augustoproiete, I'm not 100% across what you have in mind for team structure, but I've invited @shahiddev to the org with maintainer rights in this repo. Drop me a line if there's a particular setup you had in mind. Let me know if I can help get the ball rolling on anything :-) |
Is there any news on this? |
Using: .WriteTo.ApplicationInsights(services.GetRequiredService<TelemetryConfiguration>(), TelemetryConverter.Traces)) should work around this successfully, and is currently the suggested method of configuration. Can anyone please confirm? |
@nblumhardt this does not work when you need to initialize the logger earlier than in the Startup.cs/ConfigureServices already |
@sebader gotcha! So the title of this issue should be something more like Cannot configure sink using It seems like adding a connection string option to the service configuration should be pretty straightforward, is anyone able to help out/take a shot at it? |
APPINSIGHTS_CONNECTIONSTRING
without relying on TelemetryClient
service
Some Azure services (i.e. Azure Functions) automatically injects Application Insights services, as a result I think services.GetRequiredService() should always be the go to configuration. Is there any reason why one would prefer to let the sink setup application insights? |
I'm not aware of any, @zyofeng |
If I understand this correctly, you are trying to log start up errors such as one in Program.cs file
try |
I have added to my PR to switch from using InstrumentationKey to ConnectionString, however this is obviously going to be a breaking change so maybe it should be in 4.0? |
@zyofeng: if it's implemented in this way it's indeed a breaking change. You could also implement a new extension method with this version and leave the instrumentationkey implementation there and mark it as obsolete. Then you allow for a migration path for users. Just a suggestion so it can already be incorporated in version 3.x |
It would not be a problem to make it a breaking change IMHO. The instrumentation key implementation will stop working in Azure soon anyway. |
I'm fine with a breaking change if necessary 👍 What we really need right now is a full proposal for how the sink should be changed (ideally with a PR) - is anyone able to pick this up? I don't use AI at all, so it's very unlikely I'll put this together myself, but I can help review etc. if someone with the knowledge/access to AI can work through it. |
I've merged @zyofeng's PR to Can someone please check that:
If we can verify those two items, I'll merge/release 4.0.0, as there's not much else I can do on #184 right now. Thanks! |
One thing, not sure if this is intentionally or not, but if I put APPLICATIONINSIGHTS_CONNECTION_STRING in appsettings.config rather than as an environment variable it does not get picked up, I would suggest reading the connection string from IConfiguration["APPLICATIONINSIGHTS_CONNECTION_STRING"] because .net core's approach with merging multiple config sources. Technically I can have APPLICATIONINSIGHTS_CONNECTION_STRING configured in a keyvault or Azure App Configuration and shared across multiple containers/apps. |
@zyofeng thanks for checking it out. Calling this ticket done 👍 - will merge 4.0 as soon as I can write the notes. Support for the connection string variable is via the app insights SDK and not tied to any of the .NET configuration system. I think any method that sets the corresponding environment variable should work. |
I need some clarification about that part I need to set the "APPLICATIONINSIGHTS_CONNECTION_STRING" through the appsettings.json and not through Environment Variable but it doesn't seems to work.. Is it intentional that this not working with the appsettings.json config ? Or is it a bug ? Like @zyofeng said, it should be possible to use both case because : So, what about that ? |
Because you can instantiate the telemetry client and pass that into the sink as a param. You can just read the connectionstring from iconfig if needed. |
I don't think I get what you said I don't understand why we should do something different if we don't set the environment variable ... The "APPLICATIONINSIGHTS_CONNECTION_STRING " should be handle if it's set in the environment variable or set in appsettings.json. For the moment, only the environment variable way is working Is it done on purpose ?
What do you think ? |
If you are after some kind of justification I guess here are two. |
Hi, it is recommended in Microsoft docs to use Application Insights connection string over instrumentation key but when adding application insights with that sink will not submit any logs or metrics to Application Insights, when I switch back to using instrumentation key it will start working again.
https://docs.microsoft.com/en-us/azure/azure-monitor/app/ilogger#example-programcs
Thanks.
The text was updated successfully, but these errors were encountered: