From 2f0e3d4c723b6496e3dad85c11155751f8e57cc4 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Fri, 16 Nov 2018 16:16:56 -0800 Subject: [PATCH 1/3] when Activity has root id compatible with W3C Id, use it as trace id --- Src/Common/W3C/W3CActivityExtensions.cs | 6 ++- .../ServiceBusDiagnosticListenerTests.cs | 33 ++++++++++++++++ .../W3C/W3CActiviityExtentionsTests.cs | 39 +++++++++++++++++++ .../AspNetDiagnosticTelemetryModuleTest.cs | 36 +++++++++++++++++ 4 files changed, 113 insertions(+), 1 deletion(-) diff --git a/Src/Common/W3C/W3CActivityExtensions.cs b/Src/Common/W3C/W3CActivityExtensions.cs index b7d2b4571..7fec0fc52 100644 --- a/Src/Common/W3C/W3CActivityExtensions.cs +++ b/Src/Common/W3C/W3CActivityExtensions.cs @@ -35,7 +35,11 @@ public static Activity GenerateW3CContext(this Activity activity) activity.SetVersion(W3CConstants.DefaultVersion); activity.SetSampled(W3CConstants.TraceFlagRecordedAndNotRequested); activity.SetSpanId(StringUtilities.GenerateSpanId()); - activity.SetTraceId(StringUtilities.GenerateTraceId()); + + activity.SetTraceId(activity.RootId != null && TraceIdRegex.IsMatch(activity.RootId) + ? activity.RootId + : StringUtilities.GenerateTraceId()); + return activity; } diff --git a/Src/DependencyCollector/Shared.Tests/Implementation/ServiceBusDiagnosticListenerTests.cs b/Src/DependencyCollector/Shared.Tests/Implementation/ServiceBusDiagnosticListenerTests.cs index 52b92bfd4..f1f73baac 100644 --- a/Src/DependencyCollector/Shared.Tests/Implementation/ServiceBusDiagnosticListenerTests.cs +++ b/Src/DependencyCollector/Shared.Tests/Implementation/ServiceBusDiagnosticListenerTests.cs @@ -11,6 +11,7 @@ using Microsoft.ApplicationInsights.DependencyCollector.Implementation; using Microsoft.ApplicationInsights.Extensibility; using Microsoft.ApplicationInsights.Extensibility.Implementation; + using Microsoft.ApplicationInsights.W3C; using Microsoft.ApplicationInsights.Web.TestFramework; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -218,6 +219,38 @@ public void ServiceBusProcessHandingWithoutParent() } } +#pragma warning disable 612, 618 + [TestMethod] + public void ServiceBusProcessHandingExternalParentW3CCompatibleRequestId() + { + this.configuration.TelemetryInitializers.Add(new W3COperationCorrelationTelemetryInitializer()); + using (var listener = new DiagnosticListener("Microsoft.Azure.ServiceBus")) + using (var module = new DependencyTrackingTelemetryModule()) + { + module.EnableW3CHeadersInjection = true; + module.IncludeDiagnosticSourceActivities.Add("Microsoft.Azure.ServiceBus"); + module.Initialize(this.configuration); + + var telemetry = this.TrackOperation( + listener, + "Microsoft.Azure.ServiceBus.Process", + TaskStatus.RanToCompletion, + "|4bf92f3577b34da6a3ce929d0e0e4736."); + + Assert.IsNotNull(telemetry); + Assert.AreEqual("Process", telemetry.Name); + Assert.AreEqual( + $"type:{RemoteDependencyConstants.AzureServiceBus} | name:queueName | endpoint:sb://queuename.myservicebus.com/", + telemetry.Source); + Assert.IsTrue(telemetry.Success.Value); + + Assert.AreEqual("|4bf92f3577b34da6a3ce929d0e0e4736.", telemetry.Context.Operation.ParentId); + Assert.AreEqual("4bf92f3577b34da6a3ce929d0e0e4736", telemetry.Context.Operation.Id); + Assert.AreEqual("messageId", telemetry.Properties["MessageId"]); + } + } +#pragma warning restore 612, 618 + [TestMethod] public void ServiceBusExceptionsAreIgnored() { diff --git a/Src/DependencyCollector/Shared.Tests/W3C/W3CActiviityExtentionsTests.cs b/Src/DependencyCollector/Shared.Tests/W3C/W3CActiviityExtentionsTests.cs index 93a9f745c..4b7c19799 100644 --- a/Src/DependencyCollector/Shared.Tests/W3C/W3CActiviityExtentionsTests.cs +++ b/Src/DependencyCollector/Shared.Tests/W3C/W3CActiviityExtentionsTests.cs @@ -156,6 +156,45 @@ public void UpdateContextWithoutParent() Assert.IsNull(a.GetTracestate()); } + [TestMethod] + public void UpdateContextFromCompatibleRootId() + { + var a = new Activity("foo"); + a.SetParentId(TraceId); + + Assert.IsFalse(a.IsW3CActivity()); + + a.UpdateContextOnActivity(); + Assert.IsTrue(a.IsW3CActivity()); + Assert.AreEqual(TraceId, a.GetTraceId()); + Assert.IsNotNull(a.GetSpanId()); + Assert.IsNull(a.GetParentSpanId()); + Assert.IsNotNull(a.GetSpanId()); + + Assert.AreEqual($"00-{a.GetTraceId()}-{a.GetSpanId()}-02", a.GetTraceparent()); + Assert.IsNull(a.GetTracestate()); + } + + [TestMethod] + public void UpdateContextFromIncompatibleRootId() + { + var a = new Activity("foo"); + a.SetParentId("abc"); + + Assert.IsFalse(a.IsW3CActivity()); + + a.UpdateContextOnActivity(); + Assert.IsTrue(a.IsW3CActivity()); + Assert.AreNotEqual("abc", a.GetTraceId()); + Assert.IsNotNull(a.GetTraceId()); + Assert.IsNotNull(a.GetSpanId()); + Assert.IsNull(a.GetParentSpanId()); + Assert.IsNotNull(a.GetSpanId()); + + Assert.AreEqual($"00-{a.GetTraceId()}-{a.GetSpanId()}-02", a.GetTraceparent()); + Assert.IsNull(a.GetTracestate()); + } + [TestMethod] public void UpdateContextWithParent() { diff --git a/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs b/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs index 8492b940e..97fd8891e 100644 --- a/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs +++ b/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs @@ -495,6 +495,42 @@ public void RequestIdBecomesParentWhenThereAreNoW3CHeaders() Assert.IsTrue(requestTelemetry.Properties[W3CConstants.LegacyRequestIdProperty].StartsWith("|requestId.")); } + [TestMethod] + public void RequestIdBecomesParentAndRootIfCompatibleWhenThereAreNoW3CHeaders() + { + this.aspNetDiagnosticsSource.FakeContext = + HttpModuleHelper.GetFakeHttpContext(new Dictionary + { + ["Request-Id"] = "|4bf92f3577b34da6a3ce929d0e0e4736." + }); + this.module = this.CreateModule(enableW3cSupport: true); + + var activity = new Activity(FakeAspNetDiagnosticSource.IncomingRequestEventName); + + activity.Extract(HttpContext.Current.Request.Headers); + + Assert.IsTrue(this.aspNetDiagnosticsSource.IsEnabled(FakeAspNetDiagnosticSource.IncomingRequestEventName, activity)); + this.aspNetDiagnosticsSource.StartActivityWithoutChecks(activity); + this.aspNetDiagnosticsSource.StopActivity(); + + Assert.AreEqual("4bf92f3577b34da6a3ce929d0e0e4736", activity.GetTraceId()); + Assert.AreEqual(16, activity.GetSpanId().Length); + Assert.IsNull(activity.GetParentSpanId()); + + Assert.AreEqual(1, this.sendItems.Count); + + var requestTelemetry = this.sendItems[0] as RequestTelemetry; + Assert.IsNotNull(requestTelemetry); + Assert.AreEqual(activity.GetTraceId(), requestTelemetry.Context.Operation.Id); + Assert.AreEqual("|4bf92f3577b34da6a3ce929d0e0e4736.", requestTelemetry.Context.Operation.ParentId); + Assert.AreEqual($"|{activity.GetTraceId()}.{activity.GetSpanId()}.", requestTelemetry.Id); + + Assert.IsFalse(requestTelemetry.Properties.ContainsKey(W3CConstants.LegacyRootIdProperty)); + + Assert.IsTrue(requestTelemetry.Properties.ContainsKey(W3CConstants.LegacyRequestIdProperty)); + Assert.IsTrue(requestTelemetry.Properties[W3CConstants.LegacyRequestIdProperty].StartsWith("|4bf92f3577b34da6a3ce929d0e0e4736.")); + } + [TestMethod] public void CustomHeadersBecomeParentWhenThereAreNoW3CHeaders() { From 3ca3c2fb8f5d3ae5b57ebe54f89f087a2c8c3c13 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Fri, 16 Nov 2018 16:42:55 -0800 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b72cef7b..2963dbc22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - [Fix: Sql dependency tracking broken in 2.8.0+. Dependency operation is not stopped and becomes parent of subsequent operations](https://github.com/Microsoft/ApplicationInsights-dotnet-server/issues/1090) - [Fix: Wrong parentId reported on the SqlClient dependency on .NET Core](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/778) - [Perf Fix - Replace TelemetryClient.Initialize() with TelemetryClient.InitializeInstrumentationKey() to avoid calling initializers more than once. ](https://github.com/Microsoft/ApplicationInsights-dotnet-server/issues/1094) +- [When Activity has root id compatible with W3C trace Id, use it as trace id](https://github.com/Microsoft/ApplicationInsights-dotnet-server/pull/1107) ## Version 2.8.0-beta2 - [LiveMetrics (QuickPulse) TelemetryProcessor added automatically to the default ApplicationInsights.config are moved under the default telemetry sink.](https://github.com/Microsoft/ApplicationInsights-dotnet-server/pull/987) diff --git a/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs b/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs index 97fd8891e..035cfea50 100644 --- a/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs +++ b/Src/Web/Web.Net45.Tests/AspNetDiagnosticTelemetryModuleTest.cs @@ -383,6 +383,7 @@ public void TestActivityIdGenerationWithW3CEnabled() Assert.AreEqual(32, request.Context.Operation.Id.Length); Assert.IsTrue(Regex.Match(request.Context.Operation.Id, @"[a-z][0-9]").Success); + Assert.AreEqual(request.Context.Operation.Id, activity.GetTraceId()); Assert.AreEqual(request.Context.Operation.Id, activity.RootId); Assert.AreEqual(request.Context.Operation.ParentId, activity.GetParentSpanId()); Assert.AreEqual(request.Id, $"|{activity.GetTraceId()}.{activity.GetSpanId()}."); From b3a0bc611be39968d5ebe0569e9a2e94c56ca375 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 29 Nov 2018 17:30:13 -0800 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2963dbc22..861ddc54e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## Version 2.9.0-beta3 +- [When Activity has root id compatible with W3C trace Id, use it as trace id](https://github.com/Microsoft/ApplicationInsights-dotnet-server/pull/1107) + ## Version 2.9.0-beta1 - [Prevent duplicate dependency collection in multi-host apps](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/621) - [Fix missing transactions Sql dependencies](https://github.com/Microsoft/ApplicationInsights-dotnet-server/pull/1031) @@ -9,7 +12,6 @@ - [Fix: Sql dependency tracking broken in 2.8.0+. Dependency operation is not stopped and becomes parent of subsequent operations](https://github.com/Microsoft/ApplicationInsights-dotnet-server/issues/1090) - [Fix: Wrong parentId reported on the SqlClient dependency on .NET Core](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/778) - [Perf Fix - Replace TelemetryClient.Initialize() with TelemetryClient.InitializeInstrumentationKey() to avoid calling initializers more than once. ](https://github.com/Microsoft/ApplicationInsights-dotnet-server/issues/1094) -- [When Activity has root id compatible with W3C trace Id, use it as trace id](https://github.com/Microsoft/ApplicationInsights-dotnet-server/pull/1107) ## Version 2.8.0-beta2 - [LiveMetrics (QuickPulse) TelemetryProcessor added automatically to the default ApplicationInsights.config are moved under the default telemetry sink.](https://github.com/Microsoft/ApplicationInsights-dotnet-server/pull/987)