search

microsoft / FeatureManagement-Dotnet

Microsoft.FeatureManagement provides standardized APIs for enabling feature flags within applications. Utilize this library to secure a consistent experience when developing applications that use patterns such as beta access, rollout, dark deployments, and more.
MIT License
1.06k stars 115 forks source link

Namespace of ApplicationInsightsTelemetryPublisher #392

Closed zhenlan closed 7 months ago

zhenlan commented 8 months ago

The namespace of ApplicationInsightsTelemetryPublisher is Microsoft.FeatureManagement.Telemetry.ApplicationInsights today. It's cumbersome for users to add long namespaces like this. I'd propose we move it under Microsoft.FeatureManagement.Telemetry. In the future, if we add any new publishers, we can put them all under the same namespace. This way users don't have to add namespaces for each publisher individually.

jimmyca15 commented 8 months ago

Sounds reasonable. Will be more discoverable. Shouldn't cause bloat since a user would have to specifically target this package to have this type appear.

rossgrambo commented 7 months ago

To clarify- we're just changing the namespace and not the package name- correct? Meaning in the future we would have two packages:

  1. Microsoft.FeatureManagement.Telemetry.ApplicationInsights
  2. Microsoft.FeatureManagement.Telemetry.LogAnalytics

Both having:

namespace Microsoft.FeatureManagement.Telemetry;

Additionally, how do we feel about Microsoft.FeatureManagement.Telemetry.ApplicationInsights.AspNetCore ? It holds the TargetingTelemetryInitializer.

Should it follow suit and be:

namespace Microsoft.FeatureManagement.Telemetry.AspNetCore;

?

Assuming this is correct- I made a PR: https://github.com/microsoft/FeatureManagement-Dotnet/pull/417

jimmyca15 commented 7 months ago

Additionally, how do we feel about Microsoft.FeatureManagement.Telemetry.ApplicationInsights.AspNetCore ? It holds the TargetingTelemetryInitializer

I don't think it should change. Here's why. The publisher is an implementation of a generic concept. Any telemetry destination would need it's own publisher. So standardizing to simple namespace allows the implementations to be discoverable easy as Zhenlan mentioned.

But, ITelemetryInitializers are App Insights specific components. I don't think they generalize well.

rossgrambo commented 7 months ago

Makes sense. Updated PR.