Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to disable additional ADO.NET Command Types #6042

Merged
merged 6 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public static bool TryGetIntegrationDetails(
return true;
default:
string commandTypeName = commandTypeFullName.Substring(commandTypeFullName.LastIndexOf(".", StringComparison.Ordinal) + 1);
if (commandTypeName == "InterceptableDbCommand" || commandTypeName == "ProfiledDbCommand")
if (IsDisabledCommandType(commandTypeName))
{
integrationId = null;
dbType = null;
Expand Down Expand Up @@ -198,6 +198,31 @@ _ when commandTypeName.EndsWith(commandSuffix) =>
}
}

internal static bool IsDisabledCommandType(string commandTypeName)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we instead pass the Tracer object through the method parameters so this method doesn't rely on the static Tracer.Instance? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me!
Needed to propagate the parameter up a bit as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has proven to be a bit of a headache as I have ultimately needed to pass Tracer.Instance to the static Cache class

Without doing more refactoring I may just revert this and depend on the static Tracer.Instance for now and come back later to refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah @zacharycmontoya I've reverted this, I'll look at it later (follow up PR) if that sounds okay?

{
if (string.IsNullOrEmpty(commandTypeName))
{
return false;
}

var disabledTypes = Tracer.Instance.Settings.DisabledAdoNetCommandTypes;

if (disabledTypes is null || disabledTypes.Count == 0)
{
return false;
}

foreach (var disabledType in disabledTypes)
{
if (string.Equals(disabledType, commandTypeName, StringComparison.OrdinalIgnoreCase))
{
return true;
}
}

return false;
}

public static class Cache<TCommand>
{
// ReSharper disable StaticMemberInGenericType
Expand Down
8 changes: 8 additions & 0 deletions tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.cs
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,14 @@ internal static partial class ConfigurationKeys
/// </summary>
public const string RemoveClientServiceNamesEnabled = "DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED";

/// <summary>
/// Configuration key for the comma-separated list of user disabled
/// ADO.NET CommandType names that should not have Span created for them.
/// <para>"InterceptableDbCommand" and "ProfiledDbCommand" are always disabled by default.</para>
/// </summary>
/// <seealso cref="TracerSettings.DisabledAdoNetCommandTypes"/>
public const string DisabledAdoNetCommandTypes = "DD_TRACE_DISABLED_ADONET_COMMAND_TYPES";
lucaspimentel marked this conversation as resolved.
Show resolved Hide resolved
lucaspimentel marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// String constants for CI Visibility configuration keys.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ internal ImmutableTracerSettings(TracerSettings settings, bool unusedParamNotToU
}

DbmPropagationMode = settings.DbmPropagationMode;
DisabledAdoNetCommandTypes = settings.DisabledAdoNetCommandTypes;

// We need to take a snapshot of the config telemetry for the tracer settings,
// but we can't send it to the static collector, as this settings object may never be "activated"
Expand Down Expand Up @@ -638,6 +639,11 @@ internal ImmutableTracerSettings(TracerSettings settings, bool unusedParamNotToU
|| IsRunningInGCPFunctions
|| LambdaMetadata.IsRunningInLambda);

/// <summary>
/// Gets the disabled ADO.NET Command Types that won't have spans generated for them.
/// </summary>
internal HashSet<string> DisabledAdoNetCommandTypes { get; }

/// <summary>
/// Create a <see cref="ImmutableTracerSettings"/> populated from the default sources
/// returned by <see cref="GlobalConfigurationSource.Instance"/>.
Expand Down
16 changes: 16 additions & 0 deletions tracer/src/Datadog.Trace/Configuration/TracerSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,17 @@ _ when x.ToBoolean() is { } boolean => boolean,
.WithKeys(ConfigurationKeys.FeatureFlags.CommandsCollectionEnabled)
.AsBool(false);

var defaultDisabledAdoNetCommandTypes = new string[] { "InterceptableDbCommand", "ProfiledDbCommand" };
var userDisabledAdoNetCommandTypes = config.WithKeys(ConfigurationKeys.DisabledAdoNetCommandTypes).AsString();

DisabledAdoNetCommandTypes = new HashSet<string>(defaultDisabledAdoNetCommandTypes, StringComparer.OrdinalIgnoreCase);

if (!string.IsNullOrEmpty(userDisabledAdoNetCommandTypes))
{
var userSplit = TrimSplitString(userDisabledAdoNetCommandTypes, commaSeparator);
DisabledAdoNetCommandTypes.UnionWith(userSplit);
}

// we "enrich" with these values which aren't _strictly_ configuration, but which we want to track as we tracked them in v1
telemetry.Record(ConfigTelemetryData.NativeTracerVersion, Instrumentation.GetNativeTracerVersion(), recordValue: true, ConfigurationOrigins.Default);
telemetry.Record(ConfigTelemetryData.FullTrustAppDomain, value: AppDomain.CurrentDomain.IsFullyTrusted, ConfigurationOrigins.Default);
Expand Down Expand Up @@ -1040,6 +1051,11 @@ public bool DiagnosticSourceEnabled
/// </summary>
internal SchemaVersion MetadataSchemaVersion { get; }

/// <summary>
/// Gets the disabled ADO.NET Command Types that won't have spans generated for them.
/// </summary>
internal HashSet<string> DisabledAdoNetCommandTypes { get; }

/// <summary>
/// Create a <see cref="TracerSettings"/> populated from the default sources
/// returned by <see cref="GlobalConfigurationSource.Instance"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,17 @@
// </copyright>

using System.Linq;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Datadog.Trace.Configuration;
using Datadog.Trace.TestHelpers;
using VerifyXunit;
using Xunit;
using Xunit.Abstractions;

namespace Datadog.Trace.ClrProfiler.IntegrationTests.AdoNet
{
[UsesVerify]
public class FakeCommandTests : TracingIntegrationTest
{
public FakeCommandTests(ITestOutputHelper output)
Expand All @@ -30,6 +33,14 @@ public FakeCommandTests(ITestOutputHelper output)
[Trait("Category", "EndToEnd")]
public Task SubmitsTracesV1() => RunTest("v1");

[SkippableFact]
[Trait("Category", "EndToEnd")]
public Task SubmitTracesDisabledFakeCommandV0() => RunDisabledFakeCommandTest("v0");

[SkippableFact]
[Trait("Category", "EndToEnd")]
public Task SubmitTracesDisabledFakeCommandV1() => RunDisabledFakeCommandTest("v1");

private async Task RunTest(string metadataSchemaVersion)
{
// ALWAYS: 91 spans
Expand Down Expand Up @@ -63,7 +74,41 @@ private async Task RunTest(string metadataSchemaVersion)
Assert.Equal(expectedOperationName, span.Name);
}

var settings = VerifyHelper.GetSpanVerifierSettings();
var dbResourceRegex = new Regex("Test-[0-9a-fA-F]+");
settings.AddRegexScrubber(dbResourceRegex, "Test-GUID");
await VerifyHelper.VerifySpans(spans, settings)
.UseFileName(nameof(FakeCommandTests) + $"_{metadataSchemaVersion}");

telemetry.AssertIntegrationEnabled(IntegrationId.AdoNet);
}

private async Task RunDisabledFakeCommandTest(string metadataSchemaVersion)
{
// ALWAYS: 21 spans - these are the custom spans from RelationalDatabaseTestHarness
// There should be NO SQL spans in the snapshots
const int expectedSpanCount = 21;

SetEnvironmentVariable("DD_TRACE_SPAN_ATTRIBUTE_SCHEMA", metadataSchemaVersion);
SetEnvironmentVariable("DD_TRACE_DISABLED_ADONET_COMMAND_TYPES", "FakeCommand");
var isExternalSpan = metadataSchemaVersion == "v0";
var clientSpanServiceName = EnvironmentHelper.FullSampleName;

using var telemetry = this.ConfigureTelemetry();
using var agent = EnvironmentHelper.GetMockAgent();
using var process = await RunSampleAndWaitForExit(agent);
var spans = agent.WaitForSpans(expectedSpanCount);
int actualSpanCount = spans.Count();

Assert.Equal(expectedSpanCount, actualSpanCount);
var settings = VerifyHelper.GetSpanVerifierSettings();
var dbResourceRegex = new Regex("Test-[0-9a-fA-F]+");
settings.AddRegexScrubber(dbResourceRegex, "Test-GUID");
await VerifyHelper.VerifySpans(spans, settings)
.UseFileName(nameof(FakeCommandTests) + $"_{metadataSchemaVersion}_disabledFakeCommand");

// We should have created 0 spans from ADO.NET integration - spans created are from RelationaTestHarness and are manual spans
telemetry.AssertIntegrationDisabled(IntegrationId.AdoNet);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

namespace Datadog.Trace.ClrProfiler.Managed.Tests
{
[Collection("DbScopeFactoryTests")]
[TracerRestorer]
public class DbScopeFactoryTests
{
private const string DbmCommandText = "SELECT 1";
Expand Down Expand Up @@ -271,6 +273,26 @@ internal void TryGetIntegrationDetails_FailsForKnownCommandType()
Assert.Null(actualDbType2);
}

[Fact]
internal void TryGetIntegrationDetails_FailsForKnownCommandTypes_AndUserDefined()
{
Tracer.Configure(TracerSettings.Create(new Dictionary<string, object> { { ConfigurationKeys.DisabledAdoNetCommandTypes, "SomeFakeDbCommand" } }));
bool result = DbScopeFactory.TryGetIntegrationDetails("InterceptableDbCommand", out var actualIntegrationId, out var actualDbType);
Assert.False(result);
Assert.False(actualIntegrationId.HasValue);
Assert.Null(actualDbType);

bool result2 = DbScopeFactory.TryGetIntegrationDetails("ProfiledDbCommand", out var actualIntegrationId2, out var actualDbType2);
Assert.False(result2);
Assert.False(actualIntegrationId2.HasValue);
Assert.Null(actualDbType2);

bool result3 = DbScopeFactory.TryGetIntegrationDetails("SomeFakeDbCommand", out var actualIntegrationId3, out var actualDbType3);
Assert.False(result3);
Assert.False(actualIntegrationId3.HasValue);
Assert.Null(actualDbType3);
}

[Theory]
[InlineData("System.Data.SqlClient.SqlCommand", "SqlClient", "sql-server")]
[InlineData("MySql.Data.MySqlClient.MySqlCommand", "MySql", "mysql")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@
"DD_IAST_DB_ROWS_TO_TAINT": "iast_db_rows_to_taint",
"DD_IAST_COOKIE_FILTER_PATTERN": "iast_cookie_filter_pattern",
"DD_TRACE_STARTUP_LOGS": "trace_startup_logs_enabled",
"DD_TRACE_DISABLED_ADONET_COMMAND_TYPES": "trace_disabled_adonet_command_types",
"DD_MAX_LOGFILE_SIZE": "trace_log_file_max_size",
"DD_TRACE_LOGGING_RATE": "trace_log_rate",
"DD_TRACE_LOG_PATH": "trace_log_path",
Expand Down
Loading
Loading