Leverage W3C implementation from .NET in requests and depedencies collectors #1252
Conversation
Src/DependencyCollector/Shared/Implementation/ClientServerDependencyTracker.cs
Outdated
Show resolved
Hide resolved
| break; | ||
|
|
||
| case "http": | ||
| title = "Made Sync GET HTTP call to bing"; |
There was a problem hiding this comment.
Technically this adds a dependency of out tests reliability on Bing's availability, thus lowering ( :) ) our pass rate in the long run. No op, though, our pass rate is less than Bing's availability :)
There was a problem hiding this comment.
since this is a copy-paste of other func tests, I'll leave it like that. Bing availability is relatively good compared to our tests stability ;)
lmolkova
force-pushed
the
lmolkova/w3c
branch
from
0aa37dc
to
fac3f30
Compare
Src/DependencyCollector/NetCore.Tests/DependencyTrackingTelemetryModuleTestNetCore.cs
Show resolved
Hide resolved
| [TestMethod] | ||
| public void NetCore30_OnActivityStartInjectsHeaders() | ||
| { | ||
| var activity = new Activity("System.Net.Http.HttpRequestOut"); |
There was a problem hiding this comment.
would be nice to move the name of activity to const.
| @@ -62,8 +60,14 @@ public class DependencyTrackingTelemetryModule : ITelemetryModule, IDisposable | |||
| /// <summary> | |||
| /// Gets or sets a value indicating whether to enable W3C distributed tracing headers injection. | |||
| /// </summary> | |||
| [Obsolete("This field has been deprecated. Please set TelemetryConfiguration.EnableW3CCorrelation.")] | |||
There was a problem hiding this comment.
Instead of TelemetryConfiguration.EnableW3CCorrelation, do
Activity.DefaultIdFormat = ActivityIdFormat.W3C;
Activity.ForceDefaultIdFormat = true;
| if (currentActivity.ParentSpanId != default) | ||
| { | ||
| telemetry.Context.Operation.ParentId = string.Concat('|', traceId, '.', | ||
| currentActivity.ParentSpanId.ToHexString(), '.'); |
There was a problem hiding this comment.
a private helper method to do ID generation from trace/span ids,
| if (!requestHeaders.Contains(RequestResponseHeaders.RequestIdHeader)) | ||
| { | ||
| requestHeaders.Add(RequestResponseHeaders.RequestIdHeader, | ||
| string.Concat('|', currentActivity.TraceId.ToHexString(), '.', currentActivity.SpanId.ToHexString(), '.')); |
There was a problem hiding this comment.
helper method to construct requestid
| [TestMethod] | ||
| public void OnActivityStartInjectsW3COff() | ||
| { | ||
| this.configuration.EnableW3CCorrelation = false; |
There was a problem hiding this comment.
disable w3c using Activity static fields.
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that activity without parent id does not get a new W3C compatible root id. |
| activity.UpdateTelemetry(telemetry, true); | ||
| if (activity.TraceStateString != null && !telemetry.Properties.ContainsKey(W3CConstants.TracestatePropertyKey)) | ||
| { | ||
| telemetry.Properties.Add(W3CConstants.TracestatePropertyKey, activity.TraceStateString); |
There was a problem hiding this comment.
lets do this in base sdk.
There was a problem hiding this comment.
|
|
||
|
|
||
| // TODO move to base SDK? | ||
| if (!string.IsNullOrEmpty(currentActivity.TraceStateString) && !telemetry.Properties.ContainsKey(W3CConstants.TracestatePropertyKey)) |
There was a problem hiding this comment.
reminder to remove this logic from web as base does this once microsoft/ApplicationInsights-dotnet#1207 merged
Activity.Currentsetter, there is no need anymore