diff --git a/OpenTelemetry.sln b/OpenTelemetry.sln index 7c8bb952f0..28c8c85b51 100644 --- a/OpenTelemetry.sln +++ b/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 diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md index 75ca992ec0..31d8b70e2c 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md +++ b/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 diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj b/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj index 5d10612045..113c5ec5d9 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj +++ b/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj @@ -12,9 +12,6 @@ - - - @@ -29,7 +26,6 @@ - diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentationOptions.cs index f9b808b264..e2d54565ce 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentationOptions.cs +++ b/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 ConnectionDetailCache = new(StringComparer.OrdinalIgnoreCase); - private readonly bool emitOldAttributes; - private readonly bool emitNewAttributes; - - /// - /// Initializes a new instance of the class. - /// - 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); - } - /// /// Gets or sets a value indicating whether or not the should add the names of /// The default behavior is to set the SqlConnection DataSource as the tag. /// If enabled, SqlConnection DataSource will be parsed and the server name will be sent as the - /// or tag, - /// the instance name will be sent as the tag, - /// and the port will be sent as the tag if it is not 1433 (the default port). - /// - /// - /// 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 /// or tag, /// the instance name will be sent as the tag, /// and the port will be sent as the 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); } } } diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs index 59ec17c67e..774d64ed91 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs +++ b/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 => { diff --git a/src/Shared/HttpSemanticConventionHelper.cs b/src/Shared/HttpSemanticConventionHelper.cs deleted file mode 100644 index 220f9874ac..0000000000 --- a/src/Shared/HttpSemanticConventionHelper.cs +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#nullable enable - -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using Microsoft.Extensions.Configuration; - -namespace OpenTelemetry.Internal; - -/// -/// Helper class for Http Semantic Conventions. -/// -/// -/// 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. -/// . -/// -internal static class HttpSemanticConventionHelper -{ - public const string SemanticConventionOptInKeyName = "OTEL_SEMCONV_STABILITY_OPT_IN"; - - [Flags] - public enum HttpSemanticConvention - { - /// - /// Instructs an instrumentation library to emit the old experimental HTTP attributes. - /// - Old = 0x1, - - /// - /// Instructs an instrumentation library to emit the new, v1.21.0 Http attributes. - /// - New = 0x2, - - /// - /// Instructs an instrumentation library to emit both the old and new attributes. - /// - Dupe = Old | New, - } - - public static HttpSemanticConvention GetSemanticConventionOptIn(IConfiguration configuration) - { - Debug.Assert(configuration != null, "configuration was null"); - - if (configuration != null && TryGetConfiguredValues(configuration, out var values)) - { - if (values.Contains("http/dup")) - { - return HttpSemanticConvention.Dupe; - } - else if (values.Contains("http")) - { - return HttpSemanticConvention.New; - } - } - - return HttpSemanticConvention.Old; - } - - private static bool TryGetConfiguredValues(IConfiguration configuration, [NotNullWhen(true)] out HashSet? values) - { - try - { - var stringValue = configuration[SemanticConventionOptInKeyName]; - - if (string.IsNullOrWhiteSpace(stringValue)) - { - values = null; - return false; - } - - var stringValues = stringValue!.Split(separator: new[] { ',', ' ' }, options: StringSplitOptions.RemoveEmptyEntries); - values = new HashSet(stringValues, StringComparer.OrdinalIgnoreCase); - return true; - } - catch - { - values = null; - return false; - } - } -} diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientInstrumentationOptionsTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientInstrumentationOptionsTests.cs index 0e64fe4d13..7626706503 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientInstrumentationOptionsTests.cs +++ b/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 { [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 { [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)); - } } diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs index f931abb864..2aede25223 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs +++ b/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)) diff --git a/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj b/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj index 0679b5d365..86fe7e3684 100644 --- a/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj +++ b/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj @@ -20,7 +20,6 @@ - diff --git a/test/OpenTelemetry.Tests/Shared/HttpSemanticConventionHelperTest.cs b/test/OpenTelemetry.Tests/Shared/HttpSemanticConventionHelperTest.cs deleted file mode 100644 index a7fb0224bf..0000000000 --- a/test/OpenTelemetry.Tests/Shared/HttpSemanticConventionHelperTest.cs +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -using Microsoft.Extensions.Configuration; -using Xunit; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; - -namespace OpenTelemetry.Tests.Shared; - -public class HttpSemanticConventionHelperTest -{ - public static IEnumerable TestCases => new List - { - new object[] { null, HttpSemanticConvention.Old }, - new object[] { string.Empty, HttpSemanticConvention.Old }, - new object[] { " ", HttpSemanticConvention.Old }, - new object[] { "junk", HttpSemanticConvention.Old }, - new object[] { "none", HttpSemanticConvention.Old }, - new object[] { "NONE", HttpSemanticConvention.Old }, - new object[] { "http", HttpSemanticConvention.New }, - new object[] { "HTTP", HttpSemanticConvention.New }, - new object[] { "http/dup", HttpSemanticConvention.Dupe }, - new object[] { "HTTP/DUP", HttpSemanticConvention.Dupe }, - new object[] { "junk,,junk", HttpSemanticConvention.Old }, - new object[] { "junk,JUNK", HttpSemanticConvention.Old }, - new object[] { "junk1,junk2", HttpSemanticConvention.Old }, - new object[] { "junk,http", HttpSemanticConvention.New }, - new object[] { "junk,http , http ,junk", HttpSemanticConvention.New }, - new object[] { "junk,http/dup", HttpSemanticConvention.Dupe }, - new object[] { "junk, http/dup ", HttpSemanticConvention.Dupe }, - new object[] { "http/dup,http", HttpSemanticConvention.Dupe }, - new object[] { "http,http/dup", HttpSemanticConvention.Dupe }, - }; - - [Fact] - public void VerifyFlags() - { - var testValue = HttpSemanticConvention.Dupe; - Assert.True(testValue.HasFlag(HttpSemanticConvention.Old)); - Assert.True(testValue.HasFlag(HttpSemanticConvention.New)); - - testValue = HttpSemanticConvention.Old; - Assert.True(testValue.HasFlag(HttpSemanticConvention.Old)); - Assert.False(testValue.HasFlag(HttpSemanticConvention.New)); - - testValue = HttpSemanticConvention.New; - Assert.False(testValue.HasFlag(HttpSemanticConvention.Old)); - Assert.True(testValue.HasFlag(HttpSemanticConvention.New)); - } - - [Theory] - [MemberData(nameof(TestCases))] - public void VerifyGetSemanticConventionOptIn_UsingEnvironmentVariable(string input, string expectedValue) - { - try - { - Environment.SetEnvironmentVariable(SemanticConventionOptInKeyName, input); - - var expected = Enum.Parse(typeof(HttpSemanticConvention), 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 { [SemanticConventionOptInKeyName] = input }) - .Build(); - - var expected = Enum.Parse(typeof(HttpSemanticConvention), expectedValue); - Assert.Equal(expected, GetSemanticConventionOptIn(configuration)); - } -}