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

TelemetryInitializerBase is internal #351

Closed
ghost opened this issue Feb 24, 2017 · 4 comments
Closed

TelemetryInitializerBase is internal #351

ghost opened this issue Feb 24, 2017 · 4 comments
Milestone

Comments

@ghost
Copy link

ghost commented Feb 24, 2017

Commit b7db1a4 made the TelemetryInitializerBase internal. Was it really the intent? Isn't the purpose of this class to be reused to easily access the HttpContext from the request in a custom TelemetryInitilizer?

The other initializers are also internal, which would prevent from using them...

@pharring
Copy link
Member

@pakrym, since he made the commit. Did we intend to hide them? For contrast, the initializers in the ApplicationInsights-dotnet-server repo are public and so is the WebTelemetryInitializerBase.

@pakrym
Copy link
Contributor

pakrym commented Feb 24, 2017

@pharring our goal was to shrink public API surface so we can change things without breaking changes to public API's. Accessing current HttpContext can pretty easily be done with IHttpContextAccessor

@pharring
Copy link
Member

Thanks. It's very reasonable to limit the public surface area, especially early on.
Agreed that getting the current HttpContext is straightforward.

I think folks are being surprised to find that the derived initializers are also internal (#346 and one other I can't find right now). Right now, it's "all or nothing": i.e. You can call AddApplicationInsightsTelemetry and get our curated set of initializers; or, if you want a customized subset, then you're left re-implementing our list.

I recommend we monitor the feedback on this one and find out which initializers folks would really like to make public.

@AlexBulankou AlexBulankou modified the milestone: Future Jul 5, 2017
@bragma
Copy link

bragma commented Aug 28, 2017

Hi @pharring,
I'm not so sure getting the current HttpContext is straightforward. I had to reimplement something similar to TelemetryInitializerBase since it requires a lock. Nothing overly complex, but considering current documentation does not make any mention, I am still confused on the correct implementation.

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

5 participants