Skip to content

Commit 85673e7

Browse files
authored
Stop using Tracer.Instance if we don't need to (#7744)
## Summary of changes Updates a couple of places where we're calling `Tracer.Instance` where we don't need to ## Reason for change In one of my other PRs I accidentally broke something that should _only_ have affected integration tests, but a bunch of unit tests broke. It highlighted where they were using `Tracer.Instance` and setting the global tracer for tests. In other places, it revealed that code that tests that _looked_ independent of other tests really wasn't... Calling `Tracer.Instance` potentially does a _lot_ of work, as it initializes the tracer. It's also hard to follow if we're making static calls out in places we don't need to. So just pass through the settings we want instead. ## Implementation details Instead of calling `Tracer.Instance.Settings`, use the value of `Tracer` or `TracerSettings` that's already available wherever possible. It makes the tests cleaner too. ## Test coverage Same coverage, just a bit cleaner ## Other details Included as part of the config stack, just because I already refactored some of this code, and can't be bothered to faff with merge conflicts: https://datadoghq.atlassian.net/browse/LANGPLAT-819 - #7522 - #7525 - #7530 - #7532 - #7543 - #7544 - #7721 - #7722 - #7695 - #7723 - #7724 - #7744 👈
1 parent c43abbb commit 85673e7

File tree

5 files changed

+28
-25
lines changed

5 files changed

+28
-25
lines changed

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AdoNet/DbScopeFactory.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ static bool HasDbType(Span span, string dbType)
138138
}
139139

140140
public static bool TryGetIntegrationDetails(
141+
HashSet<string> disabledAdoNetCommandTypes,
141142
string? commandTypeFullName,
142143
[NotNullWhen(true)] out IntegrationId? integrationId,
143144
[NotNullWhen(true)] out string? dbType)
@@ -172,7 +173,7 @@ public static bool TryGetIntegrationDetails(
172173
return true;
173174
default:
174175
string commandTypeName = commandTypeFullName.Substring(commandTypeFullName.LastIndexOf(".", StringComparison.Ordinal) + 1);
175-
if (IsDisabledCommandType(commandTypeName))
176+
if (IsDisabledCommandType(commandTypeName, disabledAdoNetCommandTypes))
176177
{
177178
integrationId = null;
178179
dbType = null;
@@ -200,14 +201,14 @@ _ when commandTypeName.EndsWith(commandSuffix) =>
200201
}
201202
}
202203

203-
internal static bool IsDisabledCommandType(string commandTypeName)
204+
internal static bool IsDisabledCommandType(string commandTypeName, HashSet<string> disabledAdoNetCommandTypes)
204205
{
205206
if (string.IsNullOrEmpty(commandTypeName))
206207
{
207208
return false;
208209
}
209210

210-
var disabledTypes = Tracer.Instance.Settings.DisabledAdoNetCommandTypes;
211+
var disabledTypes = disabledAdoNetCommandTypes;
211212

212213
if (disabledTypes is null || disabledTypes.Count == 0)
213214
{
@@ -245,7 +246,7 @@ static Cache()
245246
{
246247
CommandType = typeof(TCommand);
247248

248-
if (TryGetIntegrationDetails(CommandType.FullName, out var integrationId, out var dbTypeName))
249+
if (TryGetIntegrationDetails(Tracer.Instance.Settings.DisabledAdoNetCommandTypes, CommandType.FullName, out var integrationId, out var dbTypeName))
249250
{
250251
// cache values for this TCommand type
251252
DbTypeName = dbTypeName;
@@ -275,7 +276,7 @@ static Cache()
275276

276277
// if command.GetType() != typeof(TCommand), we are probably instrumenting a method
277278
// defined in a base class like DbCommand and we can't use the cached values
278-
if (TryGetIntegrationDetails(commandType.FullName, out var integrationId, out var dbTypeName))
279+
if (TryGetIntegrationDetails(tracer.Settings.DisabledAdoNetCommandTypes, commandType.FullName, out var integrationId, out var dbTypeName))
279280
{
280281
var operationName = $"{dbTypeName}.query";
281282
var tagsFromConnectionString = GetTagsFromConnectionString(command);

tracer/src/Datadog.Trace/DataStreamsMonitoring/DataStreamsContextPropagator.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,8 @@ internal class DataStreamsContextPropagator
2929
/// </summary>
3030
/// <param name="context">A <see cref="PathwayContext"/> value that will be propagated into <paramref name="headers"/>.</param>
3131
/// <param name="headers">A <see cref="IHeadersCollection"/> to add new headers to.</param>
32+
/// <param name="isDataStreamsLegacyHeadersEnabled">Are legacy DSM headers enabled</param>
3233
/// <typeparam name="TCarrier">Type of header collection</typeparam>
33-
public void Inject<TCarrier>(PathwayContext context, TCarrier headers)
34-
where TCarrier : IBinaryHeadersCollection
35-
=> Inject(context, headers, Tracer.Instance.Settings.IsDataStreamsLegacyHeadersEnabled);
36-
37-
[TestingAndPrivateOnly]
3834
internal void Inject<TCarrier>(PathwayContext context, TCarrier headers, bool isDataStreamsLegacyHeadersEnabled)
3935
where TCarrier : IBinaryHeadersCollection
4036
{

tracer/src/Datadog.Trace/DataStreamsMonitoring/DataStreamsManager.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ internal class DataStreamsManager
2828
private static readonly AsyncLocal<PathwayContext?> LastConsumePathway = new(); // saves the context on consume checkpointing only
2929
private readonly ConcurrentDictionary<string, RateLimiter> _schemaRateLimiters = new();
3030
private readonly IDisposable _updateSubscription;
31+
private readonly bool _isLegacyDsmHeadersEnabled;
3132
private long _nodeHashBase; // note that this actually represents a `ulong` that we have done an unsafe cast for
3233
private bool _isEnabled;
3334
private bool _isInDefaultState;
@@ -40,6 +41,7 @@ public DataStreamsManager(
4041
{
4142
UpdateNodeHash(tracerSettings.Manager.InitialMutableSettings);
4243
_isEnabled = writer is not null;
44+
_isLegacyDsmHeadersEnabled = tracerSettings.IsDataStreamsLegacyHeadersEnabled;
4345
_writer = writer;
4446
_isInDefaultState = tracerSettings.IsDataStreamsMonitoringInDefaultState;
4547
_updateSubscription = tracerSettings.Manager.SubscribeToChanges(updates =>
@@ -129,7 +131,7 @@ public void InjectPathwayContext<TCarrier>(PathwayContext? context, TCarrier hea
129131
return;
130132
}
131133

132-
DataStreamsContextPropagator.Instance.Inject(context.Value, headers);
134+
DataStreamsContextPropagator.Instance.Inject(context.Value, headers, _isLegacyDsmHeadersEnabled);
133135
}
134136

135137
public void TrackBacklog(string tags, long value)

tracer/test/Datadog.Trace.ClrProfiler.Managed.Tests/DbScopeFactoryTests.cs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ internal void TryGetIntegrationDetails_CorrectNameGenerated(Type commandType, st
689689
var command = (IDbCommand)Activator.CreateInstance(commandType)!;
690690
command.CommandText = DbmCommandText;
691691

692-
bool result = DbScopeFactory.TryGetIntegrationDetails(command.GetType().FullName, out var actualIntegrationId, out var actualDbType);
692+
bool result = DbScopeFactory.TryGetIntegrationDetails([], command.GetType().FullName, out var actualIntegrationId, out var actualDbType);
693693
Assert.True(result);
694694
Assert.Equal(expectedIntegrationName, actualIntegrationId.ToString());
695695
Assert.Equal(expectedDbType, actualDbType);
@@ -698,12 +698,13 @@ internal void TryGetIntegrationDetails_CorrectNameGenerated(Type commandType, st
698698
[Fact]
699699
internal void TryGetIntegrationDetails_FailsForKnownCommandType()
700700
{
701-
bool result = DbScopeFactory.TryGetIntegrationDetails("InterceptableDbCommand", out var actualIntegrationId, out var actualDbType);
701+
var defaultDisabledCommands = new TracerSettings().DisabledAdoNetCommandTypes;
702+
bool result = DbScopeFactory.TryGetIntegrationDetails(defaultDisabledCommands, "InterceptableDbCommand", out var actualIntegrationId, out var actualDbType);
702703
Assert.False(result);
703704
Assert.False(actualIntegrationId.HasValue);
704705
Assert.Null(actualDbType);
705706

706-
bool result2 = DbScopeFactory.TryGetIntegrationDetails("ProfiledDbCommand", out var actualIntegrationId2, out var actualDbType2);
707+
bool result2 = DbScopeFactory.TryGetIntegrationDetails(defaultDisabledCommands, "ProfiledDbCommand", out var actualIntegrationId2, out var actualDbType2);
707708
Assert.False(result2);
708709
Assert.False(actualIntegrationId2.HasValue);
709710
Assert.Null(actualDbType2);
@@ -712,18 +713,18 @@ internal void TryGetIntegrationDetails_FailsForKnownCommandType()
712713
[Fact]
713714
internal void TryGetIntegrationDetails_FailsForKnownCommandTypes_AndUserDefined()
714715
{
715-
Tracer.Configure(TracerSettings.Create(new Dictionary<string, object> { { ConfigurationKeys.DisabledAdoNetCommandTypes, "SomeFakeDbCommand" } }));
716-
bool result = DbScopeFactory.TryGetIntegrationDetails("InterceptableDbCommand", out var actualIntegrationId, out var actualDbType);
716+
var disabledCommandTypes = TracerSettings.Create(new() { { ConfigurationKeys.DisabledAdoNetCommandTypes, "SomeFakeDbCommand" } }).DisabledAdoNetCommandTypes;
717+
bool result = DbScopeFactory.TryGetIntegrationDetails(disabledCommandTypes, "InterceptableDbCommand", out var actualIntegrationId, out var actualDbType);
717718
Assert.False(result);
718719
Assert.False(actualIntegrationId.HasValue);
719720
Assert.Null(actualDbType);
720721

721-
bool result2 = DbScopeFactory.TryGetIntegrationDetails("ProfiledDbCommand", out var actualIntegrationId2, out var actualDbType2);
722+
bool result2 = DbScopeFactory.TryGetIntegrationDetails(disabledCommandTypes, "ProfiledDbCommand", out var actualIntegrationId2, out var actualDbType2);
722723
Assert.False(result2);
723724
Assert.False(actualIntegrationId2.HasValue);
724725
Assert.Null(actualDbType2);
725726

726-
bool result3 = DbScopeFactory.TryGetIntegrationDetails("SomeFakeDbCommand", out var actualIntegrationId3, out var actualDbType3);
727+
bool result3 = DbScopeFactory.TryGetIntegrationDetails(disabledCommandTypes, "SomeFakeDbCommand", out var actualIntegrationId3, out var actualDbType3);
727728
Assert.False(result3);
728729
Assert.False(actualIntegrationId3.HasValue);
729730
Assert.Null(actualDbType3);
@@ -740,7 +741,8 @@ internal void TryGetIntegrationDetails_FailsForKnownCommandTypes_AndUserDefined(
740741
[InlineData("Custom.DB.Command", "AdoNet", "db")]
741742
internal void TryGetIntegrationDetails_CustomCommandType(string commandTypeFullName, string integrationId, string expectedDbType)
742743
{
743-
DbScopeFactory.TryGetIntegrationDetails(commandTypeFullName, out var actualIntegrationId, out var actualDbType);
744+
var defaultDisabledCommands = new TracerSettings().DisabledAdoNetCommandTypes;
745+
DbScopeFactory.TryGetIntegrationDetails(defaultDisabledCommands, commandTypeFullName, out var actualIntegrationId, out var actualDbType);
744746
Assert.Equal(integrationId, actualIntegrationId?.ToString());
745747
Assert.Equal(expectedDbType, actualDbType);
746748
}

tracer/test/Datadog.Trace.Tests/DataStreamsMonitoring/DataStreamsContextPropagatorTests.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ namespace Datadog.Trace.Tests.DataStreamsMonitoring
1515
{
1616
public class DataStreamsContextPropagatorTests
1717
{
18-
[Fact]
19-
public void CanRoundTripPathwayContext()
18+
[Theory]
19+
[CombinatorialData]
20+
public void CanRoundTripPathwayContext(bool isDataStreamsLegacyHeadersEnabled)
2021
{
2122
var oneMs = TimeSpan.FromMilliseconds(1);
2223
var headers = new TestHeadersCollection();
@@ -25,7 +26,7 @@ public void CanRoundTripPathwayContext()
2526
DateTimeOffset.UtcNow.AddSeconds(-5).ToUnixTimeNanoseconds(),
2627
DateTimeOffset.UtcNow.ToUnixTimeNanoseconds());
2728

28-
DataStreamsContextPropagator.Instance.Inject(context, headers);
29+
DataStreamsContextPropagator.Instance.Inject(context, headers, isDataStreamsLegacyHeadersEnabled);
2930

3031
var extracted = DataStreamsContextPropagator.Instance.Extract(headers);
3132

@@ -86,16 +87,17 @@ public void Extract_WhenBothHeadersPresent_PrefersBase64Header()
8687
extractedContext.Value.EdgeStart.Should().NotBe(binaryContext.EdgeStart);
8788
}
8889

89-
[Fact]
90-
public void InjectedHeaders_HaveCorrectFormat()
90+
[Theory]
91+
[CombinatorialData]
92+
public void InjectedHeaders_HaveCorrectFormat(bool isDataStreamsLegacyHeadersEnabled)
9193
{
9294
var headers = new TestHeadersCollection();
9395
var context = new PathwayContext(
9496
new PathwayHash(0x12345678),
9597
0x1122334455667788,
9698
unchecked((long)0x99AABBCCDDEEFF00));
9799

98-
DataStreamsContextPropagator.Instance.Inject(context, headers);
100+
DataStreamsContextPropagator.Instance.Inject(context, headers, isDataStreamsLegacyHeadersEnabled);
99101

100102
headers.Values.Should().ContainKey(DataStreamsPropagationHeaders.PropagationKeyBase64);
101103
var base64HeaderValueBytes = headers.Values[DataStreamsPropagationHeaders.PropagationKeyBase64];

0 commit comments

Comments
 (0)