From 07c966d1f3868decf9c63f4590dede828bbab03e Mon Sep 17 00:00:00 2001 From: Jacob Affinito Date: Fri, 6 Dec 2024 14:37:12 -0800 Subject: [PATCH 01/13] Working AWS Redshift PoC. --- .../Attributes/AttributeDefinitionService.cs | 9 +- .../Core/Segments/DatastoreSegmentData.cs | 6 + .../NewRelic.Agent.Extensions.csproj | 1 + .../IConnectionStringParser.cs | 2 + .../OdbcConnectionStringParser.cs | 108 ++++++++++++++++++ .../Parsing/SqlWrapperHelper.cs | 17 ++- .../Providers/Wrapper/Constants.cs | 1 + .../Providers/Wrapper/Sql/Instrumentation.xml | 7 ++ .../Wrapper/Sql/OdbcCommandWrapper.cs | 8 +- .../Providers/Wrapper/Sql/Sql.csproj | 3 + 10 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs diff --git a/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs b/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs index c51ea26b61..b51a3344e6 100644 --- a/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs +++ b/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs @@ -145,8 +145,9 @@ public interface IAttributeDefinitions AttributeDefinition MessageRoutingKey { get; } AttributeDefinition MessagingRabbitMqDestinationRoutingKey { get; } AttributeDefinition MessagingDestinationPublishName { get; } - } + AttributeDefinition ConfigurationEndpointAddress { get; } + } public class AttributeDefinitionService : ConfigurationBasedService, IAttributeDefinitionService { @@ -597,6 +598,12 @@ public AttributeDefinition GetTypeAttribute(TypeAttr .AppliesTo(AttributeDestinations.SpanEvent) .Build(_attribFilter)); + private AttributeDefinition _configurationEndpointAddress; + public AttributeDefinition ConfigurationEndpointAddress => _configurationEndpointAddress ?? (_configurationEndpointAddress = + AttributeDefinitionBuilder.CreateString("configuration.endpoint.address", AttributeClassification.AgentAttributes) + .AppliesTo(AttributeDestinations.SpanEvent) + .Build(_attribFilter)); + private AttributeDefinition _peerAddress; public AttributeDefinition PeerAddress => _peerAddress ?? (_peerAddress = AttributeDefinitionBuilder.CreateString("peer.address", AttributeClassification.AgentAttributes) diff --git a/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs b/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs index a1afd2162e..4bd533cbb0 100644 --- a/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs +++ b/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs @@ -233,6 +233,12 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection { AttribDefs.DbServerPort.TrySetValue(attribVals, Port.Value); } + + // For AWS Redshift relationship + if (Host.EndsWith(".redshift.amazonaws.com")) + { + AttribDefs.ConfigurationEndpointAddress.TrySetValue(attribVals, Host); + } } public void SetConnectionInfo(ConnectionInfo connInfo) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj index 782fb916be..7984aa046c 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj @@ -15,6 +15,7 @@ + diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs index aa3ec16a74..bc0d1fc938 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs @@ -43,6 +43,8 @@ private static IConnectionStringParser GetConnectionParser(DatastoreVendor vendo return new IbmDb2ConnectionStringParser(connectionString); case DatastoreVendor.Redis: return new StackExchangeRedisConnectionStringParser(connectionString); + case DatastoreVendor.ODBC: + return new OdbcConnectionStringParser(connectionString); default: return null; } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs new file mode 100644 index 0000000000..7b944b7088 --- /dev/null +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs @@ -0,0 +1,108 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +using NewRelic.Agent.Extensions.Providers.Wrapper; +using System.Collections.Generic; +using System.Data.Common; +using System.Linq; + +namespace NewRelic.Agent.Extensions.Parsing.ConnectionString +{ + public class OdbcConnectionStringParser : IConnectionStringParser + { + private static readonly List _hostKeys = new List { "server", "data source" }; + private static readonly List _portKeys = new List { "port" }; + private static readonly List _databaseNameKeys = new List { "database" }; + + private readonly DbConnectionStringBuilder _connectionStringBuilder; + + public OdbcConnectionStringParser(string connectionString) + { + _connectionStringBuilder = new DbConnectionStringBuilder { ConnectionString = connectionString }; + } + + public ConnectionInfo GetConnectionInfo(string utilizationHostName) + { + var host = ParseHost(); + if (host != null) host = ConnectionStringParserHelper.NormalizeHostname(host, utilizationHostName); + var portPathOrId = ParsePortPathOrId(); + var databaseName = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _databaseNameKeys)?.Value; + var instanceName = ParseInstanceName(); + return new ConnectionInfo(host, portPathOrId, databaseName, instanceName); + } + + private string ParseHost() + { + var host = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; + if (host == null) return null; + + // Example of want we would need to process: win-database.pdx.vm.datanerd.us,1433\SQLEXPRESS + try + { + var splitIndex = host.IndexOf(','); + if (splitIndex == -1) splitIndex = host.IndexOf('\\'); + host = splitIndex == -1 ? host : host.Substring(0, splitIndex); + } + catch + { + return null; + } + return host; + } + + private string ParsePortPathOrId() + { + // Some ODBC drivers use the "port" key to specify the port number + var portPathOrId = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _portKeys)?.Value; + if (!string.IsNullOrWhiteSpace(portPathOrId)) + { + return portPathOrId; + + } + + // Some ODBC drivers include the port in the "server" or "data source" key + portPathOrId = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; + if (portPathOrId == null) return null; + + try + { + if (portPathOrId.IndexOf(',') != -1) + { + var startOfValue = portPathOrId.IndexOf(',') + 1; + var endOfValue = portPathOrId.Contains('\\') + ? portPathOrId.IndexOf('\\') + : portPathOrId.Length; + return (startOfValue > 0) ? portPathOrId.Substring(startOfValue, endOfValue - startOfValue) : null; + } + } + catch + { + return null; + } + + return "default"; + } + + private string ParseInstanceName() + { + var instanceName = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; + if (instanceName == null) return null; + + try + { + if (instanceName.IndexOf('\\') != -1) + { + var startOfValue = instanceName.IndexOf('\\') + 1; + var endOfValue = instanceName.Length; + return (startOfValue > 0) ? instanceName.Substring(startOfValue, endOfValue - startOfValue) : null; + } + } + catch + { + return null; + } + + return null; + } + } +} diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs index 8344c4cd67..a478e63c95 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs @@ -6,8 +6,8 @@ using System.Data; using System.Data.SqlTypes; using System.Globalization; -#if NETFRAMEWORK using System.Data.Odbc; +#if NETFRAMEWORK using System.Data.OleDb; #endif using NewRelic.Agent.Api; @@ -28,17 +28,17 @@ public static class SqlWrapperHelper public static DatastoreVendor GetVendorName(IDbCommand command) { -#if NETFRAMEWORK // If this is an OdbcCommand, the only way to give the data store name is by looking at the connection driver var odbcCommand = command as OdbcCommand; if (odbcCommand != null && odbcCommand.Connection != null) return ExtractVendorNameFromString(odbcCommand.Connection.Driver); - - // If this is an OleDbCommand, the only way to give the data store name is by looking at the connection provider - var oleCommand = command as OleDbCommand; +#if NETFRAMEWORK + // If this is an OleDbCommand, the only way to give the data store name is by looking at the connection provider + var oleCommand = command as OleDbCommand; if (oleCommand != null && oleCommand.Connection != null) return ExtractVendorNameFromString(oleCommand.Connection.Provider); + #endif return GetVendorName(command.GetType().Name); } @@ -61,7 +61,7 @@ public static DatastoreVendor GetVendorName(string typeName) { "OracleCommand", DatastoreVendor.Oracle }, { "OracleDatabase", DatastoreVendor.Oracle }, { "NpgsqlCommand", DatastoreVendor.Postgres }, - { "DB2Command", DatastoreVendor.IBMDB2 }, + { "DB2Command", DatastoreVendor.IBMDB2 } }; /// @@ -86,6 +86,11 @@ private static DatastoreVendor ExtractVendorNameFromString(string text) if (text.IndexOf("DB2", StringComparison.OrdinalIgnoreCase) != -1 || text.IndexOf("IBM", StringComparison.OrdinalIgnoreCase) != -1) return DatastoreVendor.IBMDB2; + // This works for redshift since the driver reports as "Amazon Redshift ODBC Driver" + // Other drivers may not report as expected + if (text.IndexOf("ODBC", StringComparison.OrdinalIgnoreCase) != -1) + return DatastoreVendor.ODBC; + return DatastoreVendor.Other; } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Providers/Wrapper/Constants.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Providers/Wrapper/Constants.cs index 116c412f80..a3b7f593a8 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Providers/Wrapper/Constants.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Providers/Wrapper/Constants.cs @@ -81,6 +81,7 @@ public enum DatastoreVendor CosmosDB, Elasticsearch, DynamoDB, + ODBC, Other } diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Instrumentation.xml b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Instrumentation.xml index f75714b70e..b84cfe3879 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Instrumentation.xml +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Instrumentation.xml @@ -13,6 +13,13 @@ SPDX-License-Identifier: Apache-2.0 + + + + + + + diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs index db9dffa42c..daeb0dfc67 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs @@ -1,12 +1,12 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -#if NETFRAMEWORK using System; using System.Data.Odbc; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Extensions.Parsing; +using NewRelic.Agent.Extensions.Parsing.ConnectionString; namespace NewRelic.Providers.Wrapper.Sql { @@ -30,15 +30,17 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins var sql = odbcCommand.CommandText ?? string.Empty; var vendor = SqlWrapperHelper.GetVendorName(odbcCommand); + object GetConnectionInfo() => ConnectionInfoParser.FromConnectionString(vendor, odbcCommand.Connection.ConnectionString, agent.Configuration.UtilizationHostName); + var connectionInfo = (ConnectionInfo)transaction.GetOrSetValueFromCache(odbcCommand.Connection.ConnectionString, GetConnectionInfo); + var parsedStatement = transaction.GetParsedDatabaseStatement(vendor, odbcCommand.CommandType, sql); var queryParameters = SqlWrapperHelper.GetQueryParameters(odbcCommand, agent); - var segment = transaction.StartDatastoreSegment(instrumentedMethodCall.MethodCall, parsedStatement, null, sql, queryParameters); + var segment = transaction.StartDatastoreSegment(instrumentedMethodCall.MethodCall, parsedStatement, connectionInfo, sql, queryParameters); return Delegates.GetDelegateFor(segment); } } } } -#endif diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj index c2306d69e0..cf0a58c318 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj @@ -13,6 +13,9 @@ + + + From c16edf399a9028ddd29d03fa07b3cab63ade25ac Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Tue, 10 Dec 2024 11:02:05 -0800 Subject: [PATCH 02/13] Correct attribute name for Redshift server address --- .../Agent/Core/Segments/DatastoreSegmentData.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs b/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs index 4bd533cbb0..707de51eed 100644 --- a/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs +++ b/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs @@ -234,11 +234,12 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection AttribDefs.DbServerPort.TrySetValue(attribVals, Port.Value); } - // For AWS Redshift relationship - if (Host.EndsWith(".redshift.amazonaws.com")) - { - AttribDefs.ConfigurationEndpointAddress.TrySetValue(attribVals, Host); - } + // This shouldn't be necessary since we're already setting DbServerAddress above + //// For AWS Redshift relationship + //if (Host.EndsWith(".redshift.amazonaws.com")) + //{ + // AttribDefs.ConfigurationEndpointAddress.TrySetValue(attribVals, Host); + //} } public void SetConnectionInfo(ConnectionInfo connInfo) From 651c8113d1ea09697088fab9fd82929f15ede9b6 Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Tue, 10 Dec 2024 11:11:56 -0800 Subject: [PATCH 03/13] Remove new attribute definition that turned out to be unnecessary --- .../Agent/Core/Attributes/AttributeDefinitionService.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs b/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs index b51a3344e6..46b88db1d1 100644 --- a/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs +++ b/src/Agent/NewRelic/Agent/Core/Attributes/AttributeDefinitionService.cs @@ -145,8 +145,6 @@ public interface IAttributeDefinitions AttributeDefinition MessageRoutingKey { get; } AttributeDefinition MessagingRabbitMqDestinationRoutingKey { get; } AttributeDefinition MessagingDestinationPublishName { get; } - - AttributeDefinition ConfigurationEndpointAddress { get; } } public class AttributeDefinitionService : ConfigurationBasedService, IAttributeDefinitionService @@ -598,12 +596,6 @@ public AttributeDefinition GetTypeAttribute(TypeAttr .AppliesTo(AttributeDestinations.SpanEvent) .Build(_attribFilter)); - private AttributeDefinition _configurationEndpointAddress; - public AttributeDefinition ConfigurationEndpointAddress => _configurationEndpointAddress ?? (_configurationEndpointAddress = - AttributeDefinitionBuilder.CreateString("configuration.endpoint.address", AttributeClassification.AgentAttributes) - .AppliesTo(AttributeDestinations.SpanEvent) - .Build(_attribFilter)); - private AttributeDefinition _peerAddress; public AttributeDefinition PeerAddress => _peerAddress ?? (_peerAddress = AttributeDefinitionBuilder.CreateString("peer.address", AttributeClassification.AgentAttributes) From 644555a2740c4f71e3e5ca6ac12fb7f46dc0591c Mon Sep 17 00:00:00 2001 From: Alex Hemsath <57361211+nr-ahemsath@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:23:11 -0800 Subject: [PATCH 04/13] Remove references to System.Data.Odbc from wrapper and extensions (#2928) * Remove references to System.Data.Odbc from wrapper and extentions * PR feedback --- .../NewRelic.Agent.Extensions.csproj | 1 - .../Parsing/SqlWrapperHelper.cs | 34 +++++++++--- .../Wrapper/Sql/OdbcCommandWrapper.cs | 54 ++++++++++--------- .../Providers/Wrapper/Sql/Sql.csproj | 3 -- 4 files changed, 54 insertions(+), 38 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj index 7984aa046c..782fb916be 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/NewRelic.Agent.Extensions.csproj @@ -15,7 +15,6 @@ - diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs index a478e63c95..d109399ee2 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs @@ -2,11 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Data; using System.Data.SqlTypes; using System.Globalization; -using System.Data.Odbc; +using System.Text.RegularExpressions; #if NETFRAMEWORK using System.Data.OleDb; #endif @@ -19,6 +20,9 @@ public static class SqlWrapperHelper { private const string NullQueryParameterValue = "Null"; + private static Regex _getDriverFromConnectionStringRegex = new Regex(@"DRIVER\=\{(.+?)\}"); + private static ConcurrentDictionary _vendorNameCache = new ConcurrentDictionary(); + /// /// Gets the name of the datastore being used by a dbCommand. /// @@ -28,21 +32,35 @@ public static class SqlWrapperHelper public static DatastoreVendor GetVendorName(IDbCommand command) { - // If this is an OdbcCommand, the only way to give the data store name is by looking at the connection driver - - var odbcCommand = command as OdbcCommand; - if (odbcCommand != null && odbcCommand.Connection != null) - return ExtractVendorNameFromString(odbcCommand.Connection.Driver); #if NETFRAMEWORK // If this is an OleDbCommand, the only way to give the data store name is by looking at the connection provider var oleCommand = command as OleDbCommand; - if (oleCommand != null && oleCommand.Connection != null) - return ExtractVendorNameFromString(oleCommand.Connection.Provider); + if (oleCommand != null && oleCommand.Connection != null) + return ExtractVendorNameFromString(oleCommand.Connection.Provider); #endif return GetVendorName(command.GetType().Name); } + public static DatastoreVendor GetVendorNameFromOdbcConnectionString(string connectionString) + { + // Example connection string: DRIVER={SQL Server Native Client 11.0};Server=127.0.0.1;Database=NewRelic;Trusted_Connection=no;UID=sa;PWD=MssqlPassw0rd;Encrypt=no; + if (_vendorNameCache.TryGetValue(connectionString, out DatastoreVendor vendor)) + { + return vendor; + } + + var match = _getDriverFromConnectionStringRegex.Match(connectionString); + if (match.Success) + { + var driver = match.Groups[1].Value; + vendor = ExtractVendorNameFromString(driver); + _vendorNameCache[connectionString] = vendor; + return vendor; + } + return DatastoreVendor.ODBC; // or maybe Other? + } + public static DatastoreVendor GetVendorName(string typeName) { diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs index daeb0dfc67..dbdcbc78e2 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/OdbcCommandWrapper.cs @@ -2,45 +2,47 @@ // SPDX-License-Identifier: Apache-2.0 using System; -using System.Data.Odbc; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Extensions.Parsing; using NewRelic.Agent.Extensions.Parsing.ConnectionString; +using System.Data; namespace NewRelic.Providers.Wrapper.Sql { - public class OdbcCommandWrapper : IWrapper - { - public const string WrapperName = "OdbcCommandTracer"; - public bool IsTransactionRequired => true; + public class OdbcCommandWrapper : IWrapper + { + public const string WrapperName = "OdbcCommandTracer"; + public bool IsTransactionRequired => true; - public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) - { - return new CanWrapResponse(methodInfo.RequestedWrapperName.Equals(WrapperName, StringComparison.OrdinalIgnoreCase)); - } + public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) + { + return new CanWrapResponse(methodInfo.RequestedWrapperName.Equals(WrapperName, StringComparison.OrdinalIgnoreCase)); + } - public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) - { - { - var odbcCommand = (OdbcCommand)instrumentedMethodCall.MethodCall.InvocationTarget; - if (odbcCommand == null) - return Delegates.NoOp; + public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) + { + { + if (instrumentedMethodCall.MethodCall.InvocationTarget is not IDbCommand odbcCommand) + { + return Delegates.NoOp; + } - var sql = odbcCommand.CommandText ?? string.Empty; - var vendor = SqlWrapperHelper.GetVendorName(odbcCommand); + var sql = odbcCommand.CommandText; - object GetConnectionInfo() => ConnectionInfoParser.FromConnectionString(vendor, odbcCommand.Connection.ConnectionString, agent.Configuration.UtilizationHostName); - var connectionInfo = (ConnectionInfo)transaction.GetOrSetValueFromCache(odbcCommand.Connection.ConnectionString, GetConnectionInfo); + var vendor = SqlWrapperHelper.GetVendorNameFromOdbcConnectionString(odbcCommand.Connection.ConnectionString); - var parsedStatement = transaction.GetParsedDatabaseStatement(vendor, odbcCommand.CommandType, sql); + object GetConnectionInfo() => ConnectionInfoParser.FromConnectionString(vendor, odbcCommand.Connection.ConnectionString, agent.Configuration.UtilizationHostName); + var connectionInfo = (ConnectionInfo)transaction.GetOrSetValueFromCache(odbcCommand.Connection.ConnectionString, GetConnectionInfo); - var queryParameters = SqlWrapperHelper.GetQueryParameters(odbcCommand, agent); + var parsedStatement = transaction.GetParsedDatabaseStatement(vendor, odbcCommand.CommandType, sql); - var segment = transaction.StartDatastoreSegment(instrumentedMethodCall.MethodCall, parsedStatement, connectionInfo, sql, queryParameters); + var queryParameters = SqlWrapperHelper.GetQueryParameters(odbcCommand, agent); - return Delegates.GetDelegateFor(segment); - } - } - } + var segment = transaction.StartDatastoreSegment(instrumentedMethodCall.MethodCall, parsedStatement, connectionInfo, sql, queryParameters); + + return Delegates.GetDelegateFor(segment); + } + } + } } diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj index cf0a58c318..c2306d69e0 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/Sql/Sql.csproj @@ -13,9 +13,6 @@ - - - From e353db6419180bbe48510644190717f07b8cf4ad Mon Sep 17 00:00:00 2001 From: Alex Hemsath <57361211+nr-ahemsath@users.noreply.github.com> Date: Mon, 13 Jan 2025 09:42:03 -0800 Subject: [PATCH 05/13] ODBC integration tests (#2941) * SqlWrapperHelper unit tests * Working integration tests for ODBC instrumentation * Remove commented-out copy-pasted code * PR feedback --- .../Parsing/SqlWrapperHelper.cs | 6 +- .../MFALatestPackages.csproj | 5 +- .../MultiFunctionApplicationHelpers.csproj | 6 + .../MsSql/MicrosoftDataSqlClientExerciser.cs | 20 +- .../MsSql/SystemDataExerciser.cs | 55 +--- .../MsSql/SystemDataOdbcExerciser.cs | 283 ++++++++++++++++++ .../MsSql/MsSqlStoredProcedureTests.cs | 2 +- ...sSqlStoredProcedureUsingOdbcDriverTests.cs | 44 +-- .../MsSql/MsSqlTests.cs | 113 ++++++- .../Parsing/SqlWrapperHelperTests.cs | 18 +- 10 files changed, 461 insertions(+), 91 deletions(-) create mode 100644 tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataOdbcExerciser.cs diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs index d109399ee2..605dcca21e 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs @@ -20,7 +20,7 @@ public static class SqlWrapperHelper { private const string NullQueryParameterValue = "Null"; - private static Regex _getDriverFromConnectionStringRegex = new Regex(@"DRIVER\=\{(.+?)\}"); + private static Regex _getDriverFromConnectionStringRegex = new Regex(@"DRIVER\=\{(.+?)\}", RegexOptions.IgnoreCase | RegexOptions.Compiled); private static ConcurrentDictionary _vendorNameCache = new ConcurrentDictionary(); /// @@ -44,7 +44,7 @@ public static DatastoreVendor GetVendorName(IDbCommand command) public static DatastoreVendor GetVendorNameFromOdbcConnectionString(string connectionString) { - // Example connection string: DRIVER={SQL Server Native Client 11.0};Server=127.0.0.1;Database=NewRelic;Trusted_Connection=no;UID=sa;PWD=MssqlPassw0rd;Encrypt=no; + // Example connection string: DRIVER={SQL Server Native Client 11.0};Server=127.0.0.1;Database=NewRelic;Trusted_Connection=no;UID=sa;PWD=password;Encrypt=no; if (_vendorNameCache.TryGetValue(connectionString, out DatastoreVendor vendor)) { return vendor; @@ -58,7 +58,7 @@ public static DatastoreVendor GetVendorNameFromOdbcConnectionString(string conne _vendorNameCache[connectionString] = vendor; return vendor; } - return DatastoreVendor.ODBC; // or maybe Other? + return DatastoreVendor.ODBC; } public static DatastoreVendor GetVendorName(string typeName) diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj index 8defe6f000..3a8b128e61 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj @@ -94,9 +94,12 @@ - + + + + \ No newline at end of file diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj index 2d9179261f..68439e537e 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj @@ -48,6 +48,12 @@ + + + + + + diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/MicrosoftDataSqlClientExerciser.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/MicrosoftDataSqlClientExerciser.cs index 1c7cf249c7..b8692b2dce 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/MicrosoftDataSqlClientExerciser.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/MicrosoftDataSqlClientExerciser.cs @@ -18,6 +18,8 @@ namespace MultiFunctionApplicationHelpers.NetStandardLibraries.MsSql [Library] public class MicrosoftDataSqlClientExerciser : MsSqlExerciserBase { + private static string _connectionString = MsSqlConfiguration.MsSqlConnectionString; + [LibraryMethod] [Transaction] @@ -26,7 +28,7 @@ public string MsSql(string tableName) { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -76,7 +78,7 @@ public async Task MsSqlAsync(string tableName) { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { await connection.OpenAsync(); @@ -125,7 +127,7 @@ public string MsSqlWithParameterizedQuery(bool paramsWithAtSign) { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -156,7 +158,7 @@ public async Task MsSqlAsync_WithParameterizedQuery(bool paramsWithAtSig { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { await connection.OpenAsync(); @@ -192,7 +194,7 @@ private void ExecuteParameterizedStoredProcedure(string procedureName, bool para { EnsureProcedure(procedureName, DbParameterData.MsSqlParameters); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) using (var command = new SqlCommand(procedureName, connection)) { connection.Open(); @@ -213,7 +215,7 @@ private void ExecuteParameterizedStoredProcedure(string procedureName, bool para [LibraryMethod] public void CreateTable(string tableName) { - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -230,7 +232,7 @@ public void DropTable(string tableName) { var dropTableSql = string.Format(DropPersonTableMsSql, tableName); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -246,7 +248,7 @@ public void DropProcedure(string procedureName) { var dropProcedureSql = string.Format(DropProcedureSql, procedureName); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -267,7 +269,7 @@ private void EnsureProcedure(string procedureName, DbParameter[] dbParameters) { var parameters = string.Join(", ", dbParameters.Select(x => $"{x.ParameterName} {x.DbTypeName}")); var statement = string.Format(CreateProcedureStatement, procedureName, parameters); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) using (var command = new SqlCommand(statement, connection)) { connection.Open(); diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataExerciser.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataExerciser.cs index fbde388fd3..d787529032 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataExerciser.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataExerciser.cs @@ -14,13 +14,15 @@ using NewRelic.Api.Agent; using System.Threading; using System.Data.OleDb; -using System.Data.Odbc; namespace MultiFunctionApplicationHelpers.NetStandardLibraries.MsSql { [Library] public class SystemDataExerciser : MsSqlExerciserBase { + + private static string _connectionString = MsSqlConfiguration.MsSqlConnectionString; + [LibraryMethod] [Transaction] [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] @@ -28,7 +30,7 @@ public string MsSql(string tableName) { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -78,7 +80,7 @@ public async Task MsSqlAsync(string tableName) { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { await connection.OpenAsync(); @@ -127,7 +129,7 @@ public string MsSqlWithParameterizedQuery(bool paramsWithAtSign) { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -159,7 +161,7 @@ public async Task MsSqlAsync_WithParameterizedQuery(bool paramsWithAtSig { var teamMembers = new List(); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { await connection.OpenAsync(); @@ -195,7 +197,7 @@ public void MsSqlParameterizedStoredProcedure(string procedureNameWith, string p private int ExecuteParameterizedStoredProcedure(string procedureName, bool paramsWithAtSign) { EnsureProcedure(procedureName, DbParameterData.MsSqlParameters); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) using (var command = new SqlCommand(procedureName, connection)) { connection.Open(); @@ -213,39 +215,6 @@ private int ExecuteParameterizedStoredProcedure(string procedureName, bool param } } - [LibraryMethod] - [Transaction] - [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] - public void MsSqlParameterizedStoredProcedureUsingOdbcDriver(string procedureNameWith, string procedureNameWithout) - { - ExecuteParameterizedStoredProcedureUsingOdbcDriver(procedureNameWith, true); - ExecuteParameterizedStoredProcedureUsingOdbcDriver(procedureNameWithout, false); - } - - private void ExecuteParameterizedStoredProcedureUsingOdbcDriver(string procedureName, bool paramsWithAtSign) - { - EnsureProcedure(procedureName, DbParameterData.OdbcMsSqlParameters); - - var parameterPlaceholder = string.Join(",", DbParameterData.OdbcMsSqlParameters.Select(_ => "?")); - - using (var connection = new OdbcConnection(MsSqlOdbcConfiguration.MsSqlOdbcConnectionString)) - using (var command = new OdbcCommand($"{{call {procedureName}({parameterPlaceholder})}}", connection)) - { - connection.Open(); - command.CommandType = CommandType.StoredProcedure; - foreach (var parameter in DbParameterData.OdbcMsSqlParameters) - { - var paramName = paramsWithAtSign - ? parameter.ParameterName - : parameter.ParameterName.TrimStart('@'); - - command.Parameters.Add(new OdbcParameter(paramName, parameter.Value)); ; - } - - command.ExecuteNonQuery(); - } - } - [LibraryMethod] [Transaction] [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] @@ -280,7 +249,7 @@ private void ExecuteParameterizedStoredProcedureUsingOleDbDriver(string procedur [LibraryMethod] public void CreateTable(string tableName) { - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -297,7 +266,7 @@ public void DropTable(string tableName) { var dropTableSql = string.Format(DropPersonTableMsSql, tableName); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -313,7 +282,7 @@ public void DropProcedure(string procedureName) { var dropProcedureSql = string.Format(DropProcedureSql, procedureName); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) { connection.Open(); @@ -334,7 +303,7 @@ private void EnsureProcedure(string procedureName, DbParameter[] dbParameters) { var parameters = string.Join(", ", dbParameters.Select(x => $"{x.ParameterName} {x.DbTypeName}")); var statement = string.Format(CreateProcedureStatement, procedureName, parameters); - using (var connection = new SqlConnection(MsSqlConfiguration.MsSqlConnectionString)) + using (var connection = new SqlConnection(_connectionString)) using (var command = new SqlCommand(statement, connection)) { connection.Open(); diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataOdbcExerciser.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataOdbcExerciser.cs new file mode 100644 index 0000000000..befe24097e --- /dev/null +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/MsSql/SystemDataOdbcExerciser.cs @@ -0,0 +1,283 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using System.Data; +using NewRelic.Agent.IntegrationTests.Shared; +using NewRelic.Agent.IntegrationTests.Shared.ReflectionHelpers; +using NewRelic.Api.Agent; +using System.Threading; +using System.Data.Odbc; + +namespace MultiFunctionApplicationHelpers.NetStandardLibraries.MsSql +{ + + [Library] + public class SystemDataOdbcExerciser : MsSqlExerciserBase + { + private static string _connectionString = MsSqlOdbcConfiguration.MsSqlOdbcConnectionString; + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public string MsSql(string tableName) + { + var teamMembers = new List(); + + using (var connection = new OdbcConnection(_connectionString)) + { + connection.Open(); + + using (var command = new OdbcCommand(SelectPersonByFirstNameMsSql, connection)) + { + + using (var reader = command.ExecuteReader()) + { + while (reader.Read()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + if (reader.NextResult()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + } + } + } + } + + var insertSql = string.Format(InsertPersonMsSql, tableName); + var countSql = string.Format(CountPersonMsSql, tableName); + var deleteSql = string.Format(DeletePersonMsSql, tableName); + + using (var command = new OdbcCommand(insertSql, connection)) + { + var insertCount = command.ExecuteNonQuery(); + } + + using (var command = new OdbcCommand(countSql, connection)) + { + var teamMemberCount = command.ExecuteScalar(); + } + + using (var command = new OdbcCommand(deleteSql, connection)) + { + var deleteCount = command.ExecuteNonQuery(); + } + } + + return string.Join(",", teamMembers); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public async Task MsSqlAsync(string tableName) + { + var teamMembers = new List(); + + using (var connection = new OdbcConnection(_connectionString)) + { + await connection.OpenAsync(); + + using (var command = new OdbcCommand(SelectPersonByLastNameMsSql, connection)) + { + using (var reader = await command.ExecuteReaderAsync()) + { + while (await reader.ReadAsync()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + if (await reader.NextResultAsync()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + } + } + } + } + + var insertSql = string.Format(InsertPersonMsSql, tableName); + var countSql = string.Format(CountPersonMsSql, tableName); + var deleteSql = string.Format(DeletePersonMsSql, tableName); + + using (var command = new OdbcCommand(insertSql, connection)) + { + var insertCount = await command.ExecuteNonQueryAsync(); + } + + using (var command = new OdbcCommand(countSql, connection)) + { + var teamMemberCount = await command.ExecuteScalarAsync(); + } + + using (var command = new OdbcCommand(deleteSql, connection)) + { + var deleteCount = await command.ExecuteNonQueryAsync(); + } + } + + return string.Join(",", teamMembers); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public string MsSqlWithParameterizedQuery(bool paramsWithAtSign) + { + var teamMembers = new List(); + + using (var connection = new OdbcConnection(_connectionString)) + { + connection.Open(); + + using (var command = new OdbcCommand(SelectPersonByParameterizedFirstNameMsSql, connection)) + { + command.Parameters.Add(new OdbcParameter(paramsWithAtSign ? "@FN" : "FN", "O'Keefe")); + using (var reader = command.ExecuteReader()) + { + while (reader.Read()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + if (reader.NextResult()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + } + } + } + } + + } + + return string.Join(",", teamMembers); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public async Task MsSqlAsync_WithParameterizedQuery(bool paramsWithAtSign) + { + var teamMembers = new List(); + + using (var connection = new OdbcConnection(_connectionString)) + { + await connection.OpenAsync(); + + using (var command = new OdbcCommand(SelectPersonByParameterizedLastNameMsSql, connection)) + { + command.Parameters.Add(new OdbcParameter(paramsWithAtSign ? "@LN" : "LN", "Lee")); + using (var reader = await command.ExecuteReaderAsync()) + { + while (await reader.ReadAsync()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + if (await reader.NextResultAsync()) + { + teamMembers.Add(reader.GetString(reader.GetOrdinal("FirstName"))); + } + } + } + } + } + + return string.Join(",", teamMembers); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public void OdbcParameterizedStoredProcedure(string procedureNameWith, string procedureNameWithout) + { + ExecuteOdbcParameterizedStoredProcedure(procedureNameWith, true); + ExecuteOdbcParameterizedStoredProcedure(procedureNameWithout, false); + } + + private void ExecuteOdbcParameterizedStoredProcedure(string procedureName, bool paramsWithAtSign) + { + EnsureProcedure(procedureName, DbParameterData.OdbcMsSqlParameters); + + var parameterPlaceholder = string.Join(",", DbParameterData.OdbcMsSqlParameters.Select(_ => "?")); + + using (var connection = new OdbcConnection(MsSqlOdbcConfiguration.MsSqlOdbcConnectionString)) + using (var command = new OdbcCommand($"{{call {procedureName}({parameterPlaceholder})}}", connection)) + { + connection.Open(); + command.CommandType = CommandType.StoredProcedure; + foreach (var parameter in DbParameterData.OdbcMsSqlParameters) + { + var paramName = paramsWithAtSign + ? parameter.ParameterName + : parameter.ParameterName.TrimStart('@'); + + command.Parameters.Add(new OdbcParameter(paramName, parameter.Value)); ; + } + + command.ExecuteNonQuery(); + } + } + + [LibraryMethod] + public void CreateTable(string tableName) + { + using (var connection = new OdbcConnection(_connectionString)) + { + connection.Open(); + + var createTable = string.Format(CreatePersonTableMsSql, tableName); + using (var command = new OdbcCommand(createTable, connection)) + { + command.ExecuteNonQuery(); + } + } + } + + [LibraryMethod] + public void DropTable(string tableName) + { + var dropTableSql = string.Format(DropPersonTableMsSql, tableName); + + using (var connection = new OdbcConnection(_connectionString)) + { + connection.Open(); + + using (var command = new OdbcCommand(dropTableSql, connection)) + { + command.ExecuteNonQuery(); + } + } + } + + [LibraryMethod] + public void DropProcedure(string procedureName) + { + var dropProcedureSql = string.Format(DropProcedureSql, procedureName); + + using (var connection = new OdbcConnection(_connectionString)) + { + connection.Open(); + + using (var command = new OdbcCommand(dropProcedureSql, connection)) + { + command.ExecuteNonQuery(); + } + } + } + + [LibraryMethod] + public void Wait(int millisecondsTimeOut) + { + Thread.Sleep(millisecondsTimeOut); + } + + private void EnsureProcedure(string procedureName, DbParameter[] dbParameters) + { + var parameters = string.Join(", ", dbParameters.Select(x => $"{x.ParameterName} {x.DbTypeName}")); + var statement = string.Format(CreateProcedureStatement, procedureName, parameters); + using (var connection = new OdbcConnection(_connectionString)) + using (var command = new OdbcCommand(statement, connection)) + { + connection.Open(); + command.ExecuteNonQuery(); + } + } + } +} diff --git a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureTests.cs b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureTests.cs index db8564e326..d2ab44758b 100644 --- a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureTests.cs +++ b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureTests.cs @@ -76,7 +76,7 @@ public void Test() new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{_procNameWith.ToLower()}/ExecuteProcedure", callCount = 1 }, new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{_procNameWith.ToLower()}/ExecuteProcedure", callCount = 1, metricScope = _expectedTransactionName }, new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{_procNameWithout.ToLower()}/ExecuteProcedure", callCount = 1 }, - new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{_procNameWithout.ToLower()}/ExecuteProcedure", callCount = 1, metricScope = _expectedTransactionName } + new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{_procNameWithout.ToLower()}/ExecuteProcedure", callCount = 1, metricScope = _expectedTransactionName } }; var expectedTransactionTraceSegments = new List diff --git a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureUsingOdbcDriverTests.cs b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureUsingOdbcDriverTests.cs index 794afe1019..b4dce7a176 100644 --- a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureUsingOdbcDriverTests.cs +++ b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlStoredProcedureUsingOdbcDriverTests.cs @@ -29,7 +29,7 @@ public MsSqlStoredProcedureUsingOdbcDriverTestsBase(TFixture fixture, ITestOutpu _fixture = fixture; _fixture.TestLogger = output; - _expectedTransactionName = $"OtherTransaction/Custom/MultiFunctionApplicationHelpers.NetStandardLibraries.MsSql.{excerciserName}/MsSqlParameterizedStoredProcedureUsingOdbcDriver"; + _expectedTransactionName = $"OtherTransaction/Custom/MultiFunctionApplicationHelpers.NetStandardLibraries.MsSql.{excerciserName}/OdbcParameterizedStoredProcedure"; _tableName = Utilities.GenerateTableName(); var procedureName = Utilities.GenerateProcedureName(); @@ -38,7 +38,7 @@ public MsSqlStoredProcedureUsingOdbcDriverTestsBase(TFixture fixture, ITestOutpu _fixture.AddCommand($"{excerciserName} CreateTable {_tableName}"); - _fixture.AddCommand($"{excerciserName} MsSqlParameterizedStoredProcedureUsingOdbcDriver {_procNameWith} {_procNameWithout}"); + _fixture.AddCommand($"{excerciserName} OdbcParameterizedStoredProcedure {_procNameWith} {_procNameWithout}"); _fixture.AddCommand($"{excerciserName} DropTable {_tableName}"); _fixture.AddActions @@ -78,23 +78,23 @@ public void Test() var expectedMetrics = new List { - new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/Other/{expectedSqlStatementWith}/ExecuteProcedure", callCount = 1 }, - new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/Other/{expectedSqlStatementWith}/ExecuteProcedure", callCount = 1, metricScope = _expectedTransactionName}, - new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/Other/{expectedSqlStatementWithout}/ExecuteProcedure", callCount = 1 }, - new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/Other/{expectedSqlStatementWithout}/ExecuteProcedure", callCount = 1, metricScope = _expectedTransactionName} + new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{expectedSqlStatementWith}/ExecuteProcedure", callCount = 1 }, + new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{expectedSqlStatementWith}/ExecuteProcedure", callCount = 1, metricScope = _expectedTransactionName}, + new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{expectedSqlStatementWithout}/ExecuteProcedure", callCount = 1 }, + new Assertions.ExpectedMetric { metricName = $@"Datastore/statement/MSSQL/{expectedSqlStatementWithout}/ExecuteProcedure", callCount = 1, metricScope = _expectedTransactionName} }; var expectedTransactionTraceSegments = new List { - $"Datastore/statement/Other/{expectedSqlStatementWith}/ExecuteProcedure", - $"Datastore/statement/Other/{expectedSqlStatementWithout}/ExecuteProcedure" + $"Datastore/statement/MSSQL/{expectedSqlStatementWith}/ExecuteProcedure", + $"Datastore/statement/MSSQL/{expectedSqlStatementWithout}/ExecuteProcedure" }; var expectedQueryParametersWith = DbParameterData.OdbcMsSqlParameters.ToDictionary(p => p.ParameterName, p => p.ExpectedValue); var expectedQueryParametersWithout = DbParameterData.OdbcMsSqlParameters.ToDictionary(p => p.ParameterName.TrimStart('@'), p => p.ExpectedValue); - var expectedTransactionTraceQueryParametersWith = new Assertions.ExpectedSegmentQueryParameters { segmentName = $"Datastore/statement/Other/{expectedSqlStatementWith}/ExecuteProcedure", QueryParameters = expectedQueryParametersWith }; - var expectedTransactionTraceQueryParametersWithout = new Assertions.ExpectedSegmentQueryParameters { segmentName = $"Datastore/statement/Other/{expectedSqlStatementWithout}/ExecuteProcedure", QueryParameters = expectedQueryParametersWithout }; + var expectedTransactionTraceQueryParametersWith = new Assertions.ExpectedSegmentQueryParameters { segmentName = $"Datastore/statement/MSSQL/{expectedSqlStatementWith}/ExecuteProcedure", QueryParameters = expectedQueryParametersWith }; + var expectedTransactionTraceQueryParametersWithout = new Assertions.ExpectedSegmentQueryParameters { segmentName = $"Datastore/statement/MSSQL/{expectedSqlStatementWithout}/ExecuteProcedure", QueryParameters = expectedQueryParametersWithout }; var expectedSqlTraces = new List { @@ -102,14 +102,14 @@ public void Test() { TransactionName = _expectedTransactionName, Sql = $"{{call {_procNameWith}({parameterPlaceholder})}}", - DatastoreMetricName = $"Datastore/statement/Other/{expectedSqlStatementWith}/ExecuteProcedure", + DatastoreMetricName = $"Datastore/statement/MSSQL/{expectedSqlStatementWith}/ExecuteProcedure", QueryParameters = expectedQueryParametersWith }, new Assertions.ExpectedSqlTrace { TransactionName = _expectedTransactionName, Sql = $"{{call {_procNameWithout}({parameterPlaceholder})}}", - DatastoreMetricName = $"Datastore/statement/Other/{expectedSqlStatementWithout}/ExecuteProcedure", + DatastoreMetricName = $"Datastore/statement/MSSQL/{expectedSqlStatementWithout}/ExecuteProcedure", QueryParameters = expectedQueryParametersWithout } @@ -138,16 +138,26 @@ public void Test() } } - // Only tests for System.Data in .NET Framework for ODBC, since the OdbcCommandWrapper is .NET Framework only, - // and the instrumentation.xml only matches System.Data as of 2022-10-20 [NetFrameworkTest] - public class MsSqlStoredProcedureUsingOdbcDriverTests : MsSqlStoredProcedureUsingOdbcDriverTestsBase + public class MsSqlStoredProcedureUsingOdbcDriverTests_FWLatest : MsSqlStoredProcedureUsingOdbcDriverTestsBase { - public MsSqlStoredProcedureUsingOdbcDriverTests(ConsoleDynamicMethodFixtureFWLatest fixture, ITestOutputHelper output) + public MsSqlStoredProcedureUsingOdbcDriverTests_FWLatest(ConsoleDynamicMethodFixtureFWLatest fixture, ITestOutputHelper output) : base( fixture: fixture, output: output, - excerciserName: "SystemDataExerciser") + excerciserName: "SystemDataOdbcExerciser") + { + } + } + + [NetCoreTest] + public class MsSqlStoredProcedureUsingOdbcDriverTests_CoreLatest : MsSqlStoredProcedureUsingOdbcDriverTestsBase + { + public MsSqlStoredProcedureUsingOdbcDriverTests_CoreLatest(ConsoleDynamicMethodFixtureCoreLatest fixture, ITestOutputHelper output) + : base( + fixture: fixture, + output: output, + excerciserName: "SystemDataOdbcExerciser") { } } diff --git a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlTests.cs b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlTests.cs index 57b38cea67..f76fee03b1 100644 --- a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlTests.cs +++ b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/MsSql/MsSqlTests.cs @@ -25,6 +25,8 @@ public abstract class MsSqlTestsBase : NewRelicIntegrationTest { // The operation metric should not be scoped because the statement metric is scoped instead @@ -149,7 +161,6 @@ public void Test() var expectedTransactionTraceSegmentParameters = new List { - new Assertions.ExpectedSegmentParameter { segmentName = "Datastore/statement/MSSQL/teammembers/select", parameterName = "explain_plan" }, new Assertions.ExpectedSegmentParameter { segmentName = "Datastore/statement/MSSQL/teammembers/select", parameterName = "sql", parameterValue = $"SELECT * FROM NewRelic.dbo.TeamMembers WHERE {firstOrLastNameQueried} = ?"}, new Assertions.ExpectedSegmentParameter { segmentName = "Datastore/statement/MSSQL/teammembers/select", parameterName = "host", parameterValue = CommonUtils.NormalizeHostname(MsSqlConfiguration.MsSqlServer)}, new Assertions.ExpectedSegmentParameter { segmentName = "Datastore/statement/MSSQL/teammembers/select", parameterName = "port_path_or_id", parameterValue = "default"}, @@ -157,6 +168,14 @@ public void Test() }; + // The ODBC instrumentation doesn't do explain plans + var sqlTracesShouldHaveExplainPlans = false; + if (!_isOdbc) + { + expectedTransactionTraceSegmentParameters.Add(new Assertions.ExpectedSegmentParameter { segmentName = "Datastore/statement/MSSQL/teammembers/select", parameterName = "explain_plan" }); + sqlTracesShouldHaveExplainPlans = true; + } + // There should be a total of 8 traces: 4 for the sync methods and 4 for the async methods. var expectedSqlTraces = new List @@ -167,7 +186,7 @@ public void Test() Sql = "SELECT * FROM NewRelic.dbo.TeamMembers WHERE FirstName = ?", DatastoreMetricName = "Datastore/statement/MSSQL/teammembers/select", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans }, new Assertions.ExpectedSqlTrace { @@ -175,7 +194,7 @@ public void Test() Sql = "SELECT * FROM NewRelic.dbo.TeamMembers WHERE LastName = ?", DatastoreMetricName = "Datastore/statement/MSSQL/teammembers/select", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans }, new Assertions.ExpectedSqlTrace { @@ -183,7 +202,7 @@ public void Test() Sql = $"SELECT COUNT(*) FROM {_tableName} WITH(nolock)", DatastoreMetricName = $"Datastore/statement/MSSQL/{_tableName}/select", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans }, new Assertions.ExpectedSqlTrace { @@ -191,7 +210,7 @@ public void Test() Sql = $"SELECT COUNT(*) FROM {_asyncTableName} WITH(nolock)", DatastoreMetricName = $"Datastore/statement/MSSQL/{_asyncTableName}/select", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans }, new Assertions.ExpectedSqlTrace { @@ -199,7 +218,7 @@ public void Test() Sql = $"INSERT INTO {_tableName} (FirstName, LastName, Email) VALUES(?, ?, ?)", DatastoreMetricName = $"Datastore/statement/MSSQL/{_tableName}/insert", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans }, new Assertions.ExpectedSqlTrace { @@ -207,7 +226,7 @@ public void Test() Sql = $"INSERT INTO {_asyncTableName} (FirstName, LastName, Email) VALUES(?, ?, ?)", DatastoreMetricName = $"Datastore/statement/MSSQL/{_asyncTableName}/insert", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans }, new Assertions.ExpectedSqlTrace { @@ -215,7 +234,7 @@ public void Test() Sql = $"DELETE FROM {_tableName} WHERE Email = ?", DatastoreMetricName = $"Datastore/statement/MSSQL/{_tableName}/delete", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans, }, new Assertions.ExpectedSqlTrace { @@ -223,7 +242,7 @@ public void Test() Sql = $"DELETE FROM {_asyncTableName} WHERE Email = ?", DatastoreMetricName = $"Datastore/statement/MSSQL/{_asyncTableName}/delete", - HasExplainPlan = true + HasExplainPlan = sqlTracesShouldHaveExplainPlans } }; @@ -248,6 +267,8 @@ public void Test() } } + + #region System.Data.SqlClient [NetFrameworkTest] public class MsSqlTests_SystemData_FWLatest : MsSqlTestsBase { @@ -260,6 +281,9 @@ public MsSqlTests_SystemData_FWLatest(ConsoleDynamicMethodFixtureFWLatest fixtur { } } + #endregion + + #region Microsoft.Data.SqlClient [NetFrameworkTest] public class MsSqlTests_MicrosoftDataSqlClient_FWLatest : MsSqlTestsBase @@ -312,4 +336,63 @@ public MsSqlTests_MicrosoftDataSqlClient_CoreLatest(ConsoleDynamicMethodFixtureC { } } + + #endregion + + #region System.Data.Odbc + + [NetFrameworkTest] + public class MsSqlTests_SystemDataOdbc_FWLatest : MsSqlTestsBase + { + public MsSqlTests_SystemDataOdbc_FWLatest(ConsoleDynamicMethodFixtureFWLatest fixture, ITestOutputHelper output) + : base( + fixture: fixture, + output: output, + excerciserName: "SystemDataOdbcExerciser", + libraryName: "System.Data.Odbc") + { + } + } + + [NetFrameworkTest] + public class MsSqlTests_SystemDataOdbc_FW462 : MsSqlTestsBase + { + public MsSqlTests_SystemDataOdbc_FW462(ConsoleDynamicMethodFixtureFW462 fixture, ITestOutputHelper output) + : base( + fixture: fixture, + output: output, + excerciserName: "SystemDataOdbcExerciser", + libraryName: "System.Data.Odbc") + { + } + } + + [NetCoreTest] + public class MsSqlTests_SystemDataOdbc_CoreLatest : MsSqlTestsBase + { + public MsSqlTests_SystemDataOdbc_CoreLatest(ConsoleDynamicMethodFixtureCoreLatest fixture, ITestOutputHelper output) + : base( + fixture: fixture, + output: output, + excerciserName: "SystemDataOdbcExerciser", + libraryName: "System.Data.Odbc") + { + } + } + + [NetCoreTest] + public class MsSqlTests_SystemDataOdbc_CoreOldest : MsSqlTestsBase + { + public MsSqlTests_SystemDataOdbc_CoreOldest(ConsoleDynamicMethodFixtureCoreOldest fixture, ITestOutputHelper output) + : base( + fixture: fixture, + output: output, + excerciserName: "SystemDataOdbcExerciser", + libraryName: "System.Data.Odbc") + { + } + } + + #endregion + } diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs index 06be0867cb..fee82c776c 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs @@ -26,7 +26,7 @@ public class SqlWrapperHelperTests [TestCase("Oracle", ExpectedResult = DatastoreVendor.Oracle)] [TestCase("PostgreSQL", ExpectedResult = DatastoreVendor.Postgres)] [TestCase("IBMDB2", ExpectedResult = DatastoreVendor.IBMDB2)] - public DatastoreVendor GetVendorName_ReturnsCorrectHost_IfOleDbConnectionProviderContainsKnownHost( + public DatastoreVendor GetVendorName_ReturnsCorrectVendor_IfOleDbConnectionProviderContainsKnownProvider( string provider) { var command = new OleDbCommand @@ -45,7 +45,7 @@ public DatastoreVendor GetVendorName_ReturnsCorrectHost_IfOleDbConnectionProvide [TestCase("NpgsqlCommand", ExpectedResult = DatastoreVendor.Postgres)] [TestCase("DB2Command", ExpectedResult = DatastoreVendor.IBMDB2)] public DatastoreVendor - GetVendorName_ReturnsCorrectHost(string typeName) + GetVendorName_ReturnsCorrectVendor(string typeName) { return SqlWrapperHelper.GetVendorName(typeName); } @@ -82,6 +82,20 @@ public void GetVendorName_ReturnsUnknown_IfCommandIsOfUnknownType() Assert.That(datastoreName, Is.EqualTo(DatastoreVendor.Other)); } + [Test] + [TestCase("DRIVER={SQL Server Native Client 11.0};Server=127.0.0.1;Database=NewRelic;Trusted_Connection=no;UID=sa;PWD=password;Encrypt=no;", ExpectedResult = DatastoreVendor.MSSQL)] + [TestCase("Driver={MySQL ODBC 5.2 UNICODE Driver};Server=localhost;Database=myDataBase;User=myUsername;Password=myPassword;Option=3;", ExpectedResult = DatastoreVendor.MySQL)] + [TestCase("Driver={Microsoft ODBC for Oracle};Server=myServerAddress;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Oracle)] + [TestCase("Driver={Oracle in OraClient11g_home1};Dbq=myTNSServiceName;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Oracle)] + [TestCase("Driver={PostgreSQL UNICODE};Server=IP address;Port=5432;Database=myDataBase;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Postgres)] + [TestCase("Driver={IBM DB2 ODBC DRIVER};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.IBMDB2)] + [TestCase("Driver={Amazon Redshift ODBC Driver (x64)};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.ODBC)] + [TestCase("Driver={MyCoolDb ODBC DRIVER};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.ODBC)] + public DatastoreVendor GetVendorNameFromOdbcConnectionString_ReturnsExpectedVendor(string connectionString) + { + return SqlWrapperHelper.GetVendorNameFromOdbcConnectionString(connectionString); + } + public class UnknownDbCommand : IDbCommand { public void Dispose() From 33dac84728d4e17ef4affdfa451678798d8fcec6 Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Tue, 14 Jan 2025 13:26:55 -0800 Subject: [PATCH 06/13] Add unit tests for OdbcConnectionStringParser Also fix bug where server/hostname string with port included (e.g. "myservername:1234") wasn't handled correctly --- .../Helpers/StringSeparators.cs | 3 ++ .../IConnectionStringParser.cs | 2 +- .../OdbcConnectionStringParser.cs | 28 ++++++++++++------- .../Parsing/ConnectionStringParserTests.cs | 6 ++++ 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/StringSeparators.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/StringSeparators.cs index 1dc0b35f02..a7fbb12b06 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/StringSeparators.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Helpers/StringSeparators.cs @@ -8,6 +8,9 @@ public static class StringSeparators public const char PathSeparatorChar = '/'; public static readonly char[] PathSeparator = { PathSeparatorChar }; + public const char BackslashChar = '\\'; + public static readonly char[] Backslash = { BackslashChar }; + public const char CommaChar = ','; public static readonly char[] Comma = { CommaChar }; diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs index bc0d1fc938..66bcc04e95 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IConnectionStringParser.cs @@ -44,7 +44,7 @@ private static IConnectionStringParser GetConnectionParser(DatastoreVendor vendo case DatastoreVendor.Redis: return new StackExchangeRedisConnectionStringParser(connectionString); case DatastoreVendor.ODBC: - return new OdbcConnectionStringParser(connectionString); + return new OdbcConnectionStringParser(connectionString); default: return null; } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs index 7b944b7088..4557bdf8d8 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using NewRelic.Agent.Extensions.Providers.Wrapper; +using NewRelic.Agent.Helpers; using System.Collections.Generic; using System.Data.Common; using System.Linq; @@ -10,7 +11,7 @@ namespace NewRelic.Agent.Extensions.Parsing.ConnectionString { public class OdbcConnectionStringParser : IConnectionStringParser { - private static readonly List _hostKeys = new List { "server", "data source" }; + private static readonly List _hostKeys = new List { "server", "data source", "hostname" }; private static readonly List _portKeys = new List { "port" }; private static readonly List _databaseNameKeys = new List { "database" }; @@ -39,15 +40,16 @@ private string ParseHost() // Example of want we would need to process: win-database.pdx.vm.datanerd.us,1433\SQLEXPRESS try { - var splitIndex = host.IndexOf(','); - if (splitIndex == -1) splitIndex = host.IndexOf('\\'); + var splitIndex = host.IndexOf(StringSeparators.CommaChar); + if (splitIndex == -1) splitIndex = host.IndexOf(StringSeparators.BackslashChar); host = splitIndex == -1 ? host : host.Substring(0, splitIndex); } catch { return null; } - return host; + var endOfHostname = host.IndexOf(StringSeparators.ColonChar); + return endOfHostname == -1 ? host : host.Substring(0, endOfHostname); } private string ParsePortPathOrId() @@ -66,11 +68,17 @@ private string ParsePortPathOrId() try { - if (portPathOrId.IndexOf(',') != -1) + if (portPathOrId.IndexOf(StringSeparators.ColonChar) != -1) { - var startOfValue = portPathOrId.IndexOf(',') + 1; - var endOfValue = portPathOrId.Contains('\\') - ? portPathOrId.IndexOf('\\') + var startOfValue = portPathOrId.IndexOf(StringSeparators.ColonChar) + 1; + var endOfValue = portPathOrId.Length; + return (startOfValue > 0) ? portPathOrId.Substring(startOfValue, endOfValue - startOfValue) : null; + } + if (portPathOrId.IndexOf(StringSeparators.CommaChar) != -1) + { + var startOfValue = portPathOrId.IndexOf(StringSeparators.CommaChar) + 1; + var endOfValue = portPathOrId.Contains(StringSeparators.BackslashChar) + ? portPathOrId.IndexOf(StringSeparators.BackslashChar) : portPathOrId.Length; return (startOfValue > 0) ? portPathOrId.Substring(startOfValue, endOfValue - startOfValue) : null; } @@ -90,9 +98,9 @@ private string ParseInstanceName() try { - if (instanceName.IndexOf('\\') != -1) + if (instanceName.IndexOf(StringSeparators.BackslashChar) != -1) { - var startOfValue = instanceName.IndexOf('\\') + 1; + var startOfValue = instanceName.IndexOf(StringSeparators.BackslashChar) + 1; var endOfValue = instanceName.Length; return (startOfValue > 0) ? instanceName.Substring(startOfValue, endOfValue - startOfValue) : null; } diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs index 8c7ff867ea..6dc0e2a522 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs @@ -51,6 +51,12 @@ public class ConnectionStringParserTests [TestCase(DatastoreVendor.Redis, "hostname_of_localhost", "unknown", "unknown", null, "localhost,password=NOPERS")] [TestCase(DatastoreVendor.Redis, "hostname_of_localhost", "unknown", "unknown", null, "127.0.0.1,password=NOPERS")] + [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "default", "NewRelic", null, "DRIVER={SQL Server Native Client 11.0};Server=127.0.0.1;Database=NewRelic;Trusted_Connection=no;UID=sa;PWD=password;Encrypt=no;")] + [TestCase(DatastoreVendor.ODBC, "win-database.pdx.vm.datanerd.us", "1234", "myDataBase", null, "Driver={MySQL ODBC 5.2 UNICODE Driver};Server=win-database.pdx.vm.datanerd.us:1234;Database=myDataBase;User=myUsername;Password=myPassword;Option=3;")] + [TestCase(DatastoreVendor.ODBC, "myServerAddress", "1234", "myDataBase", null, "Driver={Amazon Redshift ODBC Driver (x64)};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;")] + [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "localhost,password=NOPERS")] + [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "127.0.0.1,password=NOPERS")] + public void TestConnectionStringParsing(DatastoreVendor vendor, string expectedHost, string expectedPathPortOrId, string expectedDatabaseName, string expectedInstanceName, string connectionString) { var connectionInfo = ConnectionInfoParser.FromConnectionString(vendor, connectionString, "hostname_of_localhost"); From e45ad0d86883e8ded0be459e84161508a3d2238f Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Tue, 14 Jan 2025 13:41:12 -0800 Subject: [PATCH 07/13] Refactoring and cleanup --- .../IbmDb2ConnectionStringParser.cs | 8 ++++---- .../MsSqlConnectionStringParser.cs | 18 +++++++++--------- .../MySqlConnectionStringParser.cs | 4 ++-- .../OdbcConnectionStringParser.cs | 1 - .../OracleConnectionStringParser.cs | 1 - .../PostgresConnectionStringParser.cs | 1 - ...StackExchangeRedisConnectionStringParser.cs | 1 - 7 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IbmDb2ConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IbmDb2ConnectionStringParser.cs index 9bca8cc810..a4015ad786 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IbmDb2ConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/IbmDb2ConnectionStringParser.cs @@ -1,10 +1,10 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -using NewRelic.Agent.Extensions.Providers.Wrapper; using System.Collections.Generic; using System.Data.Common; using System.Linq; +using NewRelic.Agent.Helpers; namespace NewRelic.Agent.Extensions.Parsing.ConnectionString { @@ -35,7 +35,7 @@ private string ParseHost() var host = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; if (host == null) return null; - var endOfHostname = host.IndexOf(':'); + var endOfHostname = host.IndexOf(StringSeparators.ColonChar); return endOfHostname == -1 ? host : host.Substring(0, endOfHostname); } @@ -44,8 +44,8 @@ private string ParsePortPathOrId() var host = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; if (host == null) return null; - if (host.Contains(':')) - return host.Substring(host.IndexOf(':') + 1); + if (host.Contains(StringSeparators.ColonChar)) + return host.Substring(host.IndexOf(StringSeparators.ColonChar) + 1); return "default"; } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs index 0dd99fb99c..6842ea13a5 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs @@ -1,10 +1,10 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -using NewRelic.Agent.Extensions.Providers.Wrapper; using System.Collections.Generic; using System.Data.Common; using System.Linq; +using NewRelic.Agent.Helpers; namespace NewRelic.Agent.Extensions.Parsing.ConnectionString { @@ -38,8 +38,8 @@ private string ParseHost() // Example of want we would need to process: win-database.pdx.vm.datanerd.us,1433\SQLEXPRESS try { - var splitIndex = host.IndexOf(','); - if (splitIndex == -1) splitIndex = host.IndexOf('\\'); + var splitIndex = host.IndexOf(StringSeparators.CommaChar); + if (splitIndex == -1) splitIndex = host.IndexOf(StringSeparators.BackslashChar); host = splitIndex == -1 ? host : host.Substring(0, splitIndex); } catch @@ -56,11 +56,11 @@ private string ParsePortPathOrId() try { - if (portPathOrId.IndexOf(',') != -1) + if (portPathOrId.IndexOf(StringSeparators.CommaChar) != -1) { - var startOfValue = portPathOrId.IndexOf(',') + 1; - var endOfValue = portPathOrId.Contains('\\') - ? portPathOrId.IndexOf('\\') + var startOfValue = portPathOrId.IndexOf(StringSeparators.CommaChar) + 1; + var endOfValue = portPathOrId.Contains(StringSeparators.BackslashChar) + ? portPathOrId.IndexOf(StringSeparators.BackslashChar) : portPathOrId.Length; return (startOfValue > 0) ? portPathOrId.Substring(startOfValue, endOfValue - startOfValue) : null; } @@ -80,9 +80,9 @@ private string ParseInstanceName() try { - if (instanceName.IndexOf('\\') != -1) + if (instanceName.IndexOf(StringSeparators.BackslashChar) != -1) { - var startOfValue = instanceName.IndexOf('\\') + 1; + var startOfValue = instanceName.IndexOf(StringSeparators.BackslashChar) + 1; var endOfValue = instanceName.Length; return (startOfValue > 0) ? instanceName.Substring(startOfValue, endOfValue - startOfValue) : null; } diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MySqlConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MySqlConnectionStringParser.cs index 39079ea679..3517b5009e 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MySqlConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MySqlConnectionStringParser.cs @@ -1,9 +1,9 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -using NewRelic.Agent.Extensions.Providers.Wrapper; using System.Collections.Generic; using System.Data.Common; +using NewRelic.Agent.Helpers; namespace NewRelic.Agent.Extensions.Parsing.ConnectionString { @@ -24,7 +24,7 @@ public ConnectionInfo GetConnectionInfo(string utilizationHostName) { var host = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; - var hasMultipleHosts = host != null && host.IndexOf(',') != -1; + var hasMultipleHosts = host != null && host.IndexOf(StringSeparators.CommaChar) != -1; if (hasMultipleHosts) host = null; else if (host != null) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs index 4557bdf8d8..1a7f7fd560 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs @@ -1,7 +1,6 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Helpers; using System.Collections.Generic; using System.Data.Common; diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OracleConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OracleConnectionStringParser.cs index 76294180a0..f0614ad31c 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OracleConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OracleConnectionStringParser.cs @@ -5,7 +5,6 @@ using System.Data.Common; using System.Linq; using NewRelic.Agent.Helpers; -using NewRelic.Agent.Extensions.Providers.Wrapper; namespace NewRelic.Agent.Extensions.Parsing.ConnectionString { diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/PostgresConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/PostgresConnectionStringParser.cs index f9029981e1..a0f1b842fa 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/PostgresConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/PostgresConnectionStringParser.cs @@ -1,7 +1,6 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -using NewRelic.Agent.Extensions.Providers.Wrapper; using System.Collections.Generic; using System.Data.Common; diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/StackExchangeRedisConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/StackExchangeRedisConnectionStringParser.cs index cdae23860d..c61ec1485d 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/StackExchangeRedisConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/StackExchangeRedisConnectionStringParser.cs @@ -3,7 +3,6 @@ using System.Linq; using NewRelic.Agent.Helpers; -using NewRelic.Agent.Extensions.Providers.Wrapper; namespace NewRelic.Agent.Extensions.Parsing.ConnectionString { From 7d799572f536aaa1ad2ae9018d41e6a9a859a790 Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Wed, 15 Jan 2025 12:20:18 -0800 Subject: [PATCH 08/13] Increase test coverage --- .../Parsing/SqlWrapperHelper.cs | 7 ++++-- .../Parsing/ConnectionStringParserTests.cs | 22 ++++++++++++++++--- .../Parsing/SqlWrapperHelperTests.cs | 13 +++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs index 605dcca21e..32ff4823a6 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/SqlWrapperHelper.cs @@ -89,6 +89,9 @@ public static DatastoreVendor GetVendorName(string typeName) /// private static DatastoreVendor ExtractVendorNameFromString(string text) { + // Note that, because of the ordering of the following checks, an ODBC connection string for a known DB vendor should result in that vendor + // being returned, instead of ODBC + // example: Driver={ODBC Driver 17 for SQL Server};Server=myServerAddress;Database=myDataBase; if (text.IndexOf("SQL Server", StringComparison.OrdinalIgnoreCase) != -1 || text.IndexOf("SQLServer", StringComparison.OrdinalIgnoreCase) != -1) return DatastoreVendor.MSSQL; @@ -104,9 +107,9 @@ private static DatastoreVendor ExtractVendorNameFromString(string text) if (text.IndexOf("DB2", StringComparison.OrdinalIgnoreCase) != -1 || text.IndexOf("IBM", StringComparison.OrdinalIgnoreCase) != -1) return DatastoreVendor.IBMDB2; - // This works for redshift since the driver reports as "Amazon Redshift ODBC Driver" + // This works for redshift since the driver reports as "Amazon Redshift (x64)" // Other drivers may not report as expected - if (text.IndexOf("ODBC", StringComparison.OrdinalIgnoreCase) != -1) + if (text.IndexOf("ODBC", StringComparison.OrdinalIgnoreCase) != -1 || text.IndexOf("Redshift", StringComparison.OrdinalIgnoreCase) != -1) return DatastoreVendor.ODBC; return DatastoreVendor.Other; diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs index 6dc0e2a522..c3ca4d2cd8 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs @@ -5,6 +5,7 @@ using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Extensions.Parsing.ConnectionString; using NUnit.Framework; +using NewRelic.Agent.Extensions.Parsing; namespace ParsingTests { @@ -51,9 +52,11 @@ public class ConnectionStringParserTests [TestCase(DatastoreVendor.Redis, "hostname_of_localhost", "unknown", "unknown", null, "localhost,password=NOPERS")] [TestCase(DatastoreVendor.Redis, "hostname_of_localhost", "unknown", "unknown", null, "127.0.0.1,password=NOPERS")] - [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "default", "NewRelic", null, "DRIVER={SQL Server Native Client 11.0};Server=127.0.0.1;Database=NewRelic;Trusted_Connection=no;UID=sa;PWD=password;Encrypt=no;")] - [TestCase(DatastoreVendor.ODBC, "win-database.pdx.vm.datanerd.us", "1234", "myDataBase", null, "Driver={MySQL ODBC 5.2 UNICODE Driver};Server=win-database.pdx.vm.datanerd.us:1234;Database=myDataBase;User=myUsername;Password=myPassword;Option=3;")] - [TestCase(DatastoreVendor.ODBC, "myServerAddress", "1234", "myDataBase", null, "Driver={Amazon Redshift ODBC Driver (x64)};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;")] + // A lot of the following connection strings are contrived examples for code coverage purposes + [TestCase(DatastoreVendor.ODBC, "examplecluster.abc123xyz789.us-west-2.redshift.amazonaws.com", "5439", "dev", null, "Driver={Amazon Redshift (x64)};Server=examplecluster.abc123xyz789.us-west-2.redshift.amazonaws.com;Database=dev;UID=adminuser;PWD=secret;Port=5439")] + [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "5439", "dev", null, "Driver={Amazon Redshift (x64)};Hostname=localhost;Database=dev;UID=adminuser;PWD=secret;Port=5439")] + [TestCase(DatastoreVendor.ODBC, "myServerAddress", "1234", "myDataBase", null, "Driver={Amazon Redshift ODBC Driver (x64)};Database=myDataBase;Data source=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;")] + [TestCase(DatastoreVendor.ODBC, "myServerAddress", "1234", "myDataBase", "someInstance", "Driver={ODBC Driver for Sequel Server};Server=mySequelServerHost,1234\\someInstance;Database=myDatabase")] [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "localhost,password=NOPERS")] [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "127.0.0.1,password=NOPERS")] @@ -92,5 +95,18 @@ public void ConnectionStringParser_DifferentConnectionStrings_NotMatched() Assert.That(connectionInfo2, Is.Not.SameAs(connectionInfo1)); } + + [Test] + public void ConnectionStringParser_UnknownVendor_EmptyResult() + { + var connectionString = @"Driver={Microsoft Text Driver (*.txt; *.csv)};Dbq=c:\txtFilesFolder\;Extensions=asc,csv,tab,txt;"; + + var connectionInfo = ConnectionInfoParser.FromConnectionString(DatastoreVendor.Other, connectionString, "localhost"); + + Assert.That(connectionInfo.Host, Is.EqualTo("unknown")); + Assert.That(connectionInfo.PortPathOrId, Is.EqualTo("unknown")); + Assert.That(connectionInfo.DatabaseName, Is.EqualTo("unknown")); + Assert.That(connectionInfo.InstanceName, Is.Null); + } } } diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs index fee82c776c..94530130cb 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/SqlWrapperHelperTests.cs @@ -88,14 +88,27 @@ public void GetVendorName_ReturnsUnknown_IfCommandIsOfUnknownType() [TestCase("Driver={Microsoft ODBC for Oracle};Server=myServerAddress;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Oracle)] [TestCase("Driver={Oracle in OraClient11g_home1};Dbq=myTNSServiceName;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Oracle)] [TestCase("Driver={PostgreSQL UNICODE};Server=IP address;Port=5432;Database=myDataBase;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Postgres)] + [TestCase("Driver={npgsql};Server=IP address;Port=5432;Database=myDataBase;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Postgres)] [TestCase("Driver={IBM DB2 ODBC DRIVER};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.IBMDB2)] [TestCase("Driver={Amazon Redshift ODBC Driver (x64)};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.ODBC)] [TestCase("Driver={MyCoolDb ODBC DRIVER};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.ODBC)] + [TestCase("Driver={MyCoolDb};Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.Other)] + [TestCase("Database=myDataBase;Hostname=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;", ExpectedResult = DatastoreVendor.ODBC)] // no driver specified public DatastoreVendor GetVendorNameFromOdbcConnectionString_ReturnsExpectedVendor(string connectionString) { return SqlWrapperHelper.GetVendorNameFromOdbcConnectionString(connectionString); } + [Test] + public void GetVendorNameFromOdbcConnectionString_MultipleTimes_UsesCachedVendor() + { + var connectionString = "DRIVER ={ SQL Server Native Client 11.0}; Server = 127.0.0.1; Database = NewRelic; Trusted_Connection = no; UID = sa; PWD = password; Encrypt = no;"; + + var vendor1 = SqlWrapperHelper.GetVendorNameFromOdbcConnectionString(connectionString); + var vendor2 = SqlWrapperHelper.GetVendorNameFromOdbcConnectionString(connectionString); + Assert.That(vendor1, Is.EqualTo(vendor2)); + } + public class UnknownDbCommand : IDbCommand { public void Dispose() From 2665f286ab978e98a7dce1cd76dd469dba631979 Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Wed, 15 Jan 2025 13:48:39 -0800 Subject: [PATCH 09/13] Fix unit test bugs --- .../Parsing/ConnectionStringParserTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs index c3ca4d2cd8..20f482e377 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs @@ -56,7 +56,7 @@ public class ConnectionStringParserTests [TestCase(DatastoreVendor.ODBC, "examplecluster.abc123xyz789.us-west-2.redshift.amazonaws.com", "5439", "dev", null, "Driver={Amazon Redshift (x64)};Server=examplecluster.abc123xyz789.us-west-2.redshift.amazonaws.com;Database=dev;UID=adminuser;PWD=secret;Port=5439")] [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "5439", "dev", null, "Driver={Amazon Redshift (x64)};Hostname=localhost;Database=dev;UID=adminuser;PWD=secret;Port=5439")] [TestCase(DatastoreVendor.ODBC, "myServerAddress", "1234", "myDataBase", null, "Driver={Amazon Redshift ODBC Driver (x64)};Database=myDataBase;Data source=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;")] - [TestCase(DatastoreVendor.ODBC, "myServerAddress", "1234", "myDataBase", "someInstance", "Driver={ODBC Driver for Sequel Server};Server=mySequelServerHost,1234\\someInstance;Database=myDatabase")] + [TestCase(DatastoreVendor.ODBC, "mySequelServerHost", "1234", "myDataBase", "someInstance", "Driver={ODBC Driver for Sequel Server};Server=mySequelServerHost,1234\\someInstance;Database=myDataBase")] [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "localhost,password=NOPERS")] [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "127.0.0.1,password=NOPERS")] From 3168149cbaaee0fc362241f6422b231feb75fab6 Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Wed, 15 Jan 2025 14:36:07 -0800 Subject: [PATCH 10/13] Remove cruft --- .../NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs b/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs index 707de51eed..a1afd2162e 100644 --- a/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs +++ b/src/Agent/NewRelic/Agent/Core/Segments/DatastoreSegmentData.cs @@ -233,13 +233,6 @@ public override void SetSpanTypeSpecificAttributes(SpanAttributeValueCollection { AttribDefs.DbServerPort.TrySetValue(attribVals, Port.Value); } - - // This shouldn't be necessary since we're already setting DbServerAddress above - //// For AWS Redshift relationship - //if (Host.EndsWith(".redshift.amazonaws.com")) - //{ - // AttribDefs.ConfigurationEndpointAddress.TrySetValue(attribVals, Host); - //} } public void SetConnectionInfo(ConnectionInfo connInfo) From 82aaafeffa5f322854199bfc65a4051049f82364 Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Wed, 15 Jan 2025 14:44:01 -0800 Subject: [PATCH 11/13] One more test case to increase coverage --- .../Parsing/ConnectionStringParserTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs index 20f482e377..06b1bedf2c 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs @@ -57,6 +57,7 @@ public class ConnectionStringParserTests [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "5439", "dev", null, "Driver={Amazon Redshift (x64)};Hostname=localhost;Database=dev;UID=adminuser;PWD=secret;Port=5439")] [TestCase(DatastoreVendor.ODBC, "myServerAddress", "1234", "myDataBase", null, "Driver={Amazon Redshift ODBC Driver (x64)};Database=myDataBase;Data source=myServerAddress;Port=1234;Protocol=TCPIP;Uid=myUsername;Pwd=myPassword;")] [TestCase(DatastoreVendor.ODBC, "mySequelServerHost", "1234", "myDataBase", "someInstance", "Driver={ODBC Driver for Sequel Server};Server=mySequelServerHost,1234\\someInstance;Database=myDataBase")] + [TestCase(DatastoreVendor.ODBC, "myReddishServerHost", "234", "unknown", null, "Driver={ODBC Driver for Reddish Server};Server=myReddishServerHost:234")] [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "localhost,password=NOPERS")] [TestCase(DatastoreVendor.ODBC, "hostname_of_localhost", "unknown", "unknown", null, "127.0.0.1,password=NOPERS")] From 235bbb3d1e45b70a952fc847828192b1f55e5fca Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Thu, 16 Jan 2025 14:58:19 -0800 Subject: [PATCH 12/13] Add system.data.odbc to dotty --- .../workflows/scripts/nugetSlackNotifications/packageInfo.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json b/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json index 21dc561022..7b90f6be23 100644 --- a/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json +++ b/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json @@ -156,5 +156,8 @@ }, { "packageName": "stackexchange.redis" + }, + { + "packageName": "system.data.odbc" } ] From 4d131845ece386aa824897f48c07edf3b3e1b32a Mon Sep 17 00:00:00 2001 From: Alex Hemsath Date: Wed, 22 Jan 2025 09:37:38 -0800 Subject: [PATCH 13/13] Remove unnecessary try/catch and guard logic This will help unit test coverage since we had no way of ever exercising the catch blocks as they were impossible to ever hit. --- .../MsSqlConnectionStringParser.cs | 47 +++++----------- .../OdbcConnectionStringParser.cs | 56 ++++++------------- .../Parsing/ConnectionStringParserTests.cs | 2 - 3 files changed, 30 insertions(+), 75 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs index 6842ea13a5..b6d9a46dca 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/MsSqlConnectionStringParser.cs @@ -36,16 +36,9 @@ private string ParseHost() if (host == null) return null; // Example of want we would need to process: win-database.pdx.vm.datanerd.us,1433\SQLEXPRESS - try - { - var splitIndex = host.IndexOf(StringSeparators.CommaChar); - if (splitIndex == -1) splitIndex = host.IndexOf(StringSeparators.BackslashChar); - host = splitIndex == -1 ? host : host.Substring(0, splitIndex); - } - catch - { - return null; - } + var splitIndex = host.IndexOf(StringSeparators.CommaChar); + if (splitIndex == -1) splitIndex = host.IndexOf(StringSeparators.BackslashChar); + host = splitIndex == -1 ? host : host.Substring(0, splitIndex); return host; } @@ -54,20 +47,13 @@ private string ParsePortPathOrId() var portPathOrId = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; if (portPathOrId == null) return null; - try + if (portPathOrId.IndexOf(StringSeparators.CommaChar) != -1) { - if (portPathOrId.IndexOf(StringSeparators.CommaChar) != -1) - { - var startOfValue = portPathOrId.IndexOf(StringSeparators.CommaChar) + 1; - var endOfValue = portPathOrId.Contains(StringSeparators.BackslashChar) - ? portPathOrId.IndexOf(StringSeparators.BackslashChar) - : portPathOrId.Length; - return (startOfValue > 0) ? portPathOrId.Substring(startOfValue, endOfValue - startOfValue) : null; - } - } - catch - { - return null; + var startOfValue = portPathOrId.IndexOf(StringSeparators.CommaChar) + 1; + var endOfValue = portPathOrId.Contains(StringSeparators.BackslashChar) + ? portPathOrId.IndexOf(StringSeparators.BackslashChar) + : portPathOrId.Length; + return portPathOrId.Substring(startOfValue, endOfValue - startOfValue); } return "default"; @@ -78,18 +64,11 @@ private string ParseInstanceName() var instanceName = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; if (instanceName == null) return null; - try - { - if (instanceName.IndexOf(StringSeparators.BackslashChar) != -1) - { - var startOfValue = instanceName.IndexOf(StringSeparators.BackslashChar) + 1; - var endOfValue = instanceName.Length; - return (startOfValue > 0) ? instanceName.Substring(startOfValue, endOfValue - startOfValue) : null; - } - } - catch + if (instanceName.IndexOf(StringSeparators.BackslashChar) != -1) { - return null; + var startOfValue = instanceName.IndexOf(StringSeparators.BackslashChar) + 1; + var endOfValue = instanceName.Length; + return instanceName.Substring(startOfValue, endOfValue - startOfValue); } return null; diff --git a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs index 1a7f7fd560..7f26d7bbbb 100644 --- a/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs +++ b/src/Agent/NewRelic/Agent/Extensions/NewRelic.Agent.Extensions/Parsing/ConnectionString/OdbcConnectionStringParser.cs @@ -37,16 +37,9 @@ private string ParseHost() if (host == null) return null; // Example of want we would need to process: win-database.pdx.vm.datanerd.us,1433\SQLEXPRESS - try - { - var splitIndex = host.IndexOf(StringSeparators.CommaChar); - if (splitIndex == -1) splitIndex = host.IndexOf(StringSeparators.BackslashChar); - host = splitIndex == -1 ? host : host.Substring(0, splitIndex); - } - catch - { - return null; - } + var splitIndex = host.IndexOf(StringSeparators.CommaChar); + if (splitIndex == -1) splitIndex = host.IndexOf(StringSeparators.BackslashChar); + host = splitIndex == -1 ? host : host.Substring(0, splitIndex); var endOfHostname = host.IndexOf(StringSeparators.ColonChar); return endOfHostname == -1 ? host : host.Substring(0, endOfHostname); } @@ -65,28 +58,20 @@ private string ParsePortPathOrId() portPathOrId = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; if (portPathOrId == null) return null; - try + if (portPathOrId.IndexOf(StringSeparators.ColonChar) != -1) { - if (portPathOrId.IndexOf(StringSeparators.ColonChar) != -1) - { - var startOfValue = portPathOrId.IndexOf(StringSeparators.ColonChar) + 1; - var endOfValue = portPathOrId.Length; - return (startOfValue > 0) ? portPathOrId.Substring(startOfValue, endOfValue - startOfValue) : null; - } - if (portPathOrId.IndexOf(StringSeparators.CommaChar) != -1) - { - var startOfValue = portPathOrId.IndexOf(StringSeparators.CommaChar) + 1; - var endOfValue = portPathOrId.Contains(StringSeparators.BackslashChar) - ? portPathOrId.IndexOf(StringSeparators.BackslashChar) - : portPathOrId.Length; - return (startOfValue > 0) ? portPathOrId.Substring(startOfValue, endOfValue - startOfValue) : null; - } + var startOfValue = portPathOrId.IndexOf(StringSeparators.ColonChar) + 1; + var endOfValue = portPathOrId.Length; + return portPathOrId.Substring(startOfValue, endOfValue - startOfValue); } - catch + if (portPathOrId.IndexOf(StringSeparators.CommaChar) != -1) { - return null; + var startOfValue = portPathOrId.IndexOf(StringSeparators.CommaChar) + 1; + var endOfValue = portPathOrId.Contains(StringSeparators.BackslashChar) + ? portPathOrId.IndexOf(StringSeparators.BackslashChar) + : portPathOrId.Length; + return portPathOrId.Substring(startOfValue, endOfValue - startOfValue); } - return "default"; } @@ -95,18 +80,11 @@ private string ParseInstanceName() var instanceName = ConnectionStringParserHelper.GetKeyValuePair(_connectionStringBuilder, _hostKeys)?.Value; if (instanceName == null) return null; - try - { - if (instanceName.IndexOf(StringSeparators.BackslashChar) != -1) - { - var startOfValue = instanceName.IndexOf(StringSeparators.BackslashChar) + 1; - var endOfValue = instanceName.Length; - return (startOfValue > 0) ? instanceName.Substring(startOfValue, endOfValue - startOfValue) : null; - } - } - catch + if (instanceName.IndexOf(StringSeparators.BackslashChar) != -1) { - return null; + var startOfValue = instanceName.IndexOf(StringSeparators.BackslashChar) + 1; + var endOfValue = instanceName.Length; + return instanceName.Substring(startOfValue, endOfValue - startOfValue); } return null; diff --git a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs index 06b1bedf2c..06c3c4c9ed 100644 --- a/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs +++ b/tests/Agent/UnitTests/NewRelic.Agent.Extensions.Tests/Parsing/ConnectionStringParserTests.cs @@ -1,11 +1,9 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -using System.Net; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Extensions.Parsing.ConnectionString; using NUnit.Framework; -using NewRelic.Agent.Extensions.Parsing; namespace ParsingTests {