diff --git a/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/TelemetryClientTest.cs b/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/TelemetryClientTest.cs index 145bd19f8..bb67efc9a 100644 --- a/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/TelemetryClientTest.cs +++ b/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/TelemetryClientTest.cs @@ -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; @@ -176,6 +177,54 @@ public void TrackEventWithNullNameHandlesGracefully() // Note: This test verifies error handling } + [Fact] + public void TrackEventDoesNotMutateReadOnlyPropertiesDictionary() + { + var inner = new Dictionary { { "key1", "value1" } }; + var readOnly = new ReadOnlyDictionary(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 { { "key1", "value1" } }; + var readOnly = new ReadOnlyDictionary(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 { { "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 @@ -524,6 +573,19 @@ public void TrackTraceWithEmptyMessageInTraceTelemetry() Assert.NotNull(logRecord); } + [Fact] + public void TrackTraceAcceptsReadOnlyDictionary() + { + var inner = new Dictionary { { "key1", "value1" } }; + var readOnly = new ReadOnlyDictionary(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 @@ -1070,6 +1132,33 @@ public void TrackExceptionWithInnerExceptionPreservesInnerException() Assert.Equal("Inner exception message", logRecord.Exception.InnerException.Message); } + [Fact] + public void TrackExceptionDoesNotMutatePropertiesDictionary() + { + var properties = new Dictionary { { "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 { { "key1", "value1" } }; + var readOnly = new ReadOnlyDictionary(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 diff --git a/BASE/src/Microsoft.ApplicationInsights/TelemetryClient.cs b/BASE/src/Microsoft.ApplicationInsights/TelemetryClient.cs index bed95fb55..1604a1164 100644 --- a/BASE/src/Microsoft.ApplicationInsights/TelemetryClient.cs +++ b/BASE/src/Microsoft.ApplicationInsights/TelemetryClient.cs @@ -138,13 +138,9 @@ public void TrackEvent(string eventName, IDictionary properties return; } - if (properties == null) - { - properties = new Dictionary(); - } - - 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); } @@ -165,26 +161,26 @@ public void TrackEvent(EventTelemetry telemetry) return; } - var properties = telemetry.Properties ?? new Dictionary(); - 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); } @@ -364,25 +360,25 @@ public void TrackTrace(TraceTelemetry telemetry) } // Map context properties to semantic conventions that exporter understands - var properties = telemetry.Properties ?? new Dictionary(); + 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); } @@ -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(); + 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); } @@ -1374,6 +1370,24 @@ private static void EnrichExceptionWithMetadata(Exception exception, ExceptionTe } } + /// + /// Returns the dictionary as-is if it is mutable, or creates a mutable copy if it is read-only or null. + /// + private static IDictionary EnsureMutable(IDictionary properties) + { + if (properties == null) + { + return new Dictionary(); + } + + if (properties.IsReadOnly) + { + return new Dictionary(properties); + } + + return properties; + } + /// /// Applies CloudContext (RoleName/RoleInstance) and ComponentContext (Version) to the OpenTelemetry Resource if set. /// diff --git a/CHANGELOG.md b/CHANGELOG.md index 0512a9925..c4c4a4937 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)