Skip to content
This repository has been archived by the owner on Jun 10, 2020. It is now read-only.

Make TelemetryClient service request-scoped instead of application-scoped #11

Closed
olegsych opened this issue Apr 10, 2015 · 4 comments
Closed
Assignees

Comments

@olegsych
Copy link
Member

We currently scope TelemetryClient to the entire application, which can lead to hard to diagnose problems when developers change its request-specific properties, like TelemetryClient.Context.Session.Id. I think the TelemetryClient service should be request-scoped and rely on the container to inject the application-scoped TelemetryConfiguration (see issue #10).

cc: @SergKanz , @abaranch, @upendras, @sergei-nikitin

@SergeyKanzhelev
Copy link
Contributor

It definitely makes sense with the "new" approach we will discuss. With the current approach we need to have a place to set InstrumentationKey
https://github.com/Microsoft/AppInsights-aspnetv5/wiki/Configure#code-based-approach
and set global properties:
https://github.com/Microsoft/AppInsights-aspnetv5/wiki/Configure#add-application-level-context-properties

I don't see alternative (unless we will ask to use TelemetryConfiguration.Active very explicitly)

@olegsych
Copy link
Member Author

The instrumentation key is easy - you can set it on the application scoped TelemetryConfiguration (see #10). Other application scoped properties like ComponentContext.Version can be set with a context initializer, perhaps hidden by a nice extension method.

The reason I believe we should not use the Active singleton is because it wont work in multitenant apps like Orchard. Plus we don't know how many RequestDelegate pipelines per AppDomain there could be.

@SergeyKanzhelev
Copy link
Contributor

Oh, you propose to add TelemetryConfiguration to service provider as singleton instead. Sounds reasonable for me.

@olegsych
Copy link
Member Author

Done.

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

No branches or pull requests

2 participants