Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(Publisher.ApplicationInsights): add connection string support #1480

Merged
merged 5 commits into from
Dec 29, 2022
Merged

feat(Publisher.ApplicationInsights): add connection string support #1480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4115068
feat(AspNetcore.HealthChecks.Publisher.ApplicationInsights): add conn…
skyfrk Oct 8, 2022
5e8b1a2
test(AspNetcore.HealthChecks.Publisher.ApplicationInsights): add conn…
skyfrk Oct 8, 2022
f4b0aa5
Update src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsi…
skyfrk Oct 25, 2022
28fda03
improve XML comment
skyfrk Oct 26, 2022
b80d1e9
Merge branch 'Xabaril:master' into feat/AspNetcore.HealthChecks.Publi…
skyfrk Dec 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,20 @@ internal class ApplicationInsightsPublisher : IHealthCheckPublisher
private static TelemetryClient? _client;
private static readonly object _syncRoot = new object();
private readonly TelemetryConfiguration? _telemetryConfiguration;
private readonly string? _connectionString;
private readonly string? _instrumentationKey;
private readonly bool _saveDetailedReport;
private readonly bool _excludeHealthyReports;

public ApplicationInsightsPublisher(
IOptions<TelemetryConfiguration>? telemetryConfiguration,
string? connectionString = default,
sungam3r marked this conversation as resolved.
Show resolved Hide resolved
string? instrumentationKey = default,
bool saveDetailedReport = false,
bool excludeHealthyReports = false)
{
_telemetryConfiguration = telemetryConfiguration?.Value;
_connectionString = connectionString;
_instrumentationKey = instrumentationKey;
_saveDetailedReport = saveDetailedReport;
_excludeHealthyReports = excludeHealthyReports;
Expand Down Expand Up @@ -110,12 +113,14 @@ private TelemetryClient GetOrCreateTelemetryClient()
{
if (_client == null)
{
//override instrumentation key or use default telemetry
//configuration active on the project.
// Create TelemetryConfiguration
// Hierachy: _connectionString > _instrumentationKey > _telemetryConfiguration
var configuration = string.IsNullOrWhiteSpace(_connectionString)
? string.IsNullOrWhiteSpace(_instrumentationKey)
? _telemetryConfiguration
: new TelemetryConfiguration(_instrumentationKey)
: new TelemetryConfiguration { ConnectionString = _connectionString };

var configuration = string.IsNullOrWhiteSpace(_instrumentationKey)
? _telemetryConfiguration
: new TelemetryConfiguration(_instrumentationKey);

_client = new TelemetryClient(configuration);
}
Expand Down
6 changes: 4 additions & 2 deletions ...pplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ public static class ApplicationInsightsHealthCheckBuilderExtensions
/// indicating the health check status (1 - Healthy, 0 - Unhealthy) and the total time the health check took to execute in milliseconds.
/// </remarks>
/// <param name="builder">The <see cref="IHealthChecksBuilder"/>.</param>
/// <param name="instrumentationKey">Specified Application Insights instrumentation key. Optional. If <c>null</c> TelemetryConfiguration.Active is used.</param>
/// <param name="connectionString">Specified Application Insights connection string. Optional. If <c>null</c> <paramref name="instrumentationKey"/> is used.</param>
/// <param name="instrumentationKey">Specified Application Insights instrumentation key. Optional. If <c>null</c> <see cref="TelemetryConfiguration"/> is resolved from DI container.</param>
/// <param name="saveDetailedReport">Specifies if save an Application Insights event for each HealthCheck or just save one event with the global status for all the HealthChecks. Optional</param>
/// <param name="excludeHealthyReports">Specifies if save an Application Insights event only for reports indicating an unhealthy status</param>
/// <returns>The specified <paramref name="builder"/>.</returns>
public static IHealthChecksBuilder AddApplicationInsightsPublisher(
this IHealthChecksBuilder builder,
string? connectionString = default,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is breaking. Both at binary level and source level - you should point compiler what arg to pass - instrumentationKey: or connectionString:. For someone it can be a problem though I don't see a way to do better decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, can we merge this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe after migrating repo to .NET 7 SDK and bump major version? @carlosrecuero ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

#1533

Copy link
Collaborator

Choose a reason for hiding this comment

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

@skyfrk please pull changes from master to verify CI. Also now is a good time to go on with any breaking changes if any. See #1574.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I synced my fork :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. Waiting for CI.

string? instrumentationKey = default,
bool saveDetailedReport = false,
bool excludeHealthyReports = false)
Expand All @@ -29,7 +31,7 @@ public static IHealthChecksBuilder AddApplicationInsightsPublisher(
.AddSingleton<IHealthCheckPublisher>(sp =>
{
var telemetryConfigurationOptions = sp.GetService<IOptions<TelemetryConfiguration>>();
return new ApplicationInsightsPublisher(telemetryConfigurationOptions, instrumentationKey, saveDetailedReport, excludeHealthyReports);
return new ApplicationInsightsPublisher(telemetryConfigurationOptions, connectionString, instrumentationKey, saveDetailedReport, excludeHealthyReports);
});

return builder;
Expand Down
16 changes: 15 additions & 1 deletion ...HealthChecks.Publisher.ApplicationInsights.Tests/DependencyInjection/RegistrationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,21 @@ public void add_healthcheck_when_properly_configured_with_instrumentation_key_pa
var services = new ServiceCollection();
services
.AddHealthChecks()
.AddApplicationInsightsPublisher("telemetrykey");
.AddApplicationInsightsPublisher(instrumentationKey: "telemetrykey");

using var serviceProvider = services.BuildServiceProvider();
var publisher = serviceProvider.GetService<IHealthCheckPublisher>();

Assert.NotNull(publisher);
}

[Fact]
public void add_healthcheck_when_properly_configured_with_connection_string_parameter()
{
var services = new ServiceCollection();
services
.AddHealthChecks()
.AddApplicationInsightsPublisher(connectionString: "InstrumentationKey=telemetrykey;EndpointSuffix=example.com;");

using var serviceProvider = services.BuildServiceProvider();
var publisher = serviceProvider.GetService<IHealthCheckPublisher>();
Expand Down
2 changes: 1 addition & 1 deletion ...blisher.ApplicationInsights.Tests/HealthChecks.Publisher.ApplicationInsights.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ namespace Microsoft.Extensions.DependencyInjection
{
public static class ApplicationInsightsHealthCheckBuilderExtensions
{
public static Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder AddApplicationInsightsPublisher(this Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder builder, string? instrumentationKey = null, bool saveDetailedReport = false, bool excludeHealthyReports = false) { }
public static Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder AddApplicationInsightsPublisher(this Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder builder, string? connectionString = null, string? instrumentationKey = null, bool saveDetailedReport = false, bool excludeHealthyReports = false) { }
}
}