Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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,69 @@ public void TrackEventWithNullNameHandlesGracefully()
// Note: This test verifies error handling
}

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

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

// The caller's dictionary must not be modified
Assert.Equal(originalCount, properties.Count);
Assert.False(properties.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");
}

[Fact]
public void TrackEventWithEventTelemetryDoesNotMutateProperties()
{
var eventTelemetry = new EventTelemetry("TestEvent");
eventTelemetry.Properties["userProp"] = "userValue";
var originalCount = eventTelemetry.Properties.Count;

this.telemetryClient.TrackEvent(eventTelemetry);
this.telemetryClient.Flush();

// The telemetry object's Properties must not be modified
Assert.Equal(originalCount, eventTelemetry.Properties.Count);
Assert.False(eventTelemetry.Properties.ContainsKey("microsoft.custom_event.name"),
"Internal attribute should not leak into telemetry's Properties");
}

#endregion

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

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

this.telemetryClient.TrackTrace("Test message", properties);
this.telemetryClient.Flush();

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

[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");
}

[Fact]
public void TrackTraceWithTraceTelemetryDoesNotMutateProperties()
{
var traceTelemetry = new TraceTelemetry("Test message");
traceTelemetry.Properties["userProp"] = "userValue";
var originalCount = traceTelemetry.Properties.Count;

this.telemetryClient.TrackTrace(traceTelemetry);
this.telemetryClient.Flush();

Assert.Equal(originalCount, traceTelemetry.Properties.Count);
Assert.False(traceTelemetry.Properties.ContainsKey("microsoft.client.ip"),
"Internal attributes should not leak into telemetry's Properties");
}

#endregion

#region TrackMetric
Expand Down Expand Up @@ -1070,6 +1174,48 @@ 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");
}

[Fact]
public void TrackExceptionWithExceptionTelemetryDoesNotMutateProperties()
{
var telemetry = new ExceptionTelemetry(new InvalidOperationException("Test"));
telemetry.Properties["userProp"] = "userValue";
var originalCount = telemetry.Properties.Count;

this.telemetryClient.TrackException(telemetry);
this.telemetryClient.Flush();

Assert.Equal(originalCount, telemetry.Properties.Count);
Assert.False(telemetry.Properties.ContainsKey("microsoft.client.ip"),
"Internal attributes should not leak into telemetry's Properties");
}

#endregion

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

if (properties == null)
var mergedProperties = new Dictionary<string, string>();
if (properties != null)
{
properties = new Dictionary<string, string>();
foreach (var kvp in properties)
{
mergedProperties[kvp.Key] = kvp.Value;
}
}

properties.Add("microsoft.custom_event.name", eventName);
var state = new DictionaryLogState(this.Context, properties, String.Empty);
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 +169,34 @@ public void TrackEvent(EventTelemetry telemetry)
return;
}

var properties = telemetry.Properties ?? new Dictionary<string, string>();
properties.Add("microsoft.custom_event.name", telemetry.Name);
var mergedProperties = new Dictionary<string, string>();
if (telemetry.Properties != null)
{
foreach (var kvp in telemetry.Properties)
{
mergedProperties[kvp.Key] = kvp.Value;
}
}

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 +376,32 @@ public void TrackTrace(TraceTelemetry telemetry)
}

// Map context properties to semantic conventions that exporter understands
var properties = telemetry.Properties ?? new Dictionary<string, string>();
var mergedProperties = new Dictionary<string, string>();
if (telemetry.Properties != null)
{
foreach (var kvp in telemetry.Properties)
{
mergedProperties[kvp.Key] = kvp.Value;
}
}

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 +519,31 @@ 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 = new Dictionary<string, string>();
if (telemetry.Properties != null)
{
foreach (var kvp in telemetry.Properties)
{
mergedProperties[kvp.Key] = kvp.Value;
}
}

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
6 changes: 6 additions & 0 deletions BreakingChanges.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ The following packages are part of the shimmed 3.x release:
- **2.x**: `TrackEvent(string eventName, IDictionary<string, string> properties, IDictionary<string, double> metrics)`
- **3.x**: `TrackEvent(string eventName, IDictionary<string, string> properties)` **Metrics parameter removed**

### Fixed: `TrackEvent` No Longer Mutates the `properties` Dictionary
Comment thread
harsimar marked this conversation as resolved.
Outdated

In earlier 3.x pre-releases, `TrackEvent(string eventName, IDictionary<string, string> properties)` mutated the caller's `properties` dictionary by calling `properties.Add("microsoft.custom_event.name", eventName)`. This caused `System.NotSupportedException` when passing immutable `IDictionary` implementations (such as F#'s `dict`/`Map`, `ReadOnlyDictionary<TKey, TValue>`, or `ImmutableDictionary<TKey, TValue>`), and could also cause `ArgumentException` on repeated calls with the same dictionary instance due to duplicate keys.

This has been fixed. All `Track*` methods (`TrackEvent`, `TrackTrace`, `TrackException`, etc.) now create an internal copy of the properties dictionary before adding internal attributes. The caller's dictionary is never modified. Immutable and read-only dictionary implementations are fully supported.

#### TrackAvailability
- **2.x**: `TrackAvailability(string name, DateTimeOffset timeStamp, TimeSpan duration, string runLocation, bool success, string message, IDictionary<string, string> properties, IDictionary<string, double> metrics)`
- **3.x**: `TrackAvailability(string name, DateTimeOffset timeStamp, TimeSpan duration, string runLocation, bool success, string message, IDictionary<string, string> properties)` **Metrics parameter removed**
Expand Down
Loading