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

Adding server telemetry channel, adaptive sampling and making Active as default configuraiton. #157

Merged
merged 4 commits into from Apr 19, 2016

Conversation

kartang
Copy link
Contributor

@kartang kartang commented Apr 18, 2016

Adding server telemetry channel, enabling sampling (with default adaptive) and removing existing configure option to create config.

  • TelemetryConfiguration.Active is used if services do not have an existing configuration instance, in which case it picks the configuration from the services and updates it.
  • Server telemetry channel is assigned to config.TelemetryChannel only in full framework, and if no other telemetry channel exists in services (in which case, config.TelemetryChannel will be the existing channel in services).
  • Neither the channel nor the sampling processor is added to the services collection.
  • Customers have access to telemetry configuration, and can add his own custom telemetry processors through chain builder.
  • Customers can disable sampling using the parameter DisableSampling when adding Application Insights through services -> services.AddApplicationInsightsTelemetry(config, true). This will not add AdaptiveSamplingTelemetryProcessor which is added by default in the other case.
  • Existing boolean parameter on services.AddApplicationInsightsTelemetry to control the usage of TelemetryConfiguration.Active vs TelemetryConfiguration.CreateDefault() is removed and Active is used in all the scenarios.

Note: By default, now, server telemetry channel will be used to send the telemetry, as opposed to InMemoryChannel. If customers have to use their specific channel, they can add channel to the services, and we pick that instead of our default channels.

…tive) and removing existing configure option to create config
"runtime": "clr",
"version": "1.0.0-rc1-update1"
"version": "1.0.0-rc1-update2"
Copy link
Member

@dnduffy dnduffy Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be update3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version corresponds to the dnx runtime. They have updated the run time version, and I referred to the latest available one.

@dnduffy
Copy link
Member

dnduffy commented Apr 19, 2016

:shipit:

@@ -38,10 +40,10 @@ public static IApplicationBuilder UseApplicationInsightsExceptionTelemetry(this
return app.UseMiddleware<ExceptionTrackingMiddleware>();
}

public static void AddApplicationInsightsTelemetry(this IServiceCollection services, IConfiguration config, bool reuseActiveConfig = true)
public static void AddApplicationInsightsTelemetry(this IServiceCollection services, IConfiguration config, bool disableDefaultAdaptiveSampling = false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chatted with David, let's discuss this some more tomorrow before we merge to master.

  1. Consider wrapping this flag in a config structure to allow future changes
  2. Customers who onboarded to previous version with reuseActiveConfig will not be able to notice anything changed and the fact that now this flag has completely different meaning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let us discuss about using the config. However, "reuseActiveConfig" was never available for customers. It is always in develop, and we planned to have it in this merge to master.

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

Successfully merging this pull request may close these issues.

None yet

4 participants