From 89fb05276b741be9ea926086469586314607ff4e Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Mon, 21 Oct 2024 12:20:13 -0700 Subject: [PATCH] [Instrumentation.SqlClient] Update to follow new DB span conventions (#2229) --- .../CHANGELOG.md | 26 +++++++ .../SqlClientDiagnosticListener.cs | 33 ++++++++- .../SqlEventSourceListener.netfx.cs | 20 ++++- ...Telemetry.Instrumentation.SqlClient.csproj | 3 + .../README.md | 11 ++- .../SqlClientTraceInstrumentationOptions.cs | 42 +++++++++-- src/Shared/SemanticConventions.cs | 12 ++- .../SqlClientIntegrationTests.cs | 2 +- .../SqlClientTests.cs | 73 ++++++++++++++++--- ...lClientTraceInstrumentationOptionsTests.cs | 68 +++++++++++++---- .../SqlEventSourceTests.netfx.cs | 65 +++++++++++++---- 11 files changed, 297 insertions(+), 58 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md index fa9054cb07..377f145731 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md @@ -4,6 +4,32 @@ * Drop support for .NET 6 as this target is no longer supported. ([#2159](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2159)) +* The new database semantic conventions can be opted in to by setting + the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. This allows for a + transition period for users to experiment with the new semantic conventions + and adapt as necessary. The environment variable supports the following + values: + * `database` - emit the new, frozen (proposed for stable) database + attributes, and stop emitting the old experimental database + attributes that the instrumentation emitted previously. + * `database/dup` - emit both the old and the frozen (proposed for stable) database + attributes, allowing for a more seamless transition. + * The default behavior (in the absence of one of these values) is to continue + emitting the same database semantic conventions that were emitted in + the previous version. + * Note: this option will be be removed after the new database + semantic conventions is marked stable. At which time this + instrumentation can receive a stable release, and the old database + semantic conventions will no longer be supported. Refer to the + specification for more information regarding the new database + semantic conventions for + [spans](https://github.com/open-telemetry/semantic-conventions/blob/v1.28.0/docs/database/database-spans.md). + ([#2229](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2229)) +* **Breaking change**: The `peer.service` and `server.socket.address` attributes + are no longer emitted. Users should rely on the `server.address` attribute + for the same information. Note that `server.address` is only included when + the `EnableConnectionLevelAttributes` option is enabled. + ([#2229](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2229)) ## 1.9.0-beta.1 diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs index acbecedbb2..8594f907f2 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs @@ -94,10 +94,21 @@ public override void OnEventWritten(string name, object? payload) _ = this.connectionFetcher.TryFetch(command, out var connection); _ = this.databaseFetcher.TryFetch(connection, out var database); + // TODO: Need to understand what scenario (if any) database will be null here + // so that we set DisplayName and db.name/db.namespace correctly. if (database != null) { activity.DisplayName = (string)database; - activity.SetTag(SemanticConventions.AttributeDbName, database); + + if (this.options.EmitOldAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbName, database); + } + + if (this.options.EmitNewAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbNamespace, database); + } } _ = this.dataSourceFetcher.TryFetch(connection, out var dataSource); @@ -115,7 +126,15 @@ public override void OnEventWritten(string name, object? payload) case CommandType.StoredProcedure: if (this.options.SetDbStatementForStoredProcedure) { - activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); + if (this.options.EmitOldAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); + } + + if (this.options.EmitNewAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbQueryText, commandText); + } } break; @@ -123,7 +142,15 @@ public override void OnEventWritten(string name, object? payload) case CommandType.Text: if (this.options.SetDbStatementForText) { - activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); + if (this.options.EmitOldAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); + } + + if (this.options.EmitNewAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbQueryText, commandText); + } } break; diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs index 40c3c056a8..8e8a9163d0 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs @@ -130,14 +130,30 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) if (activity.IsAllDataRequested) { - activity.SetTag(SemanticConventions.AttributeDbName, databaseName); + if (this.options.EmitOldAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbName, databaseName); + } + + if (this.options.EmitNewAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbNamespace, databaseName); + } this.options.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity); string commandText = (string)eventData.Payload[3]; if (!string.IsNullOrEmpty(commandText) && this.options.SetDbStatementForText) { - activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); + if (this.options.EmitOldAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); + } + + if (this.options.EmitNewAttributes) + { + activity.SetTag(SemanticConventions.AttributeDbQueryText, commandText); + } } } } diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj b/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj index 26c571d46d..2736e3ddc5 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj +++ b/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj @@ -17,8 +17,10 @@ + + @@ -28,6 +30,7 @@ + diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/README.md b/src/OpenTelemetry.Instrumentation.SqlClient/README.md index 4463ef2c26..f5fe916ac3 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/README.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/README.md @@ -165,12 +165,11 @@ command text will be captured. > [!NOTE] > EnableConnectionLevelAttributes is supported on all runtimes. -By default, `EnabledConnectionLevelAttributes` is disabled and this -instrumentation sets the `peer.service` attribute to the -[`DataSource`](https://docs.microsoft.com/dotnet/api/system.data.common.dbconnection.datasource) -property of the connection. If `EnabledConnectionLevelAttributes` is enabled, -the `DataSource` will be parsed and the server name will be sent as the -`net.peer.name` or `net.peer.ip` attribute, the instance name will be sent as +By default, `EnabledConnectionLevelAttributes` is disabled. +If `EnabledConnectionLevelAttributes` is enabled, +the [`DataSource`](https://docs.microsoft.com/dotnet/api/system.data.common.dbconnection.datasource) +will be parsed and the server name or IP address will be sent as +the `server.address` attribute, the instance name will be sent as the `db.mssql.instance_name` attribute, and the port will be sent as the `net.peer.port` attribute if it is not 1433 (the default port). diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs index 138f7e5b2c..4b2cd56c43 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs @@ -5,7 +5,9 @@ using System.Data; using System.Diagnostics; using System.Text.RegularExpressions; +using Microsoft.Extensions.Configuration; using OpenTelemetry.Trace; +using static OpenTelemetry.Internal.DatabaseSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.SqlClient; @@ -50,6 +52,21 @@ public class SqlClientTraceInstrumentationOptions private static readonly ConcurrentDictionary ConnectionDetailCache = new(StringComparer.OrdinalIgnoreCase); + /// + /// Initializes a new instance of the class. + /// + public SqlClientTraceInstrumentationOptions() + : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) + { + } + + internal SqlClientTraceInstrumentationOptions(IConfiguration configuration) + { + var databaseSemanticConvention = GetSemanticConventionOptIn(configuration); + this.EmitOldAttributes = databaseSemanticConvention.HasFlag(DatabaseSemanticConvention.Old); + this.EmitNewAttributes = databaseSemanticConvention.HasFlag(DatabaseSemanticConvention.New); + } + /// /// Gets or sets a value indicating whether or not the should add the names of public bool RecordException { get; set; } + /// + /// Gets or sets a value indicating whether the old database attributes should be emitted. + /// + internal bool EmitOldAttributes { get; set; } + + /// + /// Gets or sets a value indicating whether the new database attributes should be emitted. + /// + internal bool EmitNewAttributes { get; set; } + internal static SqlConnectionDetails ParseDataSource(string dataSource) { Match match = DataSourceRegex.Match(dataSource); @@ -255,11 +282,9 @@ internal static SqlConnectionDetails ParseDataSource(string dataSource) internal void AddConnectionLevelDetailsToActivity(string dataSource, Activity sqlActivity) { - if (!this.EnableConnectionLevelAttributes) - { - sqlActivity.SetTag(SemanticConventions.AttributePeerService, dataSource); - } - else + // TODO: The attributes added here are required. We need to consider + // collecting these attributes by default. + if (this.EnableConnectionLevelAttributes) { if (!ConnectionDetailCache.TryGetValue(dataSource, out SqlConnectionDetails? connectionDetails)) { @@ -267,7 +292,9 @@ internal void AddConnectionLevelDetailsToActivity(string dataSource, Activity sq ConnectionDetailCache.TryAdd(dataSource, connectionDetails); } - if (!string.IsNullOrEmpty(connectionDetails.InstanceName)) + // TODO: In the new conventions, instance name should now be captured + // as a part of db.namespace, when available. + if (this.EmitOldAttributes && !string.IsNullOrEmpty(connectionDetails.InstanceName)) { sqlActivity.SetTag(SemanticConventions.AttributeDbMsSqlInstanceName, connectionDetails.InstanceName); } @@ -278,12 +305,11 @@ internal void AddConnectionLevelDetailsToActivity(string dataSource, Activity sq } else { - sqlActivity.SetTag(SemanticConventions.AttributeServerSocketAddress, connectionDetails.ServerIpAddress); + sqlActivity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerIpAddress); } if (!string.IsNullOrEmpty(connectionDetails.Port)) { - // TODO: Should we continue to emit this if the default port (1433) is being used? sqlActivity.SetTag(SemanticConventions.AttributeServerPort, connectionDetails.Port); } } diff --git a/src/Shared/SemanticConventions.cs b/src/Shared/SemanticConventions.cs index 9f1c1ee234..cd3847cc2a 100644 --- a/src/Shared/SemanticConventions.cs +++ b/src/Shared/SemanticConventions.cs @@ -44,7 +44,6 @@ internal static class SemanticConventions public const string AttributeHttpResponseContentLength = "http.response_content_length"; public const string AttributeHttpResponseContentLengthUncompressed = "http.response_content_length_uncompressed"; - public const string AttributeDbSystem = "db.system"; public const string AttributeDbConnectionString = "db.connection_string"; public const string AttributeDbUser = "db.user"; public const string AttributeDbMsSqlInstanceName = "db.mssql.instance_name"; @@ -136,5 +135,16 @@ internal static class SemanticConventions public const string AttributeMessagingKafkaMessageKey = "messaging.kafka.message.key"; public const string AttributeMessagingKafkaMessageOffset = "messaging.kafka.message.offset"; + // New database conventions as of commit: + // https://github.com/open-telemetry/semantic-conventions/blob/25f74191d749645fdd5ec42ae661438cf2c1cf51/docs/database/database-spans.md#common-attributes + public const string AttributeDbSystem = "db.system"; + public const string AttributeDbCollectionName = "db.collection.name"; + public const string AttributeDbNamespace = "db.namespace"; + public const string AttributeDbOperationName = "db.operation.name"; + public const string AttributeResponseStatusCode = "db.response.status_code"; + public const string AttributeDbOperationBatchSize = "db.operation.batch.size"; + public const string AttributeDbQuerySummary = "db.query.summary"; + public const string AttributeDbQueryText = "db.query.text"; + #pragma warning restore CS1591 // Missing XML comment for publicly visible type or member } diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs index 7d34f18dd2..9086c31490 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientIntegrationTests.cs @@ -91,7 +91,7 @@ public void SuccessfulCommandTest( Assert.Single(activities); var activity = activities[0]; - SqlClientTests.VerifyActivityData(commandType, commandText, captureStoredProcedureCommandName, captureTextCommandContent, isFailure, recordException, shouldEnrich, dataSource, activity); + SqlClientTests.VerifyActivityData(commandType, commandText, captureStoredProcedureCommandName, captureTextCommandContent, isFailure, recordException, shouldEnrich, activity); SqlClientTests.VerifySamplingParameters(sampler.LatestSamplingParameters); } diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs index 4c2f913436..f2f68aeb71 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs @@ -73,6 +73,26 @@ public void SqlClient_NamedOptions() [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", false, true, false)] [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.Text, "select * from sys.databases", false, true)] [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.Text, "select * from sys.databases", false, true, false)] + + // Test cases when EmitOldAttributes = false and EmitNewAttributes = true (i.e., OTEL_SEMCONV_STABILITY_OPT_IN=database) + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", true, false, true, false, true)] + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", true, false, false, false, true)] + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.Text, "select * from sys.databases", true, false, true, false, true)] + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.Text, "select * from sys.databases", true, false, false, false, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", false, true, true, false, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", false, true, false, false, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.Text, "select * from sys.databases", false, true, true, false, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.Text, "select * from sys.databases", false, true, false, false, true)] + + // Test cases when EmitOldAttributes = true and EmitNewAttributes = true (i.e., OTEL_SEMCONV_STABILITY_OPT_IN=database/dup) + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", true, false, true, true, true)] + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", true, false, false, true, true)] + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.Text, "select * from sys.databases", true, false, true, true, true)] + [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataAfterExecuteCommand, CommandType.Text, "select * from sys.databases", true, false, false, true, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", false, true, true, true, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.StoredProcedure, "SP_GetOrders", false, true, false, true, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.Text, "select * from sys.databases", false, true, true, true, true)] + [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticListener.SqlMicrosoftAfterExecuteCommand, CommandType.Text, "select * from sys.databases", false, true, false, true, true)] public void SqlClientCallsAreCollectedSuccessfully( string beforeCommand, string afterCommand, @@ -80,7 +100,9 @@ public void SqlClientCallsAreCollectedSuccessfully( string commandText, bool captureStoredProcedureCommandName, bool captureTextCommandContent, - bool shouldEnrich = true) + bool shouldEnrich = true, + bool emitOldAttributes = true, + bool emitNewAttributes = false) { using var sqlConnection = new SqlConnection(TestConnectionString); using var sqlCommand = sqlConnection.CreateCommand(); @@ -96,6 +118,9 @@ public void SqlClientCallsAreCollectedSuccessfully( { opt.Enrich = ActivityEnrichment; } + + opt.EmitOldAttributes = emitOldAttributes; + opt.EmitNewAttributes = emitNewAttributes; }) .AddInMemoryExporter(activities) .Build()) @@ -140,8 +165,9 @@ public void SqlClientCallsAreCollectedSuccessfully( false, false, shouldEnrich, - sqlConnection.DataSource, - activity); + activity, + emitOldAttributes, + emitNewAttributes); } [Theory] @@ -208,7 +234,6 @@ public void SqlClientErrorsAreCollectedSuccessfully(string beforeCommand, string true, recordException, shouldEnrich, - sqlConnection.DataSource, activity); } @@ -302,8 +327,9 @@ internal static void VerifyActivityData( bool isFailure, bool recordException, bool shouldEnrich, - string dataSource, - Activity activity) + Activity activity, + bool emitOldAttributes = true, + bool emitNewAttributes = false) { Assert.Equal("master", activity.DisplayName); Assert.Equal(ActivityKind.Client, activity.Kind); @@ -342,18 +368,36 @@ internal static void VerifyActivityData( } Assert.Equal(SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName, activity.GetTagValue(SemanticConventions.AttributeDbSystem)); - Assert.Equal("master", activity.GetTagValue(SemanticConventions.AttributeDbName)); + + if (emitOldAttributes) + { + Assert.Equal("master", activity.GetTagValue(SemanticConventions.AttributeDbName)); + } + + if (emitNewAttributes) + { + Assert.Equal("master", activity.GetTagValue(SemanticConventions.AttributeDbNamespace)); + } switch (commandType) { case CommandType.StoredProcedure: if (captureStoredProcedureCommandName) { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + if (emitOldAttributes) + { + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + } + + if (emitNewAttributes) + { + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); + } } else { Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); } break; @@ -361,17 +405,24 @@ internal static void VerifyActivityData( case CommandType.Text: if (captureTextCommandContent) { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + if (emitOldAttributes) + { + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + } + + if (emitNewAttributes) + { + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); + } } else { Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); } break; } - - Assert.Equal(dataSource, activity.GetTagValue(SemanticConventions.AttributePeerService)); } internal static void VerifySamplingParameters(SamplingParameters samplingParameters) diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs index 1387a27577..b85c158134 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; +using Microsoft.Extensions.Configuration; +using OpenTelemetry.Internal; using OpenTelemetry.Trace; using Xunit; @@ -59,14 +61,30 @@ public void ParseDataSourceTests( [InlineData(true, "127.0.0.1,1433", null, "127.0.0.1", null, null)] [InlineData(true, "127.0.0.1,1434", null, "127.0.0.1", null, "1434")] [InlineData(true, "127.0.0.1\\instanceName, 1818", null, "127.0.0.1", "instanceName", "1818")] - [InlineData(false, "localhost", "localhost", null, null, null)] + [InlineData(false, "localhost", null, null, null, null)] + + // Test cases when EmitOldAttributes = false and EmitNewAttributes = true (i.e., OTEL_SEMCONV_STABILITY_OPT_IN=database) + [InlineData(true, "localhost", "localhost", null, null, null, false, true)] + [InlineData(true, "127.0.0.1,1433", null, "127.0.0.1", null, null, false, true)] + [InlineData(true, "127.0.0.1,1434", null, "127.0.0.1", null, "1434", false, true)] + [InlineData(true, "127.0.0.1\\instanceName, 1818", null, "127.0.0.1", null, "1818", false, true)] + [InlineData(false, "localhost", null, null, null, null, false, true)] + + // Test cases when EmitOldAttributes = true and EmitNewAttributes = true (i.e., OTEL_SEMCONV_STABILITY_OPT_IN=database/dup) + [InlineData(true, "localhost", "localhost", null, null, null, true, true)] + [InlineData(true, "127.0.0.1,1433", null, "127.0.0.1", null, null, true, true)] + [InlineData(true, "127.0.0.1,1434", null, "127.0.0.1", null, "1434", true, true)] + [InlineData(true, "127.0.0.1\\instanceName, 1818", null, "127.0.0.1", "instanceName", "1818", true, true)] + [InlineData(false, "localhost", null, null, null, null, true, true)] public void SqlClientTraceInstrumentationOptions_EnableConnectionLevelAttributes( bool enableConnectionLevelAttributes, string dataSource, string? expectedServerHostName, string? expectedServerIpAddress, string? expectedInstanceName, - string? expectedPort) + string? expectedPort, + bool emitOldAttributes = true, + bool emitNewAttributes = false) { var source = new ActivitySource("sql-client-instrumentation"); var activity = source.StartActivity("Test Sql Activity"); @@ -74,20 +92,44 @@ public void SqlClientTraceInstrumentationOptions_EnableConnectionLevelAttributes var options = new SqlClientTraceInstrumentationOptions() { EnableConnectionLevelAttributes = enableConnectionLevelAttributes, + EmitOldAttributes = emitOldAttributes, + EmitNewAttributes = emitNewAttributes, }; options.AddConnectionLevelDetailsToActivity(dataSource, activity); - if (!enableConnectionLevelAttributes) - { - Assert.Equal(expectedServerHostName, activity.GetTagValue(SemanticConventions.AttributePeerService)); - } - else - { - Assert.Equal(expectedServerHostName, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - } - - Assert.Equal(expectedServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeServerSocketAddress)); - Assert.Equal(expectedInstanceName, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName)); + Assert.Equal(expectedServerHostName ?? expectedServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); + Assert.Equal(emitOldAttributes ? expectedInstanceName : null, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName)); Assert.Equal(expectedPort, activity.GetTagValue(SemanticConventions.AttributeServerPort)); } + + [Fact] + public void ShouldEmitOldAttributesWhenStabilityOptInIsDatabaseDup() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { [DatabaseSemanticConventionHelper.SemanticConventionOptInKeyName] = "database/dup" }) + .Build(); + var options = new SqlClientTraceInstrumentationOptions(configuration); + Assert.True(options.EmitOldAttributes); + Assert.True(options.EmitNewAttributes); + } + + [Fact] + public void ShouldEmitOldAttributesWhenStabilityOptInIsNotSpecified() + { + var configuration = new ConfigurationBuilder().Build(); + var options = new SqlClientTraceInstrumentationOptions(configuration); + Assert.True(options.EmitOldAttributes); + Assert.False(options.EmitNewAttributes); + } + + [Fact] + public void ShouldEmitNewAttributesWhenStabilityOptInIsDatabase() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary { [DatabaseSemanticConventionHelper.SemanticConventionOptInKeyName] = "database" }) + .Build(); + var options = new SqlClientTraceInstrumentationOptions(configuration); + Assert.False(options.EmitOldAttributes); + Assert.True(options.EmitNewAttributes); + } } diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs index fe89800745..9fc2d9e979 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs @@ -83,6 +83,26 @@ public async Task SuccessfulCommandTest(CommandType commandType, string commandT [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.Text, "select 1/0", false, true)] [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.StoredProcedure, "sp_who", false)] [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.StoredProcedure, "sp_who", true, false, 0, true)] + + // Test cases when EmitOldAttributes = false and EmitNewAttributes = true (i.e., OTEL_SEMCONV_STABILITY_OPT_IN=database) + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.Text, "select 1/1", false, false, 0, false, false, true)] + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.Text, "select 1/0", false, true, 0, false, false, true)] + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.StoredProcedure, "sp_who", false, false, 0, false, false, true)] + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.StoredProcedure, "sp_who", true, false, 0, true, false, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.Text, "select 1/1", false, false, 0, false, false, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.Text, "select 1/0", false, true, 0, false, false, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.StoredProcedure, "sp_who", false, false, 0, false, false, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.StoredProcedure, "sp_who", true, false, 0, true, false, true)] + + // Test cases when EmitOldAttributes = true and EmitNewAttributes = true (i.e., OTEL_SEMCONV_STABILITY_OPT_IN=database/dup) + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.Text, "select 1/1", false, false, 0, false, true, true)] + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.Text, "select 1/0", false, true, 0, false, true, true)] + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.StoredProcedure, "sp_who", false, false, 0, false, true, true)] + [InlineData(typeof(FakeBehavingAdoNetSqlEventSource), CommandType.StoredProcedure, "sp_who", true, false, 0, true, true, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.Text, "select 1/1", false, false, 0, false, true, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.Text, "select 1/0", false, true, 0, false, true, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.StoredProcedure, "sp_who", false, false, 0, false, true, true)] + [InlineData(typeof(FakeBehavingMdsSqlEventSource), CommandType.StoredProcedure, "sp_who", true, false, 0, true, true, true)] public void EventSourceFakeTests( Type eventSourceType, CommandType commandType, @@ -90,7 +110,9 @@ public void EventSourceFakeTests( bool captureText, bool isFailure = false, int sqlExceptionNumber = 0, - bool enableConnectionLevelAttributes = false) + bool enableConnectionLevelAttributes = false, + bool emitOldAttributes = true, + bool emitNewAttributes = false) { using IFakeBehavingSqlEventSource fakeSqlEventSource = (IFakeBehavingSqlEventSource)Activator.CreateInstance(eventSourceType); @@ -101,6 +123,8 @@ public void EventSourceFakeTests( { options.SetDbStatementForText = captureText; options.EnableConnectionLevelAttributes = enableConnectionLevelAttributes; + options.EmitOldAttributes = emitOldAttributes; + options.EmitNewAttributes = emitNewAttributes; }) .Build(); @@ -125,7 +149,7 @@ public void EventSourceFakeTests( var activity = exportedItems[0]; - VerifyActivityData(commandText, captureText, isFailure, "127.0.0.1", activity, enableConnectionLevelAttributes); + VerifyActivityData(commandText, captureText, isFailure, "127.0.0.1", activity, enableConnectionLevelAttributes, emitOldAttributes, emitNewAttributes); } [Theory] @@ -214,30 +238,28 @@ private static void VerifyActivityData( bool isFailure, string dataSource, Activity activity, - bool enableConnectionLevelAttributes = false) + bool enableConnectionLevelAttributes = false, + bool emitOldAttributes = true, + bool emitNewAttributes = false) { Assert.Equal("master", activity.DisplayName); Assert.Equal(ActivityKind.Client, activity.Kind); Assert.Equal(SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName, activity.GetTagValue(SemanticConventions.AttributeDbSystem)); - if (!enableConnectionLevelAttributes) - { - Assert.Equal(dataSource, activity.GetTagValue(SemanticConventions.AttributePeerService)); - } - else + if (enableConnectionLevelAttributes) { var connectionDetails = SqlClientTraceInstrumentationOptions.ParseDataSource(dataSource); if (!string.IsNullOrEmpty(connectionDetails.ServerHostName)) { - Assert.Equal(connectionDetails.ServerHostName, activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); + Assert.Equal(connectionDetails.ServerHostName, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); } else { - Assert.Equal(connectionDetails.ServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeServerSocketAddress)); + Assert.Equal(connectionDetails.ServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); } - if (!string.IsNullOrEmpty(connectionDetails.InstanceName)) + if (emitOldAttributes && !string.IsNullOrEmpty(connectionDetails.InstanceName)) { Assert.Equal(connectionDetails.InstanceName, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName)); } @@ -248,15 +270,32 @@ private static void VerifyActivityData( } } - Assert.Equal("master", activity.GetTagValue(SemanticConventions.AttributeDbName)); + if (emitOldAttributes) + { + Assert.Equal("master", activity.GetTagValue(SemanticConventions.AttributeDbName)); + } + + if (emitNewAttributes) + { + Assert.Equal("master", activity.GetTagValue(SemanticConventions.AttributeDbNamespace)); + } if (captureText) { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + if (emitOldAttributes) + { + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + } + + if (emitNewAttributes) + { + Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); + } } else { Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); } if (!isFailure)