Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Updated Javascript Snippet with latest from [Github/ApplicationInsights-JS](https://github.com/Microsoft/ApplicationInsights-JS)
- [Make all built-in TelemetryInitializers public to allow easy removal from DI Container.](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/351)
- [Enforced limits of values read from incoming http requests to prevent security vulnerability](https://github.com/Microsoft/ApplicationInsights-aspnetcore/pull/608)
- [ApplicationInsightsLogger adds EventId into telemetry properties. It is off by default for compatibility. It can be switched on by configuring ApplicationInsightsLoggerOptions.](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/569)

## Version 2.2.1
- Updated Web/Base SDK version dependency to 2.5.1 which addresses a bug.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@ internal class ApplicationInsightsLogger : ILogger
private readonly string categoryName;
private readonly TelemetryClient telemetryClient;
private readonly Func<string, LogLevel, bool> filter;
private readonly ApplicationInsightsLoggerOptions options;
private readonly string sdkVersion;

/// <summary>
/// Creates a new instance of <see cref="ApplicationInsightsLogger"/>
/// </summary>
public ApplicationInsightsLogger(string name, TelemetryClient telemetryClient, Func<string, LogLevel, bool> filter)
public ApplicationInsightsLogger(string name, TelemetryClient telemetryClient, Func<string, LogLevel, bool> filter, ApplicationInsightsLoggerOptions options)
{
this.categoryName = name;
this.telemetryClient = telemetryClient;
this.filter = filter;
this.options = options;
this.sdkVersion = SdkVersionUtils.VersionPrefix + SdkVersionUtils.GetAssemblyVersion();
}

Expand All @@ -51,7 +53,7 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
if (exception == null)
{
var traceTelemetry = new TraceTelemetry(formatter(state, exception), this.GetSeverityLevel(logLevel));
PopulateTelemetry(traceTelemetry, stateDictionary);
PopulateTelemetry(traceTelemetry, stateDictionary, eventId);
this.telemetryClient.TrackTrace(traceTelemetry);
}
else
Expand All @@ -60,16 +62,30 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
exceptionTelemetry.Message = formatter(state, exception);
exceptionTelemetry.SeverityLevel = this.GetSeverityLevel(logLevel);
exceptionTelemetry.Context.Properties["Exception"] = exception.ToString();
PopulateTelemetry(exceptionTelemetry, stateDictionary);
PopulateTelemetry(exceptionTelemetry, stateDictionary, eventId);
this.telemetryClient.TrackException(exceptionTelemetry);
}
}
}

private void PopulateTelemetry(ITelemetry telemetry, IReadOnlyList<KeyValuePair<string, object>> stateDictionary)
private void PopulateTelemetry(ITelemetry telemetry, IReadOnlyList<KeyValuePair<string, object>> stateDictionary, EventId eventId)
{
IDictionary<string, string> dict = telemetry.Context.Properties;
dict["CategoryName"] = this.categoryName;

if (this.options?.IncludeEventId ?? false)
{
if (eventId.Id != 0)
{
dict["EventId"] = eventId.Id.ToString(System.Globalization.CultureInfo.InvariantCulture);
}

if (!string.IsNullOrEmpty(eventId.Name))
{
dict["EventName"] = eventId.Name;
}
}

if (stateDictionary != null)
{
foreach (KeyValuePair<string, object> item in stateDictionary)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using ApplicationInsights;
using ApplicationInsights.AspNetCore.Logging;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

/// <summary>
/// Extension methods for <see cref="ILoggerFactory"/> that allow adding Application Insights logger.
Expand Down Expand Up @@ -64,6 +65,12 @@ public static ILoggerFactory AddApplicationInsights(
{
var client = serviceProvider.GetService<TelemetryClient>();
var debugLoggerControl = serviceProvider.GetService<ApplicationInsightsLoggerEvents>();
var options = serviceProvider.GetService<IOptions<ApplicationInsightsLoggerOptions>>();

if (options == null)
{
options = Options.Create(new ApplicationInsightsLoggerOptions());
}

if (debugLoggerControl != null)
{
Expand All @@ -75,7 +82,7 @@ public static ILoggerFactory AddApplicationInsights(
}
}

factory.AddProvider(new ApplicationInsightsLoggerProvider(client, filter));
factory.AddProvider(new ApplicationInsightsLoggerProvider(client, filter, options));
return factory;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
namespace Microsoft.ApplicationInsights.AspNetCore.Logging
{
/// <summary>
/// Options for configuration of <see cref="ApplicationInsightsLogger"/>.
/// </summary>
public class ApplicationInsightsLoggerOptions
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is another PR which introduces ApplicationInsightsLoggerOptions
https://github.com/Microsoft/ApplicationInsights-aspnetcore/pull/574/files
Can you also have took at that as well?

I think passing ApplicationInsightsLoggerOptions to both ApplicationInsightsLoggerProvider and ApplicationInsightsLogger as done in the other PR is a better option. Can you incorporate this change to your PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the hint. Yes, it's actually better. Also I realized that classes are internal, so I can change signature of constructors.

{
/// <summary>
/// Gets or sets value indicating, whether EventId and EventName properties should be included in telemetry.
/// </summary>
public bool IncludeEventId { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{
using System;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

/// <summary>
/// <see cref="ILoggerProvider"/> implementation that creates returns instances of <see cref="ApplicationInsightsLogger"/>
Expand All @@ -11,20 +12,22 @@ internal class ApplicationInsightsLoggerProvider : ILoggerProvider
{
private readonly TelemetryClient telemetryClient;
private readonly Func<string, LogLevel, bool> filter;
private readonly ApplicationInsightsLoggerOptions options;

/// <summary>
/// Initializes a new instance of the <see cref="ApplicationInsightsLoggerProvider"/> class.
/// </summary>
public ApplicationInsightsLoggerProvider(TelemetryClient telemetryClient, Func<string, LogLevel, bool> filter)
public ApplicationInsightsLoggerProvider(TelemetryClient telemetryClient, Func<string, LogLevel, bool> filter, IOptions<ApplicationInsightsLoggerOptions> options)
{
this.telemetryClient = telemetryClient;
this.filter = filter;
this.options = options.Value;
}

/// <inheritdoc />
public ILogger CreateLogger(string categoryName)
{
return new ApplicationInsightsLogger(categoryName, this.telemetryClient, filter);
return new ApplicationInsightsLogger(categoryName, this.telemetryClient, filter, options);
}

/// <inheritdoc />
Expand Down
Loading