[Instrumentation.SqlClient] Update to follow new DB span conventions (#…

…2229)

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
 

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,15 +126,31 @@ 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;
 
                                 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;

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);
+                }
             }
         }
     }

src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj

@@ -17,8 +17,10 @@
   <ItemGroup>
     <Compile Include="$(RepoRoot)\src\Shared\ActivityInstrumentationHelper.cs" Link="Includes\ActivityInstrumentationHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\AssemblyVersionExtensions.cs" Link="Includes\AssemblyVersionExtensions.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\DatabaseSemanticConventionHelper.cs" Link="Includes\DatabaseSemanticConventionHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceListener.cs" Link="Includes\DiagnosticSourceListener.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceSubscriber.cs" Link="Includes\DiagnosticSourceSubscriber.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
     <Compile Include="$(RepoRoot)\src\Shared\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\ListenerHandler.cs" Link="Includes\ListenerHandler.cs" />
@@ -28,6 +30,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.Extensions.Configuration" Version="$(MicrosoftExtensionsConfigurationPkgVer)"/>
     <PackageReference Include="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionsOptionsPkgVer)" />
     <PackageReference Include="OpenTelemetry.Api.ProviderBuilderExtensions" Version="$(OpenTelemetryCoreLatestVersion)" />
   </ItemGroup>

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).
 

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<string, SqlConnectionDetails> ConnectionDetailCache = new(StringComparer.OrdinalIgnoreCase);
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="SqlClientTraceInstrumentationOptions"/> class.
+    /// </summary>
+    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);
+    }
+
     /// <summary>
     /// Gets or sets a value indicating whether or not the <see
     /// cref="SqlClientInstrumentation"/> should add the names of <see
@@ -169,6 +186,16 @@ public class SqlClientTraceInstrumentationOptions
     /// </remarks>
     public bool RecordException { get; set; }
 
+    /// <summary>
+    /// Gets or sets a value indicating whether the old database attributes should be emitted.
+    /// </summary>
+    internal bool EmitOldAttributes { get; set; }
+
+    /// <summary>
+    /// Gets or sets a value indicating whether the new database attributes should be emitted.
+    /// </summary>
+    internal bool EmitNewAttributes { get; set; }
+
     internal static SqlConnectionDetails ParseDataSource(string dataSource)
     {
         Match match = DataSourceRegex.Match(dataSource);
@@ -255,19 +282,19 @@ 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))
             {
                 connectionDetails = ParseDataSource(dataSource);
                 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);
             }
         }

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
 }

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);
     }