Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ namespace Microsoft.ApplicationInsights
{
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -176,6 +177,54 @@ public void TrackEventWithNullNameHandlesGracefully()
// Note: This test verifies error handling
}

[Fact]
public void TrackEventDoesNotMutateReadOnlyPropertiesDictionary()
{
var inner = new Dictionary<string, string> { { "key1", "value1" } };
var readOnly = new ReadOnlyDictionary<string, string>(inner);
var originalCount = inner.Count;

this.telemetryClient.TrackEvent("TestEvent", readOnly);
this.telemetryClient.Flush();

// The original dictionary behind the read-only wrapper must not be modified
Assert.Equal(originalCount, inner.Count);
Assert.False(inner.ContainsKey("microsoft.custom_event.name"),
"Internal attribute should not leak into caller's dictionary");
}

[Fact]
public void TrackEventAcceptsReadOnlyDictionary()
{
var inner = new Dictionary<string, string> { { "key1", "value1" } };
var readOnly = new ReadOnlyDictionary<string, string>(inner);

// Must not throw NotSupportedException
this.telemetryClient.TrackEvent("TestEvent", readOnly);
this.telemetryClient.Flush();

var logRecord = this.logItems.FirstOrDefault(l =>
l.Attributes != null && l.Attributes.Any(a =>
a.Key == "microsoft.custom_event.name" && a.Value?.ToString() == "TestEvent"));
Assert.NotNull(logRecord);

// Verify user property is still present
Assert.True(logRecord.Attributes.Any(a => a.Key == "key1" && a.Value?.ToString() == "value1"));
}

[Fact]
public void TrackEventCanBeCalledTwiceWithSameDictionary()
{
var properties = new Dictionary<string, string> { { "key1", "value1" } };

// Both calls should succeed without ArgumentException from duplicate keys
this.telemetryClient.TrackEvent("Event1", properties);
this.telemetryClient.TrackEvent("Event2", properties);
this.telemetryClient.Flush();

Assert.True(this.logItems.Count >= 2, "Both events should be recorded");
}

#endregion

#region TrackAvailability
Expand Down Expand Up @@ -524,6 +573,19 @@ public void TrackTraceWithEmptyMessageInTraceTelemetry()
Assert.NotNull(logRecord);
}

[Fact]
public void TrackTraceAcceptsReadOnlyDictionary()
{
var inner = new Dictionary<string, string> { { "key1", "value1" } };
var readOnly = new ReadOnlyDictionary<string, string>(inner);

// Must not throw NotSupportedException
this.telemetryClient.TrackTrace("Test message", readOnly);
this.telemetryClient.Flush();

Assert.True(this.logItems.Count > 0, "Log should be collected");
}

#endregion

#region TrackMetric
Expand Down Expand Up @@ -1070,6 +1132,33 @@ public void TrackExceptionWithInnerExceptionPreservesInnerException()
Assert.Equal("Inner exception message", logRecord.Exception.InnerException.Message);
}

[Fact]
public void TrackExceptionDoesNotMutatePropertiesDictionary()
{
var properties = new Dictionary<string, string> { { "key1", "value1" } };
var originalCount = properties.Count;
var exception = new InvalidOperationException("Test");

this.telemetryClient.TrackException(exception, properties);
this.telemetryClient.Flush();

Assert.Equal(originalCount, properties.Count);
}

[Fact]
public void TrackExceptionAcceptsReadOnlyDictionary()
{
var inner = new Dictionary<string, string> { { "key1", "value1" } };
var readOnly = new ReadOnlyDictionary<string, string>(inner);
var exception = new InvalidOperationException("Test");

// Must not throw NotSupportedException
this.telemetryClient.TrackException(exception, readOnly);
this.telemetryClient.Flush();

Assert.True(this.logItems.Count > 0, "Log should be collected");
}

#endregion

#region TrackRequest
Expand Down
60 changes: 37 additions & 23 deletions BASE/src/Microsoft.ApplicationInsights/TelemetryClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,9 @@ public void TrackEvent(string eventName, IDictionary<string, string> properties
return;
}

if (properties == null)
{
properties = new Dictionary<string, string>();
}

properties.Add("microsoft.custom_event.name", eventName);
var state = new DictionaryLogState(this.Context, properties, String.Empty);
var mergedProperties = EnsureMutable(properties);
mergedProperties["microsoft.custom_event.name"] = eventName;
var state = new DictionaryLogState(this.Context, mergedProperties, String.Empty);
this.Logger.Log(LogLevel.Information, 0, state, null, (s, ex) => s.Message);
}

Expand All @@ -165,26 +161,26 @@ public void TrackEvent(EventTelemetry telemetry)
return;
}

var properties = telemetry.Properties ?? new Dictionary<string, string>();
properties.Add("microsoft.custom_event.name", telemetry.Name);
var mergedProperties = EnsureMutable(telemetry.Properties);
mergedProperties["microsoft.custom_event.name"] = telemetry.Name;

// Map context properties to semantic conventions
if (!string.IsNullOrEmpty(telemetry.Context?.Location?.Ip))
{
properties["microsoft.client.ip"] = telemetry.Context.Location.Ip;
mergedProperties["microsoft.client.ip"] = telemetry.Context.Location.Ip;
}

if (!string.IsNullOrEmpty(telemetry.Context?.User?.Id))
{
properties[SemanticConventions.AttributeEnduserPseudoId] = telemetry.Context.User.Id;
mergedProperties[SemanticConventions.AttributeEnduserPseudoId] = telemetry.Context.User.Id;
}

if (!string.IsNullOrEmpty(telemetry.Context?.User?.AuthenticatedUserId))
{
properties[SemanticConventions.AttributeEnduserId] = telemetry.Context.User.AuthenticatedUserId;
mergedProperties[SemanticConventions.AttributeEnduserId] = telemetry.Context.User.AuthenticatedUserId;
}

var state = new DictionaryLogState(telemetry.Context, properties, String.Empty);
var state = new DictionaryLogState(telemetry.Context, mergedProperties, String.Empty);
this.Logger.Log(LogLevel.Information, 0, state, null, (s, ex) => s.Message);
}

Expand Down Expand Up @@ -364,25 +360,25 @@ public void TrackTrace(TraceTelemetry telemetry)
}

// Map context properties to semantic conventions that exporter understands
var properties = telemetry.Properties ?? new Dictionary<string, string>();
var mergedProperties = EnsureMutable(telemetry.Properties);

if (!string.IsNullOrEmpty(telemetry.Context?.Location?.Ip))
{
properties["microsoft.client.ip"] = telemetry.Context.Location.Ip;
mergedProperties["microsoft.client.ip"] = telemetry.Context.Location.Ip;
}

if (!string.IsNullOrEmpty(telemetry.Context?.User?.Id))
{
properties[SemanticConventions.AttributeEnduserPseudoId] = telemetry.Context.User.Id;
mergedProperties[SemanticConventions.AttributeEnduserPseudoId] = telemetry.Context.User.Id;
}

if (!string.IsNullOrEmpty(telemetry.Context?.User?.AuthenticatedUserId))
{
properties[SemanticConventions.AttributeEnduserId] = telemetry.Context.User.AuthenticatedUserId;
mergedProperties[SemanticConventions.AttributeEnduserId] = telemetry.Context.User.AuthenticatedUserId;
}

LogLevel logLevel = GetLogLevel(telemetry.SeverityLevel.Value);
var state = new DictionaryLogState(telemetry.Context, properties, telemetry.Message);
var state = new DictionaryLogState(telemetry.Context, mergedProperties, telemetry.Message);
this.Logger.Log(logLevel, 0, state, null, (s, ex) => s.Message);
}

Expand Down Expand Up @@ -500,24 +496,24 @@ public void TrackException(ExceptionTelemetry telemetry)
var reconstructedException = ConvertToException(telemetry);

// Map context properties to semantic conventions
var properties = telemetry.Properties ?? new Dictionary<string, string>();
var mergedProperties = EnsureMutable(telemetry.Properties);

if (!string.IsNullOrEmpty(telemetry.Context?.Location?.Ip))
{
properties["microsoft.client.ip"] = telemetry.Context.Location.Ip;
mergedProperties["microsoft.client.ip"] = telemetry.Context.Location.Ip;
}

if (!string.IsNullOrEmpty(telemetry.Context?.User?.Id))
{
properties[SemanticConventions.AttributeEnduserPseudoId] = telemetry.Context.User.Id;
mergedProperties[SemanticConventions.AttributeEnduserPseudoId] = telemetry.Context.User.Id;
}

if (!string.IsNullOrEmpty(telemetry.Context?.User?.AuthenticatedUserId))
{
properties[SemanticConventions.AttributeEnduserId] = telemetry.Context.User.AuthenticatedUserId;
mergedProperties[SemanticConventions.AttributeEnduserId] = telemetry.Context.User.AuthenticatedUserId;
}

var state = new DictionaryLogState(telemetry.Context, properties, reconstructedException.Message);
var state = new DictionaryLogState(telemetry.Context, mergedProperties, reconstructedException.Message);
var logLevel = GetLogLevel(telemetry.SeverityLevel ?? SeverityLevel.Error);
this.Logger.Log(logLevel, 0, state, reconstructedException, (s, ex) => s.Message);
}
Expand Down Expand Up @@ -1374,6 +1370,24 @@ private static void EnrichExceptionWithMetadata(Exception exception, ExceptionTe
}
}

/// <summary>
/// Returns the dictionary as-is if it is mutable, or creates a mutable copy if it is read-only or null.
/// </summary>
private static IDictionary<string, string> EnsureMutable(IDictionary<string, string> properties)
{
if (properties == null)
{
return new Dictionary<string, string>();
}

if (properties.IsReadOnly)
{
return new Dictionary<string, string>(properties);
}

return properties;
}

/// <summary>
/// Applies CloudContext (RoleName/RoleInstance) and ComponentContext (Version) to the OpenTelemetry Resource if set.
/// </summary>
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

## Unreleased
- [Fix Track API calls to not mutate the passed in dictionary if it is readonly](https://github.com/microsoft/ApplicationInsights-dotnet/pull/3119)

## Version 3.0.0
- [Replaced `netstandard2.0` with `net8.0` target framework in `Microsoft.ApplicationInsights.NLogTarget` package.](https://github.com/microsoft/ApplicationInsights-dotnet/pull/3102)

Expand Down
Loading