diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md index 377f145731..f68e4c49d4 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md @@ -30,6 +30,11 @@ 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)) +* When `EnableConnectionLevelAttributes` is enabled, the `server.port` attribute + will now be written as an integer to be compliant with the + [semantic conventions](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/attributes-registry/server.md). + Previously, it was written as a string. + ([#2233](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2233)) ## 1.9.0-beta.1 diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs index f089caf08d..88bf750c9d 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs @@ -26,4 +26,35 @@ internal sealed class SqlActivitySourceHelper { new KeyValuePair(SemanticConventions.AttributeDbSystem, MicrosoftSqlServerDatabaseSystemName), }; + + public static void AddConnectionLevelDetailsToActivity(string dataSource, Activity activity, SqlClientTraceInstrumentationOptions options) + { + // TODO: The attributes added here are required. We need to consider + // collecting these attributes by default. + if (options.EnableConnectionLevelAttributes) + { + var connectionDetails = SqlConnectionDetails.ParseFromDataSource((string)dataSource); + + // TODO: In the new conventions, instance name should now be captured + // as a part of db.namespace, when available. + if (options.EmitOldAttributes && !string.IsNullOrEmpty(connectionDetails.InstanceName)) + { + activity.SetTag(SemanticConventions.AttributeDbMsSqlInstanceName, connectionDetails.InstanceName); + } + + if (!string.IsNullOrEmpty(connectionDetails.ServerHostName)) + { + activity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerHostName); + } + else + { + activity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerIpAddress); + } + + if (connectionDetails.Port.HasValue) + { + activity.SetTag(SemanticConventions.AttributeServerPort, connectionDetails.Port); + } + } + } } diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs index 8594f907f2..b16f71402a 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs @@ -116,7 +116,7 @@ public override void OnEventWritten(string name, object? payload) if (dataSource != null) { - this.options.AddConnectionLevelDetailsToActivity((string)dataSource, activity); + SqlActivitySourceHelper.AddConnectionLevelDetailsToActivity((string)dataSource, activity, this.options); } if (this.commandTypeFetcher.TryFetch(command, out CommandType commandType)) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlConnectionDetails.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlConnectionDetails.cs new file mode 100644 index 0000000000..2e5c595335 --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlConnectionDetails.cs @@ -0,0 +1,130 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Collections.Concurrent; +using System.Text.RegularExpressions; + +namespace OpenTelemetry.Instrumentation.SqlClient.Implementation; + +internal sealed class SqlConnectionDetails +{ + /* + * Match... + * protocol[ ]:[ ]serverName + * serverName + * serverName[ ]\[ ]instanceName + * serverName[ ],[ ]port + * serverName[ ]\[ ]instanceName[ ],[ ]port + * + * [ ] can be any number of white-space, SQL allows it for some reason. + * + * Optional "protocol" can be "tcp", "lpc" (shared memory), or "np" (named pipes). See: + * https://docs.microsoft.com/troubleshoot/sql/connect/use-server-name-parameter-connection-string, and + * https://docs.microsoft.com/dotnet/api/system.data.sqlclient.sqlconnection.connectionstring?view=dotnet-plat-ext-5.0 + * + * In case of named pipes the Data Source string can take form of: + * np:serverName\instanceName, or + * np:\\serverName\pipe\pipeName, or + * np:\\serverName\pipe\MSSQL$instanceName\pipeName - in this case a separate regex (see NamedPipeRegex below) + * is used to extract instanceName + */ + private static readonly Regex DataSourceRegex = new("^(.*\\s*:\\s*\\\\{0,2})?(.*?)\\s*(?:[\\\\,]|$)\\s*(.*?)\\s*(?:,|$)\\s*(.*)$", RegexOptions.Compiled); + + /// + /// In a Data Source string like "np:\\serverName\pipe\MSSQL$instanceName\pipeName" match the + /// "pipe\MSSQL$instanceName" segment to extract instanceName if it is available. + /// + /// + /// + /// + private static readonly Regex NamedPipeRegex = new("pipe\\\\MSSQL\\$(.*?)\\\\", RegexOptions.Compiled); + + private static readonly ConcurrentDictionary ConnectionDetailCache = new(StringComparer.OrdinalIgnoreCase); + + private SqlConnectionDetails() + { + } + + public string? ServerHostName { get; private set; } + + public string? ServerIpAddress { get; private set; } + + public string? InstanceName { get; private set; } + + public int? Port { get; private set; } + + public static SqlConnectionDetails ParseFromDataSource(string dataSource) + { + if (ConnectionDetailCache.TryGetValue(dataSource, out SqlConnectionDetails? connectionDetails)) + { + return connectionDetails; + } + + var match = DataSourceRegex.Match(dataSource); + + string? serverHostName = match.Groups[2].Value; + string? serverIpAddress = null; + string? instanceName = null; + int? port = null; + + var uriHostNameType = Uri.CheckHostName(serverHostName); + if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) + { + serverIpAddress = serverHostName; + serverHostName = null; + } + + string maybeProtocol = match.Groups[1].Value; + bool isNamedPipe = maybeProtocol.Length > 0 && + maybeProtocol.StartsWith("np", StringComparison.OrdinalIgnoreCase); + + if (isNamedPipe) + { + string pipeName = match.Groups[3].Value; + if (pipeName.Length > 0) + { + var namedInstancePipeMatch = NamedPipeRegex.Match(pipeName); + if (namedInstancePipeMatch.Success) + { + instanceName = namedInstancePipeMatch.Groups[1].Value; + } + } + } + else + { + if (match.Groups[4].Length > 0) + { + instanceName = match.Groups[3].Value; + port = int.TryParse(match.Groups[4].Value, out int parsedPort) + ? parsedPort == 1433 ? null : parsedPort + : null; + } + else if (int.TryParse(match.Groups[3].Value, out int parsedPort)) + { + instanceName = null; + port = parsedPort == 1433 ? null : parsedPort; + } + else + { + instanceName = match.Groups[3].Value; + if (string.IsNullOrEmpty(instanceName)) + { + instanceName = null; + } + + port = null; + } + } + + connectionDetails = new SqlConnectionDetails + { + ServerHostName = serverHostName, + ServerIpAddress = serverIpAddress, + InstanceName = instanceName, + Port = port, + }; + + ConnectionDetailCache.TryAdd(dataSource, connectionDetails); + return connectionDetails; + } +} diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs index 8e8a9163d0..0b4085183a 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs @@ -140,7 +140,7 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) activity.SetTag(SemanticConventions.AttributeDbNamespace, databaseName); } - this.options.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity); + SqlActivitySourceHelper.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity, this.options); string commandText = (string)eventData.Payload[3]; if (!string.IsNullOrEmpty(commandText) && this.options.SetDbStatementForText) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs index 4b2cd56c43..48b69212e9 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs @@ -1,10 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System.Collections.Concurrent; using System.Data; using System.Diagnostics; -using System.Text.RegularExpressions; using Microsoft.Extensions.Configuration; using OpenTelemetry.Trace; using static OpenTelemetry.Internal.DatabaseSemanticConventionHelper; @@ -19,39 +17,6 @@ namespace OpenTelemetry.Instrumentation.SqlClient; /// public class SqlClientTraceInstrumentationOptions { - /* - * Match... - * protocol[ ]:[ ]serverName - * serverName - * serverName[ ]\[ ]instanceName - * serverName[ ],[ ]port - * serverName[ ]\[ ]instanceName[ ],[ ]port - * - * [ ] can be any number of white-space, SQL allows it for some reason. - * - * Optional "protocol" can be "tcp", "lpc" (shared memory), or "np" (named pipes). See: - * https://docs.microsoft.com/troubleshoot/sql/connect/use-server-name-parameter-connection-string, and - * https://docs.microsoft.com/dotnet/api/system.data.sqlclient.sqlconnection.connectionstring?view=dotnet-plat-ext-5.0 - * - * In case of named pipes the Data Source string can take form of: - * np:serverName\instanceName, or - * np:\\serverName\pipe\pipeName, or - * np:\\serverName\pipe\MSSQL$instanceName\pipeName - in this case a separate regex (see NamedPipeRegex below) - * is used to extract instanceName - */ - private static readonly Regex DataSourceRegex = new("^(.*\\s*:\\s*\\\\{0,2})?(.*?)\\s*(?:[\\\\,]|$)\\s*(.*?)\\s*(?:,|$)\\s*(.*)$", RegexOptions.Compiled); - - /// - /// In a Data Source string like "np:\\serverName\pipe\MSSQL$instanceName\pipeName" match the - /// "pipe\MSSQL$instanceName" segment to extract instanceName if it is available. - /// - /// - /// - /// - private static readonly Regex NamedPipeRegex = new("pipe\\\\MSSQL\\$(.*?)\\\\", RegexOptions.Compiled); - - private static readonly ConcurrentDictionary ConnectionDetailCache = new(StringComparer.OrdinalIgnoreCase); - /// /// Initializes a new instance of the class. /// @@ -195,134 +160,4 @@ internal SqlClientTraceInstrumentationOptions(IConfiguration configuration) /// 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); - - string? serverHostName = match.Groups[2].Value; - string? serverIpAddress = null; - - string? instanceName; - - var uriHostNameType = Uri.CheckHostName(serverHostName); - if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) - { - serverIpAddress = serverHostName; - serverHostName = null; - } - - string maybeProtocol = match.Groups[1].Value; - bool isNamedPipe = maybeProtocol.Length > 0 && - maybeProtocol.StartsWith("np", StringComparison.OrdinalIgnoreCase); - - if (isNamedPipe) - { - string pipeName = match.Groups[3].Value; - if (pipeName.Length > 0) - { - var namedInstancePipeMatch = NamedPipeRegex.Match(pipeName); - if (namedInstancePipeMatch.Success) - { - instanceName = namedInstancePipeMatch.Groups[1].Value; - return new SqlConnectionDetails - { - ServerHostName = serverHostName, - ServerIpAddress = serverIpAddress, - InstanceName = instanceName, - Port = null, - }; - } - } - - return new SqlConnectionDetails - { - ServerHostName = serverHostName, - ServerIpAddress = serverIpAddress, - InstanceName = null, - Port = null, - }; - } - - string? port; - if (match.Groups[4].Length > 0) - { - instanceName = match.Groups[3].Value; - port = match.Groups[4].Value; - if (port == "1433") - { - port = null; - } - } - else if (int.TryParse(match.Groups[3].Value, out int parsedPort)) - { - port = parsedPort == 1433 ? null : match.Groups[3].Value; - instanceName = null; - } - else - { - instanceName = match.Groups[3].Value; - - if (string.IsNullOrEmpty(instanceName)) - { - instanceName = null; - } - - port = null; - } - - return new SqlConnectionDetails - { - ServerHostName = serverHostName, - ServerIpAddress = serverIpAddress, - InstanceName = instanceName, - Port = port, - }; - } - - internal void AddConnectionLevelDetailsToActivity(string dataSource, Activity sqlActivity) - { - // 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); - } - - // 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); - } - - if (!string.IsNullOrEmpty(connectionDetails.ServerHostName)) - { - sqlActivity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerHostName); - } - else - { - sqlActivity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerIpAddress); - } - - if (!string.IsNullOrEmpty(connectionDetails.Port)) - { - sqlActivity.SetTag(SemanticConventions.AttributeServerPort, connectionDetails.Port); - } - } - } - - internal sealed class SqlConnectionDetails - { - public string? ServerHostName { get; set; } - - public string? ServerIpAddress { get; set; } - - public string? InstanceName { get; set; } - - public string? Port { get; set; } - } } diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs index f2f68aeb71..0328780b18 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs @@ -170,6 +170,59 @@ public void SqlClientCallsAreCollectedSuccessfully( emitNewAttributes); } + [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", 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 SqlClientAddsConnectionLevelAttributes( + bool enableConnectionLevelAttributes, + string dataSource, + string? expectedServerHostName, + string? expectedServerIpAddress, + string? expectedInstanceName, + int? expectedPort, + bool emitOldAttributes = true, + bool emitNewAttributes = false) + { + var activity = new Activity("Test Sql Activity"); + var options = new SqlClientTraceInstrumentationOptions() + { + EnableConnectionLevelAttributes = enableConnectionLevelAttributes, + EmitOldAttributes = emitOldAttributes, + EmitNewAttributes = emitNewAttributes, + }; + SqlActivitySourceHelper.AddConnectionLevelDetailsToActivity(dataSource, activity, options); + + Assert.Equal(expectedServerHostName ?? expectedServerIpAddress, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); + + if (emitOldAttributes) + { + Assert.Equal(expectedInstanceName, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName)); + } + else + { + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName)); + } + + Assert.Equal(expectedPort, activity.GetTagValue(SemanticConventions.AttributeServerPort)); + } + [Theory] [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataWriteCommandError)] [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand, SqlClientDiagnosticListener.SqlDataWriteCommandError, false)] diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs index b85c158134..251109d974 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTraceInstrumentationOptionsTests.cs @@ -1,107 +1,14 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System.Diagnostics; using Microsoft.Extensions.Configuration; using OpenTelemetry.Internal; -using OpenTelemetry.Trace; using Xunit; namespace OpenTelemetry.Instrumentation.SqlClient.Tests; public class SqlClientTraceInstrumentationOptionsTests { - static SqlClientTraceInstrumentationOptionsTests() - { - Activity.DefaultIdFormat = ActivityIdFormat.W3C; - Activity.ForceDefaultIdFormat = true; - - var listener = new ActivityListener - { - ShouldListenTo = _ => true, - Sample = (ref ActivityCreationOptions options) => ActivitySamplingResult.AllData, - }; - - ActivitySource.AddActivityListener(listener); - } - - [Theory] - [InlineData("localhost", "localhost", null, null, null)] - [InlineData("127.0.0.1", null, "127.0.0.1", null, null)] - [InlineData("127.0.0.1,1433", null, "127.0.0.1", null, null)] - [InlineData("127.0.0.1, 1818", null, "127.0.0.1", null, "1818")] - [InlineData("127.0.0.1 \\ instanceName", null, "127.0.0.1", "instanceName", null)] - [InlineData("127.0.0.1\\instanceName, 1818", null, "127.0.0.1", "instanceName", "1818")] - [InlineData("tcp:127.0.0.1\\instanceName, 1818", null, "127.0.0.1", "instanceName", "1818")] - [InlineData("tcp:localhost", "localhost", null, null, null)] - [InlineData("tcp : localhost", "localhost", null, null, null)] - [InlineData("np : localhost", "localhost", null, null, null)] - [InlineData("lpc:localhost", "localhost", null, null, null)] - [InlineData("np:\\\\localhost\\pipe\\sql\\query", "localhost", null, null, null)] - [InlineData("np : \\\\localhost\\pipe\\sql\\query", "localhost", null, null, null)] - [InlineData("np:\\\\localhost\\pipe\\MSSQL$instanceName\\sql\\query", "localhost", null, "instanceName", null)] - public void ParseDataSourceTests( - string dataSource, - string? expectedServerHostName, - string? expectedServerIpAddress, - string? expectedInstanceName, - string? expectedPort) - { - var sqlConnectionDetails = SqlClientTraceInstrumentationOptions.ParseDataSource(dataSource); - - Assert.NotNull(sqlConnectionDetails); - Assert.Equal(expectedServerHostName, sqlConnectionDetails.ServerHostName); - Assert.Equal(expectedServerIpAddress, sqlConnectionDetails.ServerIpAddress); - Assert.Equal(expectedInstanceName, sqlConnectionDetails.InstanceName); - Assert.Equal(expectedPort, sqlConnectionDetails.Port); - } - - [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", 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, - bool emitOldAttributes = true, - bool emitNewAttributes = false) - { - var source = new ActivitySource("sql-client-instrumentation"); - var activity = source.StartActivity("Test Sql Activity"); - Assert.NotNull(activity); - var options = new SqlClientTraceInstrumentationOptions() - { - EnableConnectionLevelAttributes = enableConnectionLevelAttributes, - EmitOldAttributes = emitOldAttributes, - EmitNewAttributes = emitNewAttributes, - }; - options.AddConnectionLevelDetailsToActivity(dataSource, activity); - - 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() { diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlConnectionDetailsTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlConnectionDetailsTests.cs new file mode 100644 index 0000000000..8b232dbe6a --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlConnectionDetailsTests.cs @@ -0,0 +1,41 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using OpenTelemetry.Instrumentation.SqlClient.Implementation; +using Xunit; + +namespace OpenTelemetry.Instrumentation.SqlClient.Tests; + +public class SqlConnectionDetailsTests +{ + [Theory] + [InlineData("localhost", "localhost", null, null, null)] + [InlineData("127.0.0.1", null, "127.0.0.1", null, null)] + [InlineData("127.0.0.1,1433", null, "127.0.0.1", null, null)] + [InlineData("127.0.0.1, 1818", null, "127.0.0.1", null, 1818)] + [InlineData("127.0.0.1 \\ instanceName", null, "127.0.0.1", "instanceName", null)] + [InlineData("127.0.0.1\\instanceName, 1818", null, "127.0.0.1", "instanceName", 1818)] + [InlineData("tcp:127.0.0.1\\instanceName, 1818", null, "127.0.0.1", "instanceName", 1818)] + [InlineData("tcp:localhost", "localhost", null, null, null)] + [InlineData("tcp : localhost", "localhost", null, null, null)] + [InlineData("np : localhost", "localhost", null, null, null)] + [InlineData("lpc:localhost", "localhost", null, null, null)] + [InlineData("np:\\\\localhost\\pipe\\sql\\query", "localhost", null, null, null)] + [InlineData("np : \\\\localhost\\pipe\\sql\\query", "localhost", null, null, null)] + [InlineData("np:\\\\localhost\\pipe\\MSSQL$instanceName\\sql\\query", "localhost", null, "instanceName", null)] + public void ParseFromDataSourceTests( + string dataSource, + string? expectedServerHostName, + string? expectedServerIpAddress, + string? expectedInstanceName, + int? expectedPort) + { + var sqlConnectionDetails = SqlConnectionDetails.ParseFromDataSource(dataSource); + + Assert.NotNull(sqlConnectionDetails); + Assert.Equal(expectedServerHostName, sqlConnectionDetails.ServerHostName); + Assert.Equal(expectedServerIpAddress, sqlConnectionDetails.ServerIpAddress); + Assert.Equal(expectedInstanceName, sqlConnectionDetails.InstanceName); + Assert.Equal(expectedPort, sqlConnectionDetails.Port); + } +} diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs index 9fc2d9e979..1d1c234c85 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlEventSourceTests.netfx.cs @@ -130,7 +130,8 @@ public void EventSourceFakeTests( int objectId = Guid.NewGuid().GetHashCode(); - fakeSqlEventSource.WriteBeginExecuteEvent(objectId, "127.0.0.1", "master", commandType == CommandType.StoredProcedure ? commandText : string.Empty); + var dataSource = "127.0.0.1\\instanceName,port"; + fakeSqlEventSource.WriteBeginExecuteEvent(objectId, dataSource, "master", commandType == CommandType.StoredProcedure ? commandText : string.Empty); // success is stored in the first bit in compositeState 0b001 int successFlag = !isFailure ? 1 : 0; @@ -149,7 +150,7 @@ public void EventSourceFakeTests( var activity = exportedItems[0]; - VerifyActivityData(commandText, captureText, isFailure, "127.0.0.1", activity, enableConnectionLevelAttributes, emitOldAttributes, emitNewAttributes); + VerifyActivityData(commandText, captureText, isFailure, dataSource, activity, enableConnectionLevelAttributes, emitOldAttributes, emitNewAttributes); } [Theory] @@ -248,7 +249,7 @@ private static void VerifyActivityData( if (enableConnectionLevelAttributes) { - var connectionDetails = SqlClientTraceInstrumentationOptions.ParseDataSource(dataSource); + var connectionDetails = SqlConnectionDetails.ParseFromDataSource(dataSource); if (!string.IsNullOrEmpty(connectionDetails.ServerHostName)) { @@ -263,10 +264,14 @@ private static void VerifyActivityData( { Assert.Equal(connectionDetails.InstanceName, activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName)); } + else + { + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbMsSqlInstanceName)); + } - if (!string.IsNullOrEmpty(connectionDetails.Port)) + if (connectionDetails.Port.HasValue) { - Assert.Equal(connectionDetails.Port, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); + Assert.Equal(connectionDetails.Port, activity.GetTagValue(SemanticConventions.AttributeServerPort)); } }