Skip to content

Commit d5ffab2

Browse files
Add fix for persistent Process.Start version-conflict issue (#7789)
## Summary of changes - Adds a workaround for the version-conflict issue that occurs on app startup on Linux - Allow doing call target modification of version conflict dll - Fix a bug in CallTarget instrumentation where we get the assembly reference wrong ## Reason for change As part of app startup, on _some_ linux distros, we shell out to `stat` to build the container tags/entity ID. We don't want to trace this `Process.Start()` call, so in #5280 we added a flag to skip instrumenting these calls. However, this fix relies on a `[ThreadStatic]` variable, and in version-conflict scenarios (2.x.x manual, 3.x.x automatic) we end up still instrumenting this call, which causes recursion in `Tracer` initialization and [errors](https://app.datadoghq.com/error-tracking?query=service%3Ainstrumentation-telemetry-data%20source%3Adotnet%20%40tracer_version%3A3.29.0.0%20-%40error.is_crash%3Atrue&et-side=activity&order=total_count&refresh_mode=sliding&source=all&sp=%5B%7B%22p%22%3A%7B%22issueId%22%3A%22cbce5fc2-3adf-11f0-a4be-da7ad0900002%22%7D%2C%22i%22%3A%22error-tracking-issue%22%7D%5D&view=spans&from_ts=1761134402500&to_ts=1762344002500&live=true). > Note that since #7453 we don't do a process start at all, but that doesn't help in this situation, because it's the 2.x.x library that's doing the `Process.Start()` ## Implementation details - Use standard call target instrumentation on the 2.x.x version of `Datadog.Trace` (i.e. version conflict only) - Hook the `ProcessHelpers.StartWithDoNotTrace()` method, and set the 3.x.x `_doNotTrace` variable for the duration of the method call - Tweak the Rejit handler so that we _do_ rejit/call target the Datadog.Trace 2.x.x module (but not the 3.x.x module) - Fix a bug in the module builder which was incorrectly injecting a reference to the 2.x.x assembly instead of the 3.x.x assembly ## Test coverage We were already working around this issue in our VersionConflict tests, so I removed the workaround, confirmed that the test failed, then made the fix, and confirmed the tests pass again. ## Other details This will only help for customers using a manual version of 2.49.0+ (when we introduced the `StartWithDoNotTrace()` call). I think that's good enough support. --------- Co-authored-by: Lucas Pimentel <[email protected]>
1 parent 3ae0571 commit d5ffab2

File tree

31 files changed

+1615
-1452
lines changed

31 files changed

+1615
-1452
lines changed

tracer/build/_build/Honeypot/IntegrationGroups.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ static IntegrationMap()
125125
NugetPackages.Add("RestSharp", Array.Empty<string>());
126126

127127
// Manual instrumentation
128+
NugetPackages.Add("Datadog.Trace", new string[] { });
128129
NugetPackages.Add("Datadog.Trace.Manual", new string[] { });
129130
NugetPackages.Add("Datadog.Trace.OpenTracing", new string[] { });
130131
}

tracer/build/supported_calltargets.g.json

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7197,6 +7197,31 @@
71977197
"IsAdoNetIntegration": false,
71987198
"InstrumentationCategory": 1
71997199
},
7200+
{
7201+
"IntegrationName": "DatadogTraceVersionConflict",
7202+
"AssemblyName": "Datadog.Trace",
7203+
"TargetTypeName": "Datadog.Trace.Util.ProcessHelpers",
7204+
"TargetMethodName": "StartWithDoNotTrace",
7205+
"TargetReturnType": "System.Diagnostics.Process",
7206+
"TargetParameterTypes": [
7207+
"System.Diagnostics.ProcessStartInfo",
7208+
"System.Boolean"
7209+
],
7210+
"MinimumVersion": {
7211+
"Item1": 2,
7212+
"Item2": 49,
7213+
"Item3": 0
7214+
},
7215+
"MaximumVersion": {
7216+
"Item1": 2,
7217+
"Item2": 65535,
7218+
"Item3": 65535
7219+
},
7220+
"InstrumentationTypeName": "Datadog.Trace.ClrProfiler.AutoInstrumentation.VersionConflict.ProcessHelpersStartWithDoNotTraceIntegration",
7221+
"IntegrationKind": 0,
7222+
"IsAdoNetIntegration": false,
7223+
"InstrumentationCategory": 1
7224+
},
72007225
{
72017226
"IntegrationName": "DotnetTest",
72027227
"AssemblyName": "coverlet.core",
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// <copyright file="ProcessHelpersStartWithDoNotTraceIntegration.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
#nullable enable
7+
8+
using System;
9+
using System.ComponentModel;
10+
using Datadog.Trace.ClrProfiler.CallTarget;
11+
using Datadog.Trace.Configuration;
12+
using Datadog.Trace.Util;
13+
14+
namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.VersionConflict;
15+
16+
/// <summary>
17+
/// System.Diagnostics.Process Datadog.Trace.Util.ProcessHelpers::StartWithDoNotTrace(System.Diagnostics.ProcessStartInfo,System.Boolean) calltarget instrumentation
18+
/// </summary>
19+
[InstrumentMethod(
20+
AssemblyName = "Datadog.Trace",
21+
TypeName = "Datadog.Trace.Util.ProcessHelpers",
22+
MethodName = "StartWithDoNotTrace",
23+
ReturnTypeName = ClrNames.Process,
24+
ParameterTypeNames = ["System.Diagnostics.ProcessStartInfo", ClrNames.Bool],
25+
MinimumVersion = "2.49.0", // We only introduced ProcessHelpers.StartWithDoNotTrace() in 2.49.0
26+
MaximumVersion = "2.*.*",
27+
IntegrationName = nameof(IntegrationId.DatadogTraceVersionConflict))]
28+
[Browsable(false)]
29+
[EditorBrowsable(EditorBrowsableState.Never)]
30+
public class ProcessHelpersStartWithDoNotTraceIntegration
31+
{
32+
internal static CallTargetState OnMethodBegin<TTarget, TStartInfo>(ref TStartInfo? startInfo, ref bool doNotTrace)
33+
{
34+
if (doNotTrace)
35+
{
36+
// When we run in a version conflict scenario (with a 2.x Datadog.Trace version of manual instrumentation
37+
// and a 3.x version of auto-instrumentation), we will incorrectly trace Process.Start() calls
38+
// that the 2.x lib is trying to avoid tracing by setting a [ThreadStatic] bool _doNotTrace on ProcessHelpers.
39+
// However, because the types aren't shared between 2.x and 3.x, and because it's the 3.x types that are
40+
// doing the tracing, this value is not respected. To work around that, we instrument the 2.x libraries
41+
// and set the 3.x version, mirroring the desired behaviour.
42+
ProcessHelpers.ForceDoNotTrace(true);
43+
}
44+
45+
return CallTargetState.GetDefault();
46+
}
47+
48+
internal static CallTargetReturn<System.Diagnostics.Process?> OnMethodEnd<TTarget>(System.Diagnostics.Process? returnValue, Exception? exception, in CallTargetState state)
49+
{
50+
// Always reset this back to false
51+
ProcessHelpers.ForceDoNotTrace(false);
52+
return new CallTargetReturn<System.Diagnostics.Process?>(returnValue);
53+
}
54+
}

tracer/src/Datadog.Trace/Configuration/IntegrationId.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ internal enum IntegrationId
8787
DatadogTraceManual,
8888
EmailHtmlInjection,
8989
Protobuf,
90-
AzureEventHubs
90+
AzureEventHubs,
91+
DatadogTraceVersionConflict,
9192
}
9293
}

tracer/src/Datadog.Trace/Generated/net461/Datadog.Trace.SourceGenerators/EnumExtensionsGenerator/IntegrationIdExtensions_EnumExtensions.g.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ internal static partial class IntegrationIdExtensions
1717
/// The number of members in the enum.
1818
/// This is a non-distinct count of defined names.
1919
/// </summary>
20-
public const int Length = 76;
20+
public const int Length = 77;
2121

2222
/// <summary>
2323
/// Returns the string representation of the <see cref="Datadog.Trace.Configuration.IntegrationId"/> value.
@@ -106,6 +106,7 @@ public static string ToStringFast(this Datadog.Trace.Configuration.IntegrationId
106106
Datadog.Trace.Configuration.IntegrationId.EmailHtmlInjection => nameof(Datadog.Trace.Configuration.IntegrationId.EmailHtmlInjection),
107107
Datadog.Trace.Configuration.IntegrationId.Protobuf => nameof(Datadog.Trace.Configuration.IntegrationId.Protobuf),
108108
Datadog.Trace.Configuration.IntegrationId.AzureEventHubs => nameof(Datadog.Trace.Configuration.IntegrationId.AzureEventHubs),
109+
Datadog.Trace.Configuration.IntegrationId.DatadogTraceVersionConflict => nameof(Datadog.Trace.Configuration.IntegrationId.DatadogTraceVersionConflict),
109110
_ => value.ToString(),
110111
};
111112

@@ -195,6 +196,7 @@ public static Datadog.Trace.Configuration.IntegrationId[] GetValues()
195196
Datadog.Trace.Configuration.IntegrationId.EmailHtmlInjection,
196197
Datadog.Trace.Configuration.IntegrationId.Protobuf,
197198
Datadog.Trace.Configuration.IntegrationId.AzureEventHubs,
199+
Datadog.Trace.Configuration.IntegrationId.DatadogTraceVersionConflict,
198200
};
199201

200202
/// <summary>
@@ -284,5 +286,6 @@ public static string[] GetNames()
284286
nameof(Datadog.Trace.Configuration.IntegrationId.EmailHtmlInjection),
285287
nameof(Datadog.Trace.Configuration.IntegrationId.Protobuf),
286288
nameof(Datadog.Trace.Configuration.IntegrationId.AzureEventHubs),
289+
nameof(Datadog.Trace.Configuration.IntegrationId.DatadogTraceVersionConflict),
287290
};
288291
}

tracer/src/Datadog.Trace/Generated/net461/Datadog.Trace.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,8 @@ internal static bool IsInstrumentedAssembly(string assemblyName)
327327
or "Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.OpenTracing.OpenTracingTracerFactoryCreateTracerIntegration"
328328
or "Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.OpenTracing.OpenTracingTracerFactoryWrapTracerIntegration"
329329
=> Datadog.Trace.Configuration.IntegrationId.DatadogTraceManual,
330+
"Datadog.Trace.ClrProfiler.AutoInstrumentation.VersionConflict.ProcessHelpersStartWithDoNotTraceIntegration"
331+
=> Datadog.Trace.Configuration.IntegrationId.DatadogTraceVersionConflict,
330332
"Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.DotnetTest.CoverageGetCoverageResultIntegration"
331333
or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.DotnetTest.TestCommand5ctorIntegration"
332334
or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.DotnetTest.TestCommandctorIntegration"

tracer/src/Datadog.Trace/Generated/net461/Datadog.Trace.SourceGenerators/TelemetryMetricGenerator/CiVisibilityMetricsTelemetryCollector_CountShared.g.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
namespace Datadog.Trace.Telemetry;
1212
internal partial class CiVisibilityMetricsTelemetryCollector
1313
{
14-
private const int CountSharedLength = 324;
14+
private const int CountSharedLength = 328;
1515

1616
/// <summary>
1717
/// Creates the buffer for the <see cref="Datadog.Trace.Telemetry.Metrics.CountShared" /> values.
@@ -28,6 +28,10 @@ private static AggregatedMetric[] GetCountSharedBuffer()
2828
new(new[] { "integration_name:opentracing", "error_type:invoker" }),
2929
new(new[] { "integration_name:opentracing", "error_type:execution" }),
3030
new(new[] { "integration_name:opentracing", "error_type:missing_member" }),
31+
new(new[] { "integration_name:version_conflict", "error_type:duck_typing" }),
32+
new(new[] { "integration_name:version_conflict", "error_type:invoker" }),
33+
new(new[] { "integration_name:version_conflict", "error_type:execution" }),
34+
new(new[] { "integration_name:version_conflict", "error_type:missing_member" }),
3135
new(new[] { "integration_name:ciapp", "error_type:duck_typing" }),
3236
new(new[] { "integration_name:ciapp", "error_type:invoker" }),
3337
new(new[] { "integration_name:ciapp", "error_type:execution" }),
@@ -352,7 +356,7 @@ private static AggregatedMetric[] GetCountSharedBuffer()
352356
/// It is equal to the cardinality of the tag combinations (or 1 if there are no tags)
353357
/// </summary>
354358
private static int[] CountSharedEntryCounts { get; }
355-
= new int[]{ 324, };
359+
= new int[]{ 328, };
356360

357361
public void RecordCountSharedIntegrationsError(Datadog.Trace.Telemetry.Metrics.MetricTags.IntegrationName tag1, Datadog.Trace.Telemetry.Metrics.MetricTags.InstrumentationError tag2, int increment = 1)
358362
{

0 commit comments

Comments
 (0)