Make using of TelemetryConfiguration.Active singleton optional

[SergeyKanzhelev]

we need to make use of TC.Active singleton optional so you can host multiple apps in the same process. Also it will make unit tests better

[dnduffy]

We had it optional but putting it in the API was sub optimal so we removed it again.
Is same process the concern or same app domain? The singleton instance shouldn't be per process since app domains are isolated from each other.
What is the impact on tests either way? We can currently configure independently for tests if need be.

[SergeyKanzhelev]

same app domain. You can run multiple apps in a single process.

tests runs asynchronously and re-use shared configuration. So there may be random failures when telemetry item sent by one test is received in StubChannel by another test.

[dnduffy]

Just to be clear, the process and the app domain are separate and different. The process can be shared, such as shared hosting in IIS, but each app is in a separate app domain (or even more than one). App domains do not share static variables with other apps, even in the same process, because that would make isolation impossible in IIS.
I sometimes think it would be nice if tests ran in parallel but I have yet to see a test runner system actually do this (at least in VS) due to the potential for conflicts with side effects from setup and teardown methods. Most of our tests re-use the shared config but, if I recall correctly, one of them does not because it actively tests the scenario of multiple different configurations for TelemetryClients. To the best of my knowledge an individual test has to wait on any asynchronous processing, either explicitly or via the test framework, because otherwise it cannot assert on the results and conflicts such as you mention would abound.
If I'm not understanding something lets discuss in person.

[SergeyKanzhelev]

This code will run two applications in a single process in a single domain:

this.hostingEngine = CreateBuilder() 
  .UseContentRoot(Directory.GetCurrentDirectory()) 
  .UseUrls("http://localhost:8080") 
  .UseKestrel() 
  .UseStartup(assemblyName) 
  .UseEnvironment("Production") 
  .Build(); 

this.hostingEngine = CreateBuilder() 
  .UseContentRoot(Directory.GetCurrentDirectory()) 
  .UseUrls("http://localhost:8081") 
  .UseKestrel() 
  .UseStartup(assemblyName) 
  .UseEnvironment("Production") 
  .Build(); 

This is an extreme case, but asp.net core is designed for this use. So we should allow telemetry to be adjusted for this use

[dnduffy]

Okay, I understand the edge case in question, someone writes one app that creates multiple self-host endpoints and wants each endpoint to report to a different iKey.
I would like to address this in vNext and discuss what and where things are registered for dependency injection for my own clarity.

[ohadschn]

This edge case could easily happen in a Service Fabric application. For example if you had different named instances of the same stateless ASP.NET Core service type, they could all end up in the same process (and more importantly in the same app domain).

Another reason for this is infra code that is shared between ASP.NET Core and non-ASP.NET Core services. That code has to know now whether ASP.NET Core AI is active, so as to avoid adding initializers, modules, and processors that will be added by ASP.NET Core AI (if it doesn't, they will end up twice in the configuration, which in most/all cases would be undesirable).

IMO when UseApplicationInsights is called it should create a new TelemetryConfiguration by merging the the existing TelemetryConfiguration.Active with the desired configuration. That is only add missing initializers/processors/modules, and for existing ones possibly merge their existing configuration with the desired one. That way I can setup my AI as I like it on service startup, and ASP.NET Core AI can do its own thing without us interfering with each other.

[SergeyKanzhelev]

@ohadschn I agree those scenarios increases the priority of this work. Having nested configuration settings is a good idea. However there should be a way to gain a full control on the configuration for specific app.

Thank you! We will be discussing the best options for configuration story improvements with aspnet team.

Meanwhile if it's blocking your scenario - feel free to send PR to fix it by passing a flag to UseApplicationInsights that will force to not use Active.

[lmolkova]

#622
microsoft/ApplicationInsights-dotnet#613

I do not think a configuration flag is an option as in shared process scenario app breaks throwing before sending HTTP request or the whole AI pipeline just becomes noop after TelemteryConfiguration.Active.TelemetryChannel is disposed.

[EnCey]

So what is the recommendation if one is forced to use TelemetryConfiguration.Active @lmolkova ?

We are building an Azure Cloud Service with a Worker Role where one must use the Active property to set configuration options. ASP.NET Core is only one of the services our Worker Role provides, so it shouldn't be the one that dictates options, especially since we need telemetry before we get to the point where it is initialized. Otherwise we're in the dark if initialisation of another component fails.

According to comments in the #622 PR it should be fine to first configure via Active and let ASP.NET Core pick up that configuration. However in the FAQs it states that "modifications to TelemetryConfiguration.Active will not be picked up by SDK's own modules", so which is it?

I'd prefer an alternative to using the static Active property for non-ASP.NET Core configuration of AI, but I haven't found any clues that something like that exists (for Cloud Service Worker Roles). I'd appreciate if the FAQs could be updated to clarify the degree to which one can combine Active and DI configuration.

[lmolkova]

@EnCey there is no requirement to use Active on worker roles. As long as you are using AspNetCore you can set instrumentation key via appsettings.json, environment variable or in code by supplying it to UseApplicationInsights or AddApplicationInsights extension methods.

In non-AspNetCore environments or telemetry reported before the host is built, you can create and setup TelemteryConfiguration manually in the code. It could be Active singleton or the instance you create and pass explicitly - it actually does not matter as long as you set instrumentation key on it.
The main caveat here is that AspNetCore configuration does not respect your changes on the Active instance. It uses another instance of TelemetryConfiguration and tracks telemetry accordingly.

We discourage anyone from using static singletons except cases where there is no dependency injection available - ASP.NET Classic applications or pure Console apps, but it's your choice what to use.
I agree we should work on our documentation to make it more clear regarding Active usage.

Could you please tell me more about the scenario and concern you have?

[EnCey]

I'd like to create a TelemetryConfiguration instance once, very early when our WorkerRole starts up. Currently we're using the Active property to populate that configuration. That configuration is intended not only for AspNetCore but also other components of our WorkerRole. We're not only setting the instrumentation key but also other stuff like TelemetryInitializers. Hence it would be best to only have a single place where this configuration is created.

I'd be happy to stop using the Active property, but I was under the impression that this is necessary to get "dependencies", such as Azure Storage components, to use the same configuration. We are currently switching to AspNetCore from a self-hosted OWIN/WebAPI stack so I'm not sure how these configuration settings are passed on now.

In short: we don't have "just" AspNetCore. AspNetCore specific TelemetryConfiguration does not carry over to our other components (does it?). We'd like to avoid having to create the same configuration in several places. If there's an alternative, I haven't found it yet – all Cloud Service documentation I found mentions the Active property.

Perhaps this makes more sense: what is the recommended way to configure AI for both AspNetCore and other components? Should we use Active and mirror the configuration in AspNetCore DI, or can we somehow pass a TelmetryConfiguration object to AspNetCore (and use it manually in other places), but if so how do we get things like Azure Storage services (blobs, tables) to use that configuration (when used outside AspNetCore)?

[kyschouv]

Is there an ETA on this? It's breaking out Integration tests as per Microsoft/ApplicationInsights-dotnet#613.

[lmolkova]

@kyschouv

#613 and #621 are fixed. #613 with AppInsights AspnetCore 2.3.0 and #621 with 2.6.0-beta3 (not stable yet).

AppInsights AspNetCore SDK creates TelemetryConfiguration per host since 2.3.0 and only sets up Active once per process lifetime.

I would not expect any issues with integration tests at the moment related to this issue if you run 2.3.0 or higher. If you do, please create another issue and provide the details.

@EnCey
Sorry for not getting to your questions earlier: we discourage use of TelemetryConfiguration.Active in AspNetCore (or any other case where DI pattern is available). In the case of AspNetCore, you can always get TelemetryConfiguration instance (or TelemetryClient) from the DI container and never create/configure another one.

If you need a custom configuration, you may create a new configuration and pass it over to other parts of service using DI or any other way you want.

If you want to affect how auto-collectors report telemetry, you may tweak TelemetryConfiguration created by AppInsights for AspNetCore - you can find examples in the wiki.

None of this affects dependency collection (unless you configure DependencyTrackingTelemetryModule differently).

I think this issue could be closed now since Active singleton is no longer used by AppInsights for AspNetCore.

[EnCey]

@lmolkova thanks for getting back on this. The fundamental problem that I see is that there is an ApplicationInsights.config in our main cloud service project that has nothing to do with AspNetCore. It defines a bunch of telemetry modules, initializers, processors and so on. As far as I'm aware, only the Active property can be used to configure these.

We have a separate config for our AspNetCore project that is used by the worker role and use DI here. The thing is, we have other projects that are independent from AspNetCore. They have their own entry points so no config can "flow" via execution contexts (e.g. Redis pub/sub). As a result, no AspNetCore config can work there.

I realize that all of this is a Cloud Service issue and not an AspNetCore one; I just wished there would have been a way to pass a TelemetryConfiguration instance to AspNetCore so we can set them up in a central place. As things are, one must currently use various builder methods to configure AspNetCore AI, which cannot be done outside the AspNetCore project. In the end we now have a central place for configuration and must manually mirror the same config in the AspNetCore project because there's no way to pass or copy existing settings over.