From 5db3f5b1bd11714b987b7303d862f917b1a7bea8 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Fri, 10 Feb 2023 16:31:41 -0800 Subject: [PATCH] [AzureMonitorExporter] Fix how arrays stored in Activity Tags are serialized. (#34086) * fix array tostring * cleanup --- .../src/Internals/ArrayExtensions.cs | 47 ++++++++++++++++ .../src/Internals/TagEnumerationState.cs | 18 +----- .../src/Internals/TraceHelper.cs | 9 ++- .../TelemetryItemValidationHelper.cs | 17 ++++-- .../E2ETelemetryItemValidation/LogsTests.cs | 4 +- .../E2ETelemetryItemValidation/TracesTests.cs | 56 ++++++++++++++++++- 6 files changed, 125 insertions(+), 26 deletions(-) create mode 100644 sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/ArrayExtensions.cs diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/ArrayExtensions.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/ArrayExtensions.cs new file mode 100644 index 0000000000000..d931822b2e297 --- /dev/null +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/ArrayExtensions.cs @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Text; + +namespace Azure.Monitor.OpenTelemetry.Exporter.Internals +{ + internal static class ArrayExtensions + { + /// + /// Builds a comma delimited string of the components of an array. + /// + /// + /// For example: new int[] { 1, 2, 3 } would be returned as "1,2,3". + /// + /// An array to be evaluated. + /// A comma delimited string of the components of the input array. + [return: System.Diagnostics.CodeAnalysis.NotNullIfNotNull(nameof(input))] + public static string? ToCommaDelimitedString(this Array? input) + { + if (input == null) + { + return null; + } + + StringBuilder sb = new(input.Length); + foreach (var item in input) + { + // TODO: Consider changing it to JSon array. + if (item != null) + { + sb.Append(item); + sb.Append(','); + } + } + + // remove trailing comma + if (sb.Length > 0) + { + sb.Length--; + } + + return sb.ToString(); + } + } +} diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TagEnumerationState.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TagEnumerationState.cs index b7719db570dd8..4949403de8ab1 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TagEnumerationState.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TagEnumerationState.cs @@ -80,23 +80,7 @@ public void ForEach(IEnumerable> activityTags) if (activityTag.Value is Array array) { - StringBuilder sw = new StringBuilder(); - foreach (var item in array) - { - // TODO: Consider changing it to JSon array. - if (item != null) - { - sw.Append(item); - sw.Append(','); - } - } - - if (sw.Length > 0) - { - sw.Length--; - } - - AzMonList.Add(ref UnMappedTags, new KeyValuePair(activityTag.Key, sw.ToString())); + AzMonList.Add(ref UnMappedTags, new KeyValuePair(activityTag.Key, array.ToCommaDelimitedString())); continue; } diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TraceHelper.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TraceHelper.cs index 83fe4c7f14429..7215c57fa1403 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TraceHelper.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/TraceHelper.cs @@ -222,7 +222,14 @@ private static MonitorBase GetTraceTelemetryData(ActivityEvent activityEvent) foreach (var tag in activityEvent.Tags) { - messageData.Properties.Add(tag.Key, tag.Value.ToString()); + if (tag.Value is Array arrayValue) + { + messageData.Properties.Add(tag.Key, arrayValue.ToCommaDelimitedString()); + } + else + { + messageData.Properties.Add(tag.Key, tag.Value.ToString()); + } } return new MonitorBase diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/CommonTestFramework/TelemetryItemValidationHelper.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/CommonTestFramework/TelemetryItemValidationHelper.cs index 7050eeaedb057..9f851a52022d6 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/CommonTestFramework/TelemetryItemValidationHelper.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/CommonTestFramework/TelemetryItemValidationHelper.cs @@ -14,11 +14,11 @@ namespace Azure.Monitor.OpenTelemetry.Exporter.Tests.CommonTestFramework { internal static class TelemetryItemValidationHelper { - public static void AssertLog_As_MessageTelemetry( + public static void AssertMessageTelemetry( TelemetryItem telemetryItem, string expectedSeverityLevel, string expectedMessage, - IDictionary expectedMeessageProperties, + IDictionary expectedMessageProperties, string expectedSpanId, string expectedTraceId) { @@ -43,10 +43,19 @@ public static void AssertLog_As_MessageTelemetry( Assert.Contains("ai.internal.sdkVersion", telemetryItem.Tags.Keys); var messageData = (MessageData)telemetryItem.Data.BaseData; - Assert.Equal(expectedSeverityLevel, messageData.SeverityLevel); + + if (expectedSeverityLevel == null) + { + Assert.Null(messageData.SeverityLevel); + } + else + { + Assert.Equal(expectedSeverityLevel, messageData.SeverityLevel); + } + Assert.Equal(expectedMessage, messageData.Message); - foreach (var prop in expectedMeessageProperties) + foreach (var prop in expectedMessageProperties) { Assert.Equal(prop.Value, messageData.Properties[prop.Key]); } diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/LogsTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/LogsTests.cs index 2ce2952be84b5..453b6b7fb9d9a 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/LogsTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/LogsTests.cs @@ -72,11 +72,11 @@ public void VerifyLog(LogLevel logLevel, string expectedSeverityLevel) this.telemetryOutput.Write(telemetryItems); var telemetryItem = telemetryItems.Single(); - TelemetryItemValidationHelper.AssertLog_As_MessageTelemetry( + TelemetryItemValidationHelper.AssertMessageTelemetry( telemetryItem: telemetryItem, expectedSeverityLevel: expectedSeverityLevel, expectedMessage: "Hello {name}.", - expectedMeessageProperties: new Dictionary { { "name", "World" }}, + expectedMessageProperties: new Dictionary { { "name", "World" }}, expectedSpanId: null, expectedTraceId: null); } diff --git a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/TracesTests.cs b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/TracesTests.cs index 9f5cf6776c25f..ff016963662d4 100644 --- a/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/TracesTests.cs +++ b/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/E2ETelemetryItemValidation/TracesTests.cs @@ -256,11 +256,63 @@ public void VerifyLogWithinActivity(LogLevel logLevel, string expectedSeverityLe this.telemetryOutput.Write(logTelemetryItems); var logTelemetryItem = logTelemetryItems.Single(); - TelemetryItemValidationHelper.AssertLog_As_MessageTelemetry( + TelemetryItemValidationHelper.AssertMessageTelemetry( telemetryItem: logTelemetryItem, expectedSeverityLevel: expectedSeverityLevel, expectedMessage: "Hello {name}.", - expectedMeessageProperties: new Dictionary { { "name", "World" } }, + expectedMessageProperties: new Dictionary { { "name", "World" } }, + expectedSpanId: spanId, + expectedTraceId: traceId); + } + + [Fact] + public void TestActivityEvents() + { + // SETUP + var uniqueTestId = Guid.NewGuid(); + + var activitySourceName = $"activitySourceName{uniqueTestId}"; + using var activitySource = new ActivitySource(activitySourceName); + + var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddSource(activitySourceName) + .AddAzureMonitorTraceExporterForTest(out ConcurrentBag telemetryItems) + .Build(); + + // ACT + string spanId = null, traceId = null; + + using (var activity = activitySource.StartActivity(name: "ActivityWithException")) + { + traceId = activity.TraceId.ToHexString(); + spanId = activity.SpanId.ToHexString(); + + var eventTags = new Dictionary + { + { "integer", 1 }, + { "string", "Hello, World!" }, + { "intArray", new int[] { 1, 2, 3 } } + }; + activity?.AddEvent(new("Gonna try it!", DateTimeOffset.Now, new(eventTags))); + } + + // CLEANUP + tracerProvider.Dispose(); + + // ASSERT + Assert.True(telemetryItems.Any(), "Unit test failed to collect telemetry."); + this.telemetryOutput.Write(telemetryItems); + + var messageTelemetryItem = telemetryItems.First(x => x.Name == "Message"); + + TelemetryItemValidationHelper.AssertMessageTelemetry( + telemetryItem: messageTelemetryItem, + expectedSeverityLevel: null, + expectedMessage: "Gonna try it!", + expectedMessageProperties: new Dictionary { + { "integer", "1" }, + { "string", "Hello, World!" }, + { "intArray", "1,2,3" } }, expectedSpanId: spanId, expectedTraceId: traceId); }