[Instrumentation.SqlClient] Support only stable HTTP semantic convention

OpenTelemetry.sln

@@ -265,7 +265,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{A49299
  src\Shared\DiagnosticDefinitions.cs = src\Shared\DiagnosticDefinitions.cs
  src\Shared\ExceptionExtensions.cs = src\Shared\ExceptionExtensions.cs
  src\Shared\Guard.cs = src\Shared\Guard.cs
- src\Shared\HttpSemanticConventionHelper.cs = src\Shared\HttpSemanticConventionHelper.cs
  src\Shared\MathHelper.cs = src\Shared\MathHelper.cs
  src\Shared\PeerServiceResolver.cs = src\Shared\PeerServiceResolver.cs
  src\Shared\PeriodicExportingMetricReaderHelper.cs = src\Shared\PeriodicExportingMetricReaderHelper.cs

src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+* Removed support for the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable
+ which toggled the use of the new conventions for the
+ [server, client, and shared network attributes](https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/general/attributes.md#server-client-and-shared-network-attributes).
+ Now that this suite of attributes are stable, this instrumentation will only
+ emit the new attributes.
+ ([#5270](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5270))
+
 ## 1.6.0-beta.3
 
 Released 2023-Nov-17

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

@@ -12,9 +12,6 @@
 
  <ItemGroup>
  <Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
- <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
- <Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
- <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
  <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
  </ItemGroup>
 
@@ -29,7 +26,6 @@
 
  <ItemGroup>
  <PackageReference Include="Microsoft.Extensions.Options" />
- <PackageReference Include="Microsoft.Extensions.Configuration" />
  </ItemGroup>
 
 </Project>

src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentationOptions.cs

@@ -5,9 +5,7 @@
 using System.Data;
 using System.Diagnostics;
 using System.Text.RegularExpressions;
-using Microsoft.Extensions.Configuration;
 using OpenTelemetry.Trace;
-using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
 
 namespace OpenTelemetry.Instrumentation.SqlClient;
 
@@ -52,26 +50,6 @@ public class SqlClientInstrumentationOptions
 
  private static readonly ConcurrentDictionary<string, SqlConnectionDetails> ConnectionDetailCache = new(StringComparer.OrdinalIgnoreCase);
 
- private readonly bool emitOldAttributes;
- private readonly bool emitNewAttributes;
-
- /// <summary>
- /// Initializes a new instance of the <see cref="SqlClientInstrumentationOptions"/> class.
- /// </summary>
- public SqlClientInstrumentationOptions()
- : this(new ConfigurationBuilder().AddEnvironmentVariables().Build())
- {
- }
-
- internal SqlClientInstrumentationOptions(IConfiguration configuration)
- {
- Debug.Assert(configuration != null, "configuration was null");
-
- var httpSemanticConvention = GetSemanticConventionOptIn(configuration);
- this.emitOldAttributes = httpSemanticConvention.HasFlag(HttpSemanticConvention.Old);
- this.emitNewAttributes = httpSemanticConvention.HasFlag(HttpSemanticConvention.New);
- }
-
  /// <summary>
  /// Gets or sets a value indicating whether or not the <see
  /// cref="SqlClientInstrumentation"/> should add the names of <see
@@ -130,13 +108,6 @@ internal SqlClientInstrumentationOptions(IConfiguration configuration)
  /// <para>
  /// The default behavior is to set the SqlConnection DataSource as the <see cref="SemanticConventions.AttributePeerService"/> tag.
  /// If enabled, SqlConnection DataSource will be parsed and the server name will be sent as the
- /// <see cref="SemanticConventions.AttributeNetPeerName"/> or <see cref="SemanticConventions.AttributeNetPeerIp"/> tag,
- /// the instance name will be sent as the <see cref="SemanticConventions.AttributeDbMsSqlInstanceName"/> tag,
- /// and the port will be sent as the <see cref="SemanticConventions.AttributeNetPeerPort"/> tag if it is not 1433 (the default port).
- /// </para>
- /// <para>
- /// If the environment variable OTEL_SEMCONV_STABILITY_OPT_IN is set to "http", the newer Semantic Convention v1.21.0 Attributes will be emitted.
- /// SqlConnection DataSource will be parsed and the server name will be sent as the
  /// <see cref="SemanticConventions.AttributeServerAddress"/> or <see cref="SemanticConventions.AttributeServerSocketAddress"/> tag,
  /// the instance name will be sent as the <see cref="SemanticConventions.AttributeDbMsSqlInstanceName"/> tag,
  /// and the port will be sent as the <see cref="SemanticConventions.AttributeServerPort"/> tag if it is not 1433 (the default port).
@@ -301,40 +272,19 @@ internal void AddConnectionLevelDetailsToActivity(string dataSource, Activity sq
  sqlActivity.SetTag(SemanticConventions.AttributeDbMsSqlInstanceName, connectionDetails.InstanceName);
  }
 
- if (this.emitOldAttributes)
+ if (!string.IsNullOrEmpty(connectionDetails.ServerHostName))
  {
- if (!string.IsNullOrEmpty(connectionDetails.ServerHostName))
- {
- sqlActivity.SetTag(SemanticConventions.AttributeNetPeerName, connectionDetails.ServerHostName);
- }
- else
- {
- sqlActivity.SetTag(SemanticConventions.AttributeNetPeerIp, connectionDetails.ServerIpAddress);
- }
-
- if (!string.IsNullOrEmpty(connectionDetails.Port))
- {
- sqlActivity.SetTag(SemanticConventions.AttributeNetPeerPort, connectionDetails.Port);
- }
+ sqlActivity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerHostName);
  }
-
- // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/database/database-spans.md
- if (this.emitNewAttributes)
+ else
  {
- if (!string.IsNullOrEmpty(connectionDetails.ServerHostName))
- {
- sqlActivity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerHostName);
- }
- else
- {
- sqlActivity.SetTag(SemanticConventions.AttributeServerSocketAddress, connectionDetails.ServerIpAddress);
- }
+ sqlActivity.SetTag(SemanticConventions.AttributeServerSocketAddress, 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);
- }
+ 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/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs

@@ -62,15 +62,10 @@ public static TracerProviderBuilder AddSqlClientInstrumentation(
 
  name ??= Options.DefaultName;
 
- builder.ConfigureServices(services =>
+ if (configureSqlClientInstrumentationOptions != null)
  {
- if (configureSqlClientInstrumentationOptions != null)
- {
- services.Configure(name, configureSqlClientInstrumentationOptions);
- }
-
- services.RegisterOptionsFactory(configuration => new SqlClientInstrumentationOptions(configuration));
- });
+ builder.ConfigureServices(services => services.Configure(name, configureSqlClientInstrumentationOptions));
+ }
 
  builder.AddInstrumentation(sp =>
  {

test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientInstrumentationOptionsTests.cs

@@ -2,10 +2,8 @@
 // SPDX-License-Identifier: Apache-2.0
 
 using System.Diagnostics;
-using Microsoft.Extensions.Configuration;
 using OpenTelemetry.Trace;
 using Xunit;
-using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
 
 namespace OpenTelemetry.Instrumentation.SqlClient.Tests;
 
@@ -72,52 +70,7 @@ public void SqlClientInstrumentationOptions_EnableConnectionLevelAttributes(
  {
  var source = new ActivitySource("sql-client-instrumentation");
  var activity = source.StartActivity("Test Sql Activity");
- var options = new SqlClientInstrumentationOptions
- {
- EnableConnectionLevelAttributes = enableConnectionLevelAttributes,
- };
- options.AddConnectionLevelDetailsToActivity(dataSource, activity);
-
- if (!enableConnectionLevelAttributes)
- {
- Assert.Equal(expectedServerHostName, activity.GetTagValue(SemanticConventions.AttributePeerService));
- }
- else
- {
- Assert.Equal(expectedServerHostName, activity.GetTagValue(SemanticConventions.AttributeNetPeerName));
- }
-
- Assert.Equal(expectedServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeNetPeerIp));
- Assert.Equal(expectedInstanceName, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName));
- Assert.Equal(expectedPort, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort));
- }
-
- // Tests for v1.21.0 Semantic Conventions for database client calls.
- // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/database/database-spans.md
- // This test emits the new attributes.
- // This test method can replace the other (old) test method when this library is GA.
- [Theory]
- [InlineData(true, "localhost", "localhost", null, null, null)]
- [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)]
- public void SqlClientInstrumentationOptions_EnableConnectionLevelAttributes_New(
- bool enableConnectionLevelAttributes,
- string dataSource,
- string expectedServerHostName,
- string expectedServerIpAddress,
- string expectedInstanceName,
- string expectedPort)
- {
- var source = new ActivitySource("sql-client-instrumentation");
- var activity = source.StartActivity("Test Sql Activity");
-
- var configuration = new ConfigurationBuilder()
- .AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http" })
- .Build();
-
- var options = new SqlClientInstrumentationOptions(configuration)
+ var options = new SqlClientInstrumentationOptions()
  {
  EnableConnectionLevelAttributes = enableConnectionLevelAttributes,
  };
@@ -136,64 +89,4 @@ public void SqlClientInstrumentationOptions_EnableConnectionLevelAttributes_New(
  Assert.Equal(expectedInstanceName, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName));
  Assert.Equal(expectedPort, activity.GetTagValue(SemanticConventions.AttributeServerPort));
  }
-
- // Tests for v1.21.0 Semantic Conventions for database client calls.
- // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/database/database-spans.md
- // This test emits both the new and older attributes.
- // This test method can be deleted when this library is GA.
- [Theory]
- [InlineData(true, "localhost", "localhost", null, null, null)]
- [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)]
- public void SqlClientInstrumentationOptions_EnableConnectionLevelAttributes_Dupe(
- bool enableConnectionLevelAttributes,
- string dataSource,
- string expectedServerHostName,
- string expectedServerIpAddress,
- string expectedInstanceName,
- string expectedPort)
- {
- var source = new ActivitySource("sql-client-instrumentation");
- var activity = source.StartActivity("Test Sql Activity");
-
- var configuration = new ConfigurationBuilder()
- .AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http/dup" })
- .Build();
-
- var options = new SqlClientInstrumentationOptions(configuration)
- {
- EnableConnectionLevelAttributes = enableConnectionLevelAttributes,
- };
- options.AddConnectionLevelDetailsToActivity(dataSource, activity);
-
- // New
- 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(expectedPort, activity.GetTagValue(SemanticConventions.AttributeServerPort));
-
- // Old
- if (!enableConnectionLevelAttributes)
- {
- Assert.Equal(expectedServerHostName, activity.GetTagValue(SemanticConventions.AttributePeerService));
- }
- else
- {
- Assert.Equal(expectedServerHostName, activity.GetTagValue(SemanticConventions.AttributeNetPeerName));
- }
-
- Assert.Equal(expectedServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeNetPeerIp));
- Assert.Equal(expectedInstanceName, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName));
- Assert.Equal(expectedPort, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort));
- }
 }

test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs

@@ -230,7 +230,7 @@ private static void VerifyActivityData(
  }
  else
  {
- Assert.Equal(connectionDetails.ServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeNetPeerIp));
+ Assert.Equal(connectionDetails.ServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeServerSocketAddress));
  }
 
  if (!string.IsNullOrEmpty(connectionDetails.InstanceName))

test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj

@@ -20,7 +20,6 @@
 
  <ItemGroup>
  <Compile Include="$(RepoRoot)\src\Shared\ActivityInstrumentationHelper.cs" Link="Includes\ActivityInstrumentationHelper.cs" />
- <Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceInstrumentation\ListenerHandler.cs" Link="Includes\ListenerHandler.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceInstrumentation\PropertyFetcher.cs" Link="Includes\PropertyFetcher.cs" />
  <Compile Include="$(RepoRoot)\src\Shared\Metrics\Base2ExponentialBucketHistogramHelper.cs" Link="Includes\Metrics\Base2ExponentialBucketHistogramHelper.cs" />