From 6b7f2dd77cf9d37260a853fcc95f7b77e296065d Mon Sep 17 00:00:00 2001 From: John Du Hart Date: Tue, 13 Apr 2021 23:09:45 -0400 Subject: [PATCH] SqlClient: Provide `db.system` when creating activity (#1979) --- .../CHANGELOG.md | 2 ++ .../Implementation/SqlActivitySourceHelper.cs | 7 ++++ .../SqlClientDiagnosticListener.cs | 8 +++-- .../SqlEventSourceListener.netfx.cs | 8 +++-- ...try.Instrumentation.SqlClient.Tests.csproj | 1 + .../SqlClientTests.cs | 36 +++++++++++++++++++ 6 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md index 6691b7a2dc8..7c6a963dcb3 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased * Instrumentation modified to depend only on the API. +* Activities are now created with the `db.system` attribute set for usage + during sampling. ([#1979](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1979)) ## 1.0.0-rc3 diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs index 49720e385f9..e45690c68fe 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs @@ -14,7 +14,9 @@ // limitations under the License. // using System; +using System.Collections.Generic; using System.Diagnostics; +using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation.SqlClient.Implementation { @@ -29,6 +31,11 @@ internal class SqlActivitySourceHelper public const string MicrosoftSqlServerDatabaseSystemName = "mssql"; + public static readonly IEnumerable> CreationTags = new[] + { + new KeyValuePair(SemanticConventions.AttributeDbSystem, MicrosoftSqlServerDatabaseSystemName), + }; + private static readonly Version Version = typeof(SqlActivitySourceHelper).Assembly.GetName().Version; #pragma warning disable SA1202 // Elements should be ordered by access <- In this case, Version MUST come before ActivitySource otherwise null ref exception is thrown. internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString()); diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs index 5b3502e7957..ce88d82ec03 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs @@ -57,7 +57,12 @@ public override void OnCustom(string name, Activity activity, object payload) case SqlMicrosoftBeforeExecuteCommand: { // SqlClient does not create an Activity. So the activity coming in here will be null or the root span. - activity = SqlActivitySourceHelper.ActivitySource.StartActivity(SqlActivitySourceHelper.ActivityName, ActivityKind.Client); + activity = SqlActivitySourceHelper.ActivitySource.StartActivity( + SqlActivitySourceHelper.ActivityName, + ActivityKind.Client, + default(ActivityContext), + SqlActivitySourceHelper.CreationTags); + if (activity == null) { // There is no listener or it decided not to sample the current request. @@ -82,7 +87,6 @@ public override void OnCustom(string name, Activity activity, object payload) _ = this.dataSourceFetcher.TryFetch(connection, out var dataSource); _ = this.commandTextFetcher.TryFetch(command, out var commandText); - activity.SetTag(SemanticConventions.AttributeDbSystem, SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName); activity.SetTag(SemanticConventions.AttributeDbName, (string)database); this.options.AddConnectionLevelDetailsToActivity((string)dataSource, activity); diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs index 539ea03a656..aaf97663e68 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs @@ -125,7 +125,12 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) return; } - var activity = SqlActivitySourceHelper.ActivitySource.StartActivity(SqlActivitySourceHelper.ActivityName, ActivityKind.Client); + var activity = SqlActivitySourceHelper.ActivitySource.StartActivity( + SqlActivitySourceHelper.ActivityName, + ActivityKind.Client, + default(ActivityContext), + SqlActivitySourceHelper.CreationTags); + if (activity == null) { // There is no listener or it decided not to sample the current request. @@ -138,7 +143,6 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) if (activity.IsAllDataRequested) { - activity.SetTag(SemanticConventions.AttributeDbSystem, SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName); activity.SetTag(SemanticConventions.AttributeDbName, databaseName); this.options.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity); diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj index a489bedee5e..a6df4d2c761 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj @@ -10,6 +10,7 @@ + diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs index 70d19b35328..7f999811522 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs @@ -86,8 +86,10 @@ public void SuccessfulCommandTest( bool shouldEnrich = true) { var activityProcessor = new Mock>(); + var sampler = new TestSampler(); using var shutdownSignal = Sdk.CreateTracerProviderBuilder() .AddProcessor(activityProcessor.Object) + .SetSampler(sampler) .AddSqlClientInstrumentation(options => { #if !NETFRAMEWORK @@ -135,6 +137,7 @@ public void SuccessfulCommandTest( var activity = (Activity)activityProcessor.Invocations[1].Arguments[0]; VerifyActivityData(commandType, commandText, captureStoredProcedureCommandName, captureTextCommandContent, isFailure, recordException, dataSource, activity); + VerifySamplingParameters(sampler.LatestSamplingParameters); } // DiagnosticListener-based instrumentation is only available on .NET Core @@ -280,6 +283,30 @@ public void SqlClientErrorsAreCollectedSuccessfully(string beforeCommand, string sqlConnection.DataSource, (Activity)processor.Invocations[2].Arguments[0]); } + + [Theory] + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand)] + public void SqlClientCreatesActivityWithDbSystem( + string beforeCommand) + { + using var sqlConnection = new SqlConnection(TestConnectionString); + using var sqlCommand = sqlConnection.CreateCommand(); + + var sampler = new TestSampler + { + SamplingAction = _ => new SamplingResult(SamplingDecision.Drop), + }; + using (Sdk.CreateTracerProviderBuilder() + .AddSqlClientInstrumentation() + .SetSampler(sampler) + .Build()) + { + this.fakeSqlClientDiagnosticSource.Write(beforeCommand, new { }); + } + + VerifySamplingParameters(sampler.LatestSamplingParameters); + } #endif private static void VerifyActivityData( @@ -351,6 +378,15 @@ private static void VerifyActivityData( Assert.Equal(dataSource, activity.GetTagValue(SemanticConventions.AttributePeerService)); } + private static void VerifySamplingParameters(SamplingParameters samplingParameters) + { + Assert.NotNull(samplingParameters.Tags); + Assert.Contains( + samplingParameters.Tags, + kvp => kvp.Key == SemanticConventions.AttributeDbSystem + && (string)kvp.Value == SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName); + } + private static void ActivityEnrichment(Activity activity, string method, object obj) { switch (method)