[Instrumentation.EntityFrameworkCore] Update semantic conventions for stable release (#2130)

joegoldman2 · web-flow · commit 5a9c9a88b7cb · 2024-10-17T10:07:08.000-07:00
Co-authored-by: joegoldman2 &lt;147369450+joegoldman@users.noreply.github.com&gt;
diff --git a/opentelemetry-dotnet-contrib.sln b/opentelemetry-dotnet-contrib.sln
@@ -239,6 +239,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{1FCC8E
 	ProjectSection(SolutionItems) = preProject
 		src\Shared\ActivityInstrumentationHelper.cs = src\Shared\ActivityInstrumentationHelper.cs
 		src\Shared\AssemblyVersionExtensions.cs = src\Shared\AssemblyVersionExtensions.cs
+		src\Shared\DatabaseSemanticConventionHelper.cs = src\Shared\DatabaseSemanticConventionHelper.cs
 		src\Shared\DiagnosticSourceListener.cs = src\Shared\DiagnosticSourceListener.cs
 		src\Shared\DiagnosticSourceSubscriber.cs = src\Shared\DiagnosticSourceSubscriber.cs
 		src\Shared\ExceptionExtensions.cs = src\Shared\ExceptionExtensions.cs
diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md
@@ -2,6 +2,28 @@
 
 ## Unreleased
 
+* 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).
+  ([#2130](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2130))
+
 ## 1.0.0-beta.12
 
 Released 2024-Jun-18
diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentationOptions.cs
@@ -3,6 +3,8 @@
 
 using System.Data;
 using System.Diagnostics;
+using Microsoft.Extensions.Configuration;
+using static OpenTelemetry.Internal.DatabaseSemanticConventionHelper;
 
 namespace OpenTelemetry.Instrumentation.EntityFrameworkCore;
 
@@ -11,6 +13,21 @@ namespace OpenTelemetry.Instrumentation.EntityFrameworkCore;
 /// </summary>
 public class EntityFrameworkInstrumentationOptions
 {
+    /// <summary>
+    /// Initializes a new instance of the <see cref="EntityFrameworkInstrumentationOptions"/> class.
+    /// </summary>
+    public EntityFrameworkInstrumentationOptions()
+        : this(new ConfigurationBuilder().AddEnvironmentVariables().Build())
+    {
+    }
+
+    internal EntityFrameworkInstrumentationOptions(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="EntityFrameworkInstrumentation"/> should add the names of <see cref="CommandType.StoredProcedure"/> commands as the <see cref="Implementation.EntityFrameworkDiagnosticListener.AttributeDbStatement"/> tag. Default value: True.
     /// </summary>
@@ -50,4 +67,14 @@ public class EntityFrameworkInstrumentationOptions
     /// </list>
     /// </remarks>
     public Func<string?, IDbCommand, bool>? Filter { 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; }
 }
diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs
@@ -19,9 +19,12 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler
     internal const string EntityFrameworkCoreCommandError = "Microsoft.EntityFrameworkCore.Database.Command.CommandError";
 
     internal const string AttributePeerService = "peer.service";
+    internal const string AttributeServerAddress = "server.address";
     internal const string AttributeDbSystem = "db.system";
     internal const string AttributeDbName = "db.name";
+    internal const string AttributeDbNamespace = "db.namespace";
     internal const string AttributeDbStatement = "db.statement";
+    internal const string AttributeDbQueryText = "db.query.text";
 
     internal static readonly Assembly Assembly = typeof(EntityFrameworkDiagnosticListener).Assembly;
     internal static readonly string ActivitySourceName = Assembly.GetName().Name;
@@ -139,10 +142,19 @@ public override void OnEventWritten(string name, object? payload)
                         }
 
                         var dataSource = (string)this.dataSourceFetcher.Fetch(connection);
-                        activity.AddTag(AttributeDbName, database);
                         if (!string.IsNullOrEmpty(dataSource))
                         {
-                            activity.AddTag(AttributePeerService, dataSource);
+                            activity.AddTag(AttributeServerAddress, dataSource);
+                        }
+
+                        if (this.options.EmitOldAttributes)
+                        {
+                            activity.AddTag(AttributeDbName, database);
+                        }
+
+                        if (this.options.EmitNewAttributes)
+                        {
+                            activity.AddTag(AttributeDbNamespace, database);
                         }
                     }
                 }
@@ -196,15 +208,31 @@ public override void OnEventWritten(string name, object? payload)
                                 case CommandType.StoredProcedure:
                                     if (this.options.SetDbStatementForStoredProcedure)
                                     {
-                                        activity.AddTag(AttributeDbStatement, commandText);
+                                        if (this.options.EmitOldAttributes)
+                                        {
+                                            activity.AddTag(AttributeDbStatement, commandText);
+                                        }
+
+                                        if (this.options.EmitNewAttributes)
+                                        {
+                                            activity.AddTag(AttributeDbQueryText, commandText);
+                                        }
                                     }
 
                                     break;
 
                                 case CommandType.Text:
                                     if (this.options.SetDbStatementForText)
                                     {
-                                        activity.AddTag(AttributeDbStatement, commandText);
+                                        if (this.options.EmitOldAttributes)
+                                        {
+                                            activity.AddTag(AttributeDbStatement, commandText);
+                                        }
+
+                                        if (this.options.EmitNewAttributes)
+                                        {
+                                            activity.AddTag(AttributeDbQueryText, commandText);
+                                        }
                                     }
 
                                     break;
diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj
@@ -7,24 +7,28 @@
     <MinVerTagPrefix>Instrumentation.EntityFrameworkCore-</MinVerTagPrefix>
   </PropertyGroup>
 
-	<!-- Do not run Package Baseline Validation as this package has never released a stable version.
+  <!-- Do not run Package Baseline Validation as this package has never released a stable version.
   Remove this property once we have released a stable version and add PackageValidationBaselineVersion property. -->
   <PropertyGroup>
     <DisablePackageBaselineValidation>true</DisablePackageBaselineValidation>
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.Extensions.Configuration" Version="$(MicrosoftExtensionsConfigurationPkgVer)"/>
     <PackageReference Include="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionsOptionsPkgVer)" />
     <PackageReference Include="OpenTelemetry.Api.ProviderBuilderExtensions" Version="$(OpenTelemetryCoreLatestVersion)" />
   </ItemGroup>
 
   <ItemGroup>
     <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" />
+    <Compile Include="$(RepoRoot)\src\Shared\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\PropertyFetcher.cs" Link="Includes\PropertyFetcher.cs" />
   </ItemGroup>
 
diff --git a/src/Shared/DatabaseSemanticConventionHelper.cs b/src/Shared/DatabaseSemanticConventionHelper.cs
@@ -0,0 +1,83 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#nullable enable
+
+using System.Diagnostics.CodeAnalysis;
+using Microsoft.Extensions.Configuration;
+
+namespace OpenTelemetry.Internal;
+
+/// <summary>
+/// Helper class for Database Semantic Conventions.
+/// </summary>
+/// <remarks>
+/// Due to a breaking change in the semantic convention, affected instrumentation libraries
+/// must inspect an environment variable to determine which attributes to emit.
+/// This is expected to be removed when the instrumentation libraries reach Stable.
+/// <see href="https://github.com/open-telemetry/semantic-conventions/blob/v1.28.0/docs/database/database-spans.md"/>.
+/// </remarks>
+internal static class DatabaseSemanticConventionHelper
+{
+    internal const string SemanticConventionOptInKeyName = "OTEL_SEMCONV_STABILITY_OPT_IN";
+    internal static readonly char[] Separator = new[] { ',', ' ' };
+
+    [Flags]
+    public enum DatabaseSemanticConvention
+    {
+        /// <summary>
+        /// Instructs an instrumentation library to emit the old experimental Database attributes.
+        /// </summary>
+        Old = 0x1,
+
+        /// <summary>
+        /// Instructs an instrumentation library to emit the new, v1.28.0 Database attributes.
+        /// </summary>
+        New = 0x2,
+
+        /// <summary>
+        /// Instructs an instrumentation library to emit both the old and new attributes.
+        /// </summary>
+        Dupe = Old | New,
+    }
+
+    public static DatabaseSemanticConvention GetSemanticConventionOptIn(IConfiguration configuration)
+    {
+        if (TryGetConfiguredValues(configuration, out var values))
+        {
+            if (values.Contains("database/dup"))
+            {
+                return DatabaseSemanticConvention.Dupe;
+            }
+            else if (values.Contains("database"))
+            {
+                return DatabaseSemanticConvention.New;
+            }
+        }
+
+        return DatabaseSemanticConvention.Old;
+    }
+
+    private static bool TryGetConfiguredValues(IConfiguration configuration, [NotNullWhen(true)] out HashSet<string>? values)
+    {
+        try
+        {
+            var stringValue = configuration[SemanticConventionOptInKeyName];
+
+            if (string.IsNullOrWhiteSpace(stringValue))
+            {
+                values = null;
+                return false;
+            }
+
+            var stringValues = stringValue!.Split(separator: Separator, options: StringSplitOptions.RemoveEmptyEntries);
+            values = new HashSet<string>(stringValues, StringComparer.OrdinalIgnoreCase);
+            return true;
+        }
+        catch
+        {
+            values = null;
+            return false;
+        }
+    }
+}
diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/DatabaseSemanticConventionHelperTests.cs b/test/OpenTelemetry.Contrib.Shared.Tests/DatabaseSemanticConventionHelperTests.cs
@@ -0,0 +1,79 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+using Microsoft.Extensions.Configuration;
+using Xunit;
+using static OpenTelemetry.Internal.DatabaseSemanticConventionHelper;
+
+namespace OpenTelemetry.Internal.Tests;
+
+public class DatabaseSemanticConventionHelperTests
+{
+    public static IEnumerable<object[]> TestCases => new List<object[]>
+    {
+        new object[] { null!,  DatabaseSemanticConvention.Old },
+        new object[] { string.Empty,  DatabaseSemanticConvention.Old },
+        new object[] { " ",  DatabaseSemanticConvention.Old },
+        new object[] { "junk",  DatabaseSemanticConvention.Old },
+        new object[] { "none",  DatabaseSemanticConvention.Old },
+        new object[] { "NONE",  DatabaseSemanticConvention.Old },
+        new object[] { "database",  DatabaseSemanticConvention.New },
+        new object[] { "DATABASE",  DatabaseSemanticConvention.New },
+        new object[] { "database/dup",  DatabaseSemanticConvention.Dupe },
+        new object[] { "DATABASE/DUP",  DatabaseSemanticConvention.Dupe },
+        new object[] { "junk,,junk",  DatabaseSemanticConvention.Old },
+        new object[] { "junk,JUNK",  DatabaseSemanticConvention.Old },
+        new object[] { "junk1,junk2",  DatabaseSemanticConvention.Old },
+        new object[] { "junk,database",  DatabaseSemanticConvention.New },
+        new object[] { "junk,database , database ,junk",  DatabaseSemanticConvention.New },
+        new object[] { "junk,database/dup",  DatabaseSemanticConvention.Dupe },
+        new object[] { "junk, database/dup ",  DatabaseSemanticConvention.Dupe },
+        new object[] { "database/dup,database",  DatabaseSemanticConvention.Dupe },
+        new object[] { "database,database/dup",  DatabaseSemanticConvention.Dupe },
+    };
+
+    [Fact]
+    public void VerifyFlags()
+    {
+        var testValue = DatabaseSemanticConvention.Dupe;
+        Assert.True(testValue.HasFlag(DatabaseSemanticConvention.Old));
+        Assert.True(testValue.HasFlag(DatabaseSemanticConvention.New));
+
+        testValue = DatabaseSemanticConvention.Old;
+        Assert.True(testValue.HasFlag(DatabaseSemanticConvention.Old));
+        Assert.False(testValue.HasFlag(DatabaseSemanticConvention.New));
+
+        testValue = DatabaseSemanticConvention.New;
+        Assert.False(testValue.HasFlag(DatabaseSemanticConvention.Old));
+        Assert.True(testValue.HasFlag(DatabaseSemanticConvention.New));
+    }
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public void VerifyGetSemanticConventionOptIn_UsingEnvironmentVariable(string input, string expectedValue)
+    {
+        try
+        {
+            Environment.SetEnvironmentVariable(SemanticConventionOptInKeyName, input);
+
+            var expected = Enum.Parse(typeof(DatabaseSemanticConvention), expectedValue);
+            Assert.Equal(expected, GetSemanticConventionOptIn(new ConfigurationBuilder().AddEnvironmentVariables().Build()));
+        }
+        finally
+        {
+            Environment.SetEnvironmentVariable(SemanticConventionOptInKeyName, null);
+        }
+    }
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public void VerifyGetSemanticConventionOptIn_UsingIConfiguration(string input, string expectedValue)
+    {
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> { [SemanticConventionOptInKeyName] = input })
+            .Build();
+
+        var expected = Enum.Parse(typeof(DatabaseSemanticConvention), expectedValue);
+        Assert.Equal(expected, GetSemanticConventionOptIn(configuration));
+    }
+}
diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/OpenTelemetry.Contrib.Shared.Tests.csproj b/test/OpenTelemetry.Contrib.Shared.Tests/OpenTelemetry.Contrib.Shared.Tests.csproj
@@ -7,14 +7,18 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.Extensions.Configuration" Version="$(MicrosoftExtensionsConfigurationPkgVer)"/>
     <PackageReference Include="OpenTelemetry.Api" Version="$(OpenTelemetryCoreLatestVersion)" />
   </ItemGroup>
 
   <ItemGroup>
     <Compile Include="$(RepoRoot)\src\Shared\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\ActivityInstrumentationHelper.cs" Link="Includes\ActivityInstrumentationHelper.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\DatabaseSemanticConventionHelper.cs" Link="Includes\DatabaseSemanticConventionHelper.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
     <Compile Include="$(RepoRoot)\src\Shared\GrpcTagHelper.cs" Link="Includes\GrpcTagHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\GrpcStatusCanonicalCode.cs" Link="Includes\GrpcStatusCanonicalCode.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\PropertyFetcher.AOT.cs" Link="Includes\PropertyFetcher.AOT.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\RedactionHelper.cs" Link="Includes\RedactionHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\RequestDataHelper.cs" Link="Includes\RequestDataHelper.cs" />
diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs
diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkInstrumentationOptionsTests b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkInstrumentationOptionsTests
