-
Notifications
You must be signed in to change notification settings - Fork 119
Fix couple 2.0-beta issues #317
Changes from all commits
0225a1a
b385fff
8767c4b
28c1861
d16183c
0d3e55c
45f3a4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,8 @@ public ApplicationInsightsServiceOptions() | |
this.EnableQuickPulseMetricStream = true; | ||
this.EnableAdaptiveSampling = true; | ||
this.EnableDebugLogger = true; | ||
this.ApplicationVersion = Assembly.GetEntryAssembly().GetName().Version.ToString(); | ||
this.EnableAuthenticationTrackingJavaScript = false; | ||
this.ApplicationVersion = Assembly.GetEntryAssembly()?.GetName().Version.ToString(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious but when can GetEntryAssembly return null? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When running in xunit test. |
||
} | ||
|
||
/// <summary> | ||
|
@@ -55,5 +56,11 @@ public ApplicationInsightsServiceOptions() | |
/// Gets or sets a value indicating whether a logger would be registered automatically in debug mode. | ||
/// </summary> | ||
public bool EnableDebugLogger { get; set; } | ||
|
||
/// <summary> | ||
/// Gets or sets a value indicating whether a JavaScript snippet to track the current authenticated user should | ||
/// be printed along with the main ApplicationInsights tracking script. | ||
/// </summary> | ||
public bool EnableAuthenticationTrackingJavaScript { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,13 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using Microsoft.Extensions.Configuration; | ||
|
||
namespace Microsoft.Extensions.Logging | ||
{ | ||
using System; | ||
using ApplicationInsights; | ||
using ApplicationInsights.AspNetCore.Logging; | ||
using Microsoft.Extensions.DependencyInjection; | ||
|
||
/// <summary> | ||
/// Extension methods for <see cref="ILoggerFactory"/> that allow adding Application Insights logger. | ||
|
@@ -17,24 +17,25 @@ public static class ApplicationInsightsLoggerFactoryExtensions | |
/// <summary> | ||
/// Adds an ApplicationInsights logger that is enabled for <see cref="LogLevel.Warning"/> or higher. | ||
/// </summary> | ||
public static ILoggerFactory AddApplicationInsights(this ILoggerFactory factory, | ||
TelemetryClient client) | ||
/// <param name="factory"></param> | ||
/// <param name="serviceProvider">The instance of <see cref="IServiceProvider"/> to use for service resolution.</param> | ||
public static ILoggerFactory AddApplicationInsights(this ILoggerFactory factory, IServiceProvider serviceProvider) | ||
{ | ||
return factory.AddApplicationInsights(client, LogLevel.Warning); | ||
return factory.AddApplicationInsights(serviceProvider, LogLevel.Warning); | ||
} | ||
|
||
/// <summary> | ||
/// Adds an ApplicationInsights logger that is enabled for <see cref="LogLevel"/>s of minLevel or higher. | ||
/// </summary> | ||
/// <param name="factory"></param> | ||
/// <param name="client">The instance of <see cref="TelemetryClient"/> to use for logging.</param> | ||
/// <param name="serviceProvider">The instance of <see cref="IServiceProvider"/> to use for service resolution.</param> | ||
/// <param name="minLevel">The minimum <see cref="LogLevel"/> to be logged</param> | ||
public static ILoggerFactory AddApplicationInsights( | ||
this ILoggerFactory factory, | ||
TelemetryClient client, | ||
IServiceProvider serviceProvider, | ||
LogLevel minLevel) | ||
{ | ||
factory.AddApplicationInsights(client, (category, logLevel) => logLevel >= minLevel); | ||
factory.AddApplicationInsights(serviceProvider, (category, logLevel) => logLevel >= minLevel); | ||
return factory; | ||
} | ||
|
||
|
@@ -43,12 +44,19 @@ public static ILoggerFactory AddApplicationInsights( | |
/// </summary> | ||
/// <param name="factory"></param> | ||
/// <param name="filter"></param> | ||
/// <param name="client">The instance of <see cref="TelemetryClient"/> to use for logging.</param> | ||
/// <param name="serviceProvider">The instance of <see cref="IServiceProvider"/> to use for service resolution.</param> | ||
public static ILoggerFactory AddApplicationInsights( | ||
this ILoggerFactory factory, | ||
TelemetryClient client, | ||
IServiceProvider serviceProvider, | ||
Func<string, LogLevel, bool> filter) | ||
{ | ||
var client = serviceProvider.GetService<TelemetryClient>(); | ||
var debugLoggerControl = serviceProvider.GetService<DebugLoggerControl>(); | ||
if (debugLoggerControl != null) | ||
{ | ||
debugLoggerControl.EnableDebugLogger = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious so I understand this, when DebugLoggerControl is created the flag is defaulted to true but then here when adding App Insights it is set to false, can you explain the flow for enabling and disabling the debug logger? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idea here is to enable debug time logger only when there is no other logger added explicitly. So when adding debug logger we include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank-you. |
||
} | ||
|
||
factory.AddProvider(new ApplicationInsightsLoggerProvider(client, filter)); | ||
return factory; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
namespace Microsoft.ApplicationInsights.AspNetCore.Logging | ||
{ | ||
using Microsoft.Extensions.Logging; | ||
|
||
/// <summary> | ||
/// Class to control default debug logger and disable it if logger was added to <see cref="ILoggerFactory"/> explicetely. | ||
/// </summary> | ||
internal class DebugLoggerControl | ||
{ | ||
public DebugLoggerControl() | ||
{ | ||
EnableDebugLogger = true; | ||
} | ||
|
||
/// <summary> | ||
/// This property gets set to <code>false</code> by <see cref="ApplicationInsightsLoggerFactoryExtensions.AddApplicationInsights"/>. | ||
/// </summary> | ||
public bool EnableDebugLogger { get; set; } | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
namespace Microsoft.ApplicationInsights.AspNetCore.TelemetryInitializers | ||
{ | ||
using Microsoft.ApplicationInsights.Channel; | ||
using Microsoft.ApplicationInsights.Extensibility; | ||
using Microsoft.AspNetCore.Hosting; | ||
|
||
/// <summary> | ||
/// <see cref="ITelemetryInitializer"/> implementation that stamps ASP.NET Core environment name | ||
/// on telemetries. | ||
/// </summary> | ||
internal class AspNetCoreEnvironmentTelemetryInitializer: ITelemetryInitializer | ||
{ | ||
private const string AspNetCoreEnvironmentPropertyName = "AspNetCoreEnvironment"; | ||
private readonly IHostingEnvironment environment; | ||
|
||
/// <summary> | ||
/// Initializes a new instance of the <see cref="AspNetCoreEnvironmentTelemetryInitializer"/> class. | ||
/// </summary> | ||
public AspNetCoreEnvironmentTelemetryInitializer(IHostingEnvironment environment) | ||
{ | ||
this.environment = environment; | ||
} | ||
|
||
/// <inheritdoc /> | ||
public void Initialize(ITelemetry telemetry) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add doc comments. |
||
{ | ||
if (environment != null && !telemetry.Context.Properties.ContainsKey(AspNetCoreEnvironmentPropertyName)) | ||
{ | ||
telemetry.Context.Properties.Add(AspNetCoreEnvironmentPropertyName, environment.EnvironmentName); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can "Value" ever be null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I don't think so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
IOptions
infrastructure will ensure a value is created.