From 7467309f6a09ad99e8b39658a5dbb456a2eba952 Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Fri, 15 Nov 2024 15:58:16 -0600 Subject: [PATCH 1/8] RabbitMQ v7 instrumentation updates. WIP. --- .../Wrapper/RabbitMq/BasicGetWrapper.cs | 16 ++- .../Wrapper/RabbitMq/BasicPublishWrapper.cs | 26 +++- .../RabbitMq/BasicPublishWrapperLegacy.cs | 2 +- .../Wrapper/RabbitMq/Instrumentation.xml | 82 +++++++++---- .../Wrapper/RabbitMq/QueuePurgeWrapper.cs | 1 + .../Wrapper/RabbitMq/RabbitMqHelper.cs | 115 +++++++++++++----- 6 files changed, 177 insertions(+), 65 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicGetWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicGetWrapper.cs index 29e554f33f..717851dd3c 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicGetWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicGetWrapper.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using System; +using System.Threading.Tasks; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Extensions.SystemExtensions; @@ -34,7 +35,20 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins serverPort: RabbitMqHelper.GetServerPort(instrumentedMethodCall, agent), routingKey: queue); // no way to get routing key from BasicGet - return Delegates.GetDelegateFor( + return instrumentedMethodCall.IsAsync ? + Delegates.GetAsyncDelegateFor( + agent, segment, false, + onComplete: (t) => + { + if (t.IsFaulted) + { + transaction.NoticeError(t.Exception); + } + + segment.End(); + }) + : + Delegates.GetDelegateFor( onFailure: transaction.NoticeError, onComplete: segment.End ); diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs index 815eabd0bd..92f836f111 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs @@ -23,12 +23,26 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) { // 3.6.0+ (5.1.0+) (IModel)void BasicPublish(string exchange, string routingKey, bool mandatory, IBasicProperties basicProperties, byte[] body) - - var segment = (RabbitMqHelper.GetRabbitMQVersion(instrumentedMethodCall) >= 6) ? - RabbitMqHelper.CreateSegmentForPublishWrappers6Plus(instrumentedMethodCall, transaction, BasicPropertiesIndex, agent) : - RabbitMqHelper.CreateSegmentForPublishWrappers(instrumentedMethodCall, transaction, BasicPropertiesIndex, agent); - - return Delegates.GetDelegateFor(segment); + // v7+: + // public async ValueTask BasicPublishAsync(string exchange, string routingKey, + // bool mandatory, TProperties basicProperties, ReadOnlyMemory body, + // CancellationToken cancellationToken = default) where TProperties : IReadOnlyBasicProperties, IAmqpHeader + var rabbitMqVersion = RabbitMqHelper.GetRabbitMQVersion(instrumentedMethodCall); + + var segment = (rabbitMqVersion >= 6) ? + RabbitMqHelper.CreateSegmentForPublishWrappers6Plus(instrumentedMethodCall, transaction, agent) + : + RabbitMqHelper.CreateSegmentForPublishWrappers(instrumentedMethodCall, transaction, agent); + + if (rabbitMqVersion >= 6) + RabbitMqHelper.InsertDTHeaders6Plus(instrumentedMethodCall, transaction, BasicPropertiesIndex); + else + RabbitMqHelper.InsertDTHeaders(instrumentedMethodCall, transaction, BasicPropertiesIndex); + + + // TODO: probably need to do something special for v7 since the return type is ValueTask + //return Delegates.GetDelegateFor(segment); + return Delegates.NoOp; } } } diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapperLegacy.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapperLegacy.cs index 8cde703189..fc91ff099a 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapperLegacy.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapperLegacy.cs @@ -22,7 +22,7 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) { // 3.5.X (IModel)void BasicPublish(string exchange, string routingKey, bool mandatory, bool immediate, IBasicProperties basicProperties, byte[] body) - var segment = RabbitMqHelper.CreateSegmentForPublishWrappers(instrumentedMethodCall, transaction, BasicPropertiesIndex, agent); + var segment = RabbitMqHelper.CreateSegmentForPublishWrappers(instrumentedMethodCall, transaction, agent); return Delegates.GetDelegateFor(segment); } } diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml index 5a396a575e..4a63fa3025 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml @@ -5,41 +5,71 @@ SPDX-License-Identifier: Apache-2.0 --> - + - - - - - + + + + + + + + + - - - - - - + + + + + + + + + + - - - - - + + + + + - + - - - - - - - + + + + + - + + + + + + + + + + diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs index 714a59a71c..1ce9437a44 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs @@ -21,6 +21,7 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) { // (IModel) uint QueuePurge(string queue) + // Task QueuePurgeAsync(string queue, CancellationToken cancellationToken) var queue = instrumentedMethodCall.MethodCall.MethodArguments.ExtractNotNullAs(0); var destType = RabbitMqHelper.GetBrokerDestinationType(queue); var destName = RabbitMqHelper.ResolveDestinationName(destType, queue); diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/RabbitMqHelper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/RabbitMqHelper.cs index 306946d03c..6795f43fe7 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/RabbitMqHelper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/RabbitMqHelper.cs @@ -3,12 +3,10 @@ using System; using System.Collections.Generic; -using System.Text; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Reflection; using NewRelic.Agent.Extensions.SystemExtensions; -using System.Reflection; namespace NewRelic.Providers.Wrapper.RabbitMq { @@ -16,6 +14,7 @@ public class RabbitMqHelper { private const string TempQueuePrefix = "amq."; private const string BasicPropertiesType = "RabbitMQ.Client.Framing.BasicProperties"; + private const string BasicProperties7PlusType = "RabbitMQ.Client.BasicProperties"; public const string VendorName = "RabbitMQ"; public const string AssemblyName = "RabbitMQ.Client"; public const string TypeName = "RabbitMQ.Client.Framing.Impl.Model"; @@ -36,6 +35,22 @@ public static IDictionary GetHeaders(object properties) var func = _getHeadersFunc ?? (_getHeadersFunc = VisibilityBypasser.Instance.GeneratePropertyAccessor(properties.GetType(), "Headers")); return func(properties) as IDictionary; } +#nullable enable + private static Func? _getHeaders7PlusFunc = null; + public static IDictionary? GetHeaders7Plus(object properties) + { + // v7: public IDictionary? Headers { get; set; } + var func = _getHeaders7PlusFunc ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(properties.GetType(), "Headers"); + return func(properties) as IDictionary; + } + public static void SetHeaders7Plus(object properties, IDictionary headers) + { + // Unlike the GetHeaders function, we can't cache this action. It is only valid for the specific Properties object instance provided. + var action = VisibilityBypasser.Instance.GeneratePropertySetter>(properties, "Headers"); + + action(headers); + } +#nullable disable public static void SetHeaders(object properties, IDictionary headers) { @@ -62,12 +77,10 @@ public static string ResolveDestinationName(MessageBrokerDestinationType destina : queueNameOrRoutingKey; } - public static ISegment CreateSegmentForPublishWrappers(InstrumentedMethodCall instrumentedMethodCall, ITransaction transaction, int basicPropertiesIndex, IAgent agent) + public static ISegment CreateSegmentForPublishWrappers(InstrumentedMethodCall instrumentedMethodCall, ITransaction transaction, IAgent agent) { // ATTENTION: We have validated that the use of dynamic here is appropriate based on the visibility of the data we're working with. // If we implement newer versions of the API or new methods we'll need to re-evaluate. - // never null. Headers property can be null. - var basicProperties = instrumentedMethodCall.MethodCall.MethodArguments.ExtractAs(basicPropertiesIndex); var routingKey = instrumentedMethodCall.MethodCall.MethodArguments.ExtractNotNullAs(1); var destType = GetBrokerDestinationType(routingKey); @@ -83,10 +96,15 @@ public static ISegment CreateSegmentForPublishWrappers(InstrumentedMethodCall in serverPort: GetServerPort(instrumentedMethodCall, agent), routingKey: routingKey); + return segment; + } + public static void InsertDTHeaders(InstrumentedMethodCall instrumentedMethodCall, ITransaction transaction, int basicPropertiesIndex) + { + var basicProperties = instrumentedMethodCall.MethodCall.MethodArguments.ExtractAs(basicPropertiesIndex); //If the RabbitMQ version doesn't provide the BasicProperties parameter we just bail. if (basicProperties.GetType().FullName != BasicPropertiesType) { - return segment; + return; } var setHeaders = new Action((carrier, key, value) => @@ -109,13 +127,10 @@ public static ISegment CreateSegmentForPublishWrappers(InstrumentedMethodCall in transaction.InsertDistributedTraceHeaders(basicProperties, setHeaders); - return segment; } - public static ISegment CreateSegmentForPublishWrappers6Plus(InstrumentedMethodCall instrumentedMethodCall, ITransaction transaction, int basicPropertiesIndex, IAgent agent) + public static ISegment CreateSegmentForPublishWrappers6Plus(InstrumentedMethodCall instrumentedMethodCall, ITransaction transaction, IAgent agent) { - var basicProperties = instrumentedMethodCall.MethodCall.MethodArguments.ExtractAs(basicPropertiesIndex); - var routingKey = instrumentedMethodCall.MethodCall.MethodArguments.ExtractNotNullAs(1); var destType = GetBrokerDestinationType(routingKey); var destName = ResolveDestinationName(destType, routingKey); @@ -130,34 +145,72 @@ public static ISegment CreateSegmentForPublishWrappers6Plus(InstrumentedMethodCa serverPort: GetServerPort(instrumentedMethodCall, agent), routingKey: routingKey); - //If the RabbitMQ version doesn't provide the BasicProperties parameter we just bail. - if (basicProperties.GetType().FullName != BasicPropertiesType) - { - - return segment; - } + return segment; + } - var setHeaders = new Action((carrier, key, value) => + public static void InsertDTHeaders6Plus(InstrumentedMethodCall instrumentedMethodCall, ITransaction transaction, int basicPropertiesIndex) + { + // v7+ basicProperties type is IReadOnlyBasicProperties + var basicProperties = instrumentedMethodCall.MethodCall.MethodArguments.ExtractAs(basicPropertiesIndex); + if (GetRabbitMQVersion(instrumentedMethodCall) >= 7) { - var headers = GetHeaders(carrier); - - if (headers == null) + // in v7, the properties property can sometimes be `EmptyBasicProperty` which + // can't be modified. + // So for now, if the property isn't a `BasicProperties` type, we just bail + if (basicProperties.GetType().FullName != BasicProperties7PlusType) { - headers = new Dictionary(); - SetHeaders(carrier, headers); + return; } - else if (headers is IReadOnlyDictionary) +#nullable enable + var setHeaders = new Action((carrier, key, value) => + { + var headers = GetHeaders7Plus(carrier); + + if (headers == null) + { + headers = new Dictionary(); + SetHeaders7Plus(carrier, headers); + } + else if (headers is IReadOnlyDictionary) + { + headers = new Dictionary(headers); + SetHeaders7Plus(carrier, headers); + } + + headers[key] = value; + }); + transaction.InsertDistributedTraceHeaders(basicProperties, setHeaders); +#nullable disable + } + else // v6 + { + //If the RabbitMQ version doesn't provide the BasicProperties parameter we just bail. + if (basicProperties.GetType().FullName != BasicPropertiesType) { - headers = new Dictionary(headers); - SetHeaders(carrier, headers); - } - - headers[key] = value; - }); - transaction.InsertDistributedTraceHeaders(basicProperties, setHeaders); + return; + } - return segment; + var setHeaders = new Action((carrier, key, value) => + { + var headers = GetHeaders(carrier); + + if (headers == null) + { + headers = new Dictionary(); + SetHeaders(carrier, headers); + } + else if (headers is IReadOnlyDictionary) + { + headers = new Dictionary(headers); + SetHeaders(carrier, headers); + } + + headers[key] = value; + }); + + transaction.InsertDistributedTraceHeaders(basicProperties, setHeaders); + } } public static int GetRabbitMQVersion(InstrumentedMethodCall methodCall) From badab6a4f362ed1e6a88b14fa43f48610e3f0ed1 Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Mon, 18 Nov 2024 11:37:04 -0600 Subject: [PATCH 2/8] HandleBasicDeliverWrapper working --- .../RabbitMq/HandleBasicDeliverWrapper.cs | 36 +++++++++++-------- .../Wrapper/RabbitMq/Instrumentation.xml | 11 +++--- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs index 68eaa43c57..ea221d3c08 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs @@ -40,6 +40,7 @@ public CanWrapResponse CanWrap(InstrumentedMethodInfo methodInfo) public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall instrumentedMethodCall, IAgent agent, ITransaction transaction) { // (IBasicConsumer) void HandleBasicDeliver(string consumerTag, ulong deliveryTag, bool redelivered, string exchange, string routingKey, IBasicProperties properties, byte[] body) + // (V7 IAsyncBasicConsumer) Task HandleBasicDeliverAsync(string consumerTag, ulong deliveryTag, bool redelivered, string exchange, string routingKey, IReadOnlyBasicProperties properties, ReadOnlyMemory body, CancellationToken cancellationToken = default) var routingKey = instrumentedMethodCall.MethodCall.MethodArguments.ExtractNotNullAs(4); var destType = RabbitMqHelper.GetBrokerDestinationType(routingKey); var destName = RabbitMqHelper.ResolveDestinationName(destType, routingKey); @@ -107,14 +108,22 @@ private void GetServerDetails(InstrumentedMethodCall instrumentedMethodCall, out try { - _modelGetter ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(instrumentedMethodCall.MethodCall.InvocationTarget.GetType(), "Model"); + // v7 renamed "model" to "channel" + if (RabbitMqHelper.GetRabbitMQVersion(instrumentedMethodCall.MethodCall.InvocationTarget.GetType()) >= 7) + { + _modelGetter ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(instrumentedMethodCall.MethodCall.InvocationTarget.GetType(), "Channel"); + } + else + { + _modelGetter ??= VisibilityBypasser.Instance.GeneratePropertyAccessor(instrumentedMethodCall.MethodCall.InvocationTarget.GetType(), "Model"); + } var model = _modelGetter(instrumentedMethodCall.MethodCall.InvocationTarget); object connection = null; var modelType = model.GetType(); var connectionGetter = _connectionGetter.GetOrAdd(modelType, GetConnectionForType); if (connectionGetter != null) - { + { connection = connectionGetter(modelType, model); } @@ -138,20 +147,17 @@ private void GetServerDetails(InstrumentedMethodCall instrumentedMethodCall, out static Func GetConnectionForType(Type modelType) { var version = RabbitMqHelper.GetRabbitMQVersion(modelType); // caches version in RabbitMqHelper. - if (modelType.ToString() == "RabbitMQ.Client.Framing.Impl.Model") + return modelType.ToString() switch { - return GetConnectionFromFramingModel; - } - else if (modelType.ToString() == "RabbitMQ.Client.Impl.AutorecoveringModel" && version <= 5) - { - return GetConnectionFromAutorecoveryModel5OrOlder; - } - else if (modelType.ToString() == "RabbitMQ.Client.Impl.AutorecoveringModel" && version >= 6) - { - return GetConnectionFromAutorecoveryModel6OrNewer; - } - - return null; + "RabbitMQ.Client.Framing.Impl.Model" => GetConnectionFromFramingModel, + "RabbitMQ.Client.Impl.AutorecoveringModel" when version <= 5 => + GetConnectionFromAutorecoveryModel5OrOlder, + "RabbitMQ.Client.Impl.AutorecoveringModel" when version == 6 => + GetConnectionFromAutorecoveryModel6OrNewer, + "RabbitMQ.Client.Impl.AutorecoveringChannel" when version >= 7 => + GetConnectionFromAutorecoveryModel6OrNewer, + _ => null + }; } static object GetConnectionFromFramingModel(Type modelType, object model) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml index 4a63fa3025..53c2dcd320 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml @@ -51,11 +51,12 @@ SPDX-License-Identifier: Apache-2.0 - - + RabbitMQ v7+ + public override Task HandleBasicDeliverAsync(string consumerTag, ulong deliveryTag, bool redelivered, string exchange, string routingKey, + IReadOnlyBasicProperties properties, ReadOnlyMemory body, CancellationToken cancellationToken = default) + --> + + From de3a017746163663eed6e85e901e94d03a9b5584 Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Mon, 18 Nov 2024 16:12:47 -0600 Subject: [PATCH 3/8] Code cleanup --- .../Wrapper/RabbitMq/BasicPublishWrapper.cs | 8 +++-- .../RabbitMq/HandleBasicDeliverWrapper.cs | 30 ++++++++++++++----- .../Wrapper/RabbitMq/Instrumentation.xml | 2 +- .../Wrapper/RabbitMq/QueuePurgeWrapper.cs | 3 +- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs index 92f836f111..0a493e289a 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using System; +using System.Threading.Tasks; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; @@ -40,9 +41,10 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins RabbitMqHelper.InsertDTHeaders(instrumentedMethodCall, transaction, BasicPropertiesIndex); - // TODO: probably need to do something special for v7 since the return type is ValueTask - //return Delegates.GetDelegateFor(segment); - return Delegates.NoOp; + // TODO: probably need to do something special for v7 since the return type is ValueTask + return instrumentedMethodCall.IsAsync ? + Delegates.GetAsyncDelegateFor(agent, segment) + : Delegates.GetDelegateFor(segment); } } } diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs index ea221d3c08..d551b349de 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/HandleBasicDeliverWrapper.cs @@ -5,6 +5,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Text; +using System.Threading.Tasks; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Extensions.SystemExtensions; @@ -70,13 +71,28 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins serverPort: port, routingKey: routingKey); - return Delegates.GetDelegateFor( - onFailure: transaction.NoticeError, - onComplete: () => - { - segment.End(); - transaction.End(); - }); + return instrumentedMethodCall.IsAsync + ? Delegates.GetAsyncDelegateFor( + agent, + segment, + false, + onComplete: (t) => + { + if (t.IsFaulted) + { + transaction.NoticeError(t.Exception); + } + + segment.End(); + transaction.End(); + }) + : Delegates.GetDelegateFor( + onFailure: transaction.NoticeError, + onComplete: () => + { + segment.End(); + transaction.End(); + }); IEnumerable GetHeaderValue(IDictionary carrier, string key) { diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml index 53c2dcd320..0c7c5f630f 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/Instrumentation.xml @@ -15,7 +15,7 @@ SPDX-License-Identifier: Apache-2.0 RabbitMQ v7+ public async Task BasicGetAsync(string queue, bool autoAck, CancellationToken cancellationToken) --> - + diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs index 1ce9437a44..8247d9b033 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/QueuePurgeWrapper.cs @@ -1,6 +1,7 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +using System.Threading.Tasks; using NewRelic.Agent.Api; using NewRelic.Agent.Extensions.Providers.Wrapper; using NewRelic.Agent.Extensions.SystemExtensions; @@ -38,7 +39,7 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins // Routing key is not available for this method. // It only returns uint and invocationTarget does not have the value. - return Delegates.GetDelegateFor(segment); + return instrumentedMethodCall.IsAsync ? Delegates.GetAsyncDelegateFor(agent, segment) : Delegates.GetDelegateFor(segment); } } } From 06f3e844296ddc6945fbc530d599b8da98c47d24 Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Mon, 18 Nov 2024 16:13:27 -0600 Subject: [PATCH 4/8] Integration test updates --- .../MFALatestPackages.csproj | 4 +- .../{RabbitMQ.cs => RabbitMQ6AndOlder.cs} | 6 +- .../NetStandardLibraries/RabbitMQ7AndNewer.cs | 307 ++++++++++++++++++ .../RabbitMqDistributedTracingTests.cs | 24 +- .../RabbitMq/RabbitMqTests.cs | 25 +- .../RabbitMq/RabbitMqW3cTracingTests.cs | 122 +++++-- 6 files changed, 451 insertions(+), 37 deletions(-) rename tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/{RabbitMQ.cs => RabbitMQ6AndOlder.cs} (98%) create mode 100644 tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ7AndNewer.cs diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj index b72937d49d..bf58bf164d 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MFALatestPackages/MFALatestPackages.csproj @@ -75,8 +75,8 @@ - - + + diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ6AndOlder.cs similarity index 98% rename from tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ.cs rename to tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ6AndOlder.cs index b60deaacb0..a14f958ab7 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ6AndOlder.cs @@ -1,6 +1,8 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +#if !NET8_0 && !NET481 // .NET 8 and FW481 are tested by RabbitMQ6AndNewer + // See this project's .csproj file for target framework => RabbitMQ.Client version mappings #if NET48_OR_GREATER || NET6_0_OR_GREATER #define RABBIT6PLUS @@ -24,7 +26,7 @@ namespace MultiFunctionApplicationHelpers.NetStandardLibraries { [Library] - class RabbitMQ + class RabbitMQ6AndOlder { private static readonly ConnectionFactory ChannelFactory = new ConnectionFactory() { HostName = RabbitMqConfiguration.RabbitMqServerIp, @@ -270,6 +272,6 @@ private void VerifyHeaders(IDictionary headers) } } } - } } +#endif diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ7AndNewer.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ7AndNewer.cs new file mode 100644 index 0000000000..a9702e2591 --- /dev/null +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ7AndNewer.cs @@ -0,0 +1,307 @@ +// Copyright 2020 New Relic, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#if NET8_0 || NET481 // Other TFMs are tested in RabbitMQ6AndOlder +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Diagnostics; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using NewRelic.Agent.IntegrationTests.Shared; +using NewRelic.Agent.IntegrationTests.Shared.ReflectionHelpers; +using NewRelic.Api.Agent; +using RabbitMQ.Client; +using RabbitMQ.Client.Events; + +namespace MultiFunctionApplicationHelpers.NetStandardLibraries +{ + [Library] + public class RabbitMQ7AndNewer + { + private IConnection _connection; + private IChannel _channel; + private bool _initialized; + + private static readonly ConnectionFactory ConnectionFactory = new() + { + HostName = RabbitMqConfiguration.RabbitMqServerIp, + UserName = RabbitMqConfiguration.RabbitMqUsername, + Password = RabbitMqConfiguration.RabbitMqPassword + }; + + // "User" headers to be set when publishing messages and then read when receiving them. + // This verifies that our instrumentation does not overwrite or modify user headers. + // A SortedDictionary is used to verify that the instrumentation interacts with the message + // headers through the IDictionary interface. + // See https://github.com/newrelic/newrelic-dotnet-agent/issues/639 for context + private static readonly IDictionary UserHeaders = new ReadOnlyDictionary(new SortedDictionary() { { "aNumber", 123 }, { "aString", "foo" } }); + + [LibraryMethod] + public async Task Initialize() + { + _connection = await ConnectionFactory.CreateConnectionAsync(); + _channel = await _connection.CreateChannelAsync(); + ConsoleMFLogger.Info("RabbitMQ channel and connection created."); + _initialized = true; + } + + [LibraryMethod] + public async Task Shutdown() + { + await _channel.CloseAsync(); + await _connection.CloseAsync(); + ConsoleMFLogger.Info("RabbitMQ channel and connection closed."); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public async Task SendReceive(string queueName, string message) + { + if (!_initialized) + { + throw new InvalidOperationException("RabbitMQ channel and connection not initialized."); + } + + if (string.IsNullOrEmpty(message)) { message = "Caller provided no message."; } + + await DeclareQueue(queueName); + + await BasicPublishMessage(queueName, message); + + string receiveMessage = await BasicGetMessage(queueName); + + await DeleteQueue(queueName); + + // This sleep ensures that this transaction method is the one sampled for transaction trace data + Thread.Sleep(1000); + + ConsoleMFLogger.Info( + $"method=SendReceive,sent message={message},received message={receiveMessage}, queueName={queueName}"); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public async Task SendReceiveWithEventingConsumer(string queueName, string message) + { + if (!_initialized) + { + throw new InvalidOperationException("RabbitMQ channel and connection not initialized."); + } + + if (string.IsNullOrEmpty(message)) { message = "Caller provided no message."; } + + await DeclareQueue(queueName); + + await BasicPublishMessage(queueName, message); + + string receiveMessage = await EventingConsumerGetMessage(queueName); + + await DeleteQueue(queueName); + + ConsoleMFLogger.Info( + $"method=SendReceiveWithEventingConsumer,sent message={message},received message={receiveMessage}, queueName={queueName}"); + } + + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public async Task SendReceiveTopic(string exchangeName, string topicName, string message) + { + if (!_initialized) + { + throw new InvalidOperationException("RabbitMQ channel and connection not initialized."); + } + + //Publish + await _channel.ExchangeDeclareAsync(exchange: exchangeName, type: "topic"); + + string routingKey = topicName; + byte[] body = Encoding.UTF8.GetBytes(message); + await _channel.BasicPublishAsync(exchange: exchangeName, + routingKey: routingKey, + mandatory: true, + body: body); + + //Consume + string queueName = (await _channel.QueueDeclareAsync()).QueueName; + + await _channel.QueueBindAsync(queue: queueName, + exchange: exchangeName, + routingKey: routingKey); + + BasicGetResult basicGetResult = await _channel.BasicGetAsync(queueName, true); + + //Cleanup + await DeleteExchange(exchangeName); + await DeleteQueue(queueName); + + ConsoleMFLogger.Info($"method=SendReceiveTopic,exchangeName={exchangeName},queueName={queueName},topicName={topicName},message={message},basicGetResult={basicGetResult}"); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public async Task SendReceiveTempQueue(string message) + { + if (!_initialized) + { + throw new InvalidOperationException("RabbitMQ channel and connection not initialized."); + } + + string queueName = (await _channel.QueueDeclareAsync()).QueueName; + + await BasicPublishMessage(queueName, message); + + string resultMessage = await BasicGetMessage(queueName); + + ConsoleMFLogger.Info($"method=SendReceiveTempQueue,queueName={queueName},sent message={message},received message={resultMessage}"); + } + + [LibraryMethod] + [Transaction] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + public async Task QueuePurge(string queueName) + { + if (!_initialized) + { + throw new InvalidOperationException("RabbitMQ channel and connection not initialized."); + } + + await DeclareQueue(queueName); + + await BasicPublishMessage(queueName, "I will be purged"); + + uint countMessages = await _channel.QueuePurgeAsync(queueName); + + return $"Purged {countMessages} message from queue: {queueName}"; + } + + public async Task DeleteQueue(string queueName) + { + await _channel.QueueDeleteAsync(queueName, false, false); + } + + public async Task DeleteExchange(string exchangeName) + { + await _channel.ExchangeDeleteAsync(exchangeName, false); + } + + [LibraryMethod] + public void PrintVersion() + { + Assembly rabbitAssembly = Assembly.GetAssembly(typeof(ConnectionFactory)); + string assemblyPath = rabbitAssembly.Location; + string rabbitClientVersion = FileVersionInfo.GetVersionInfo(assemblyPath).FileVersion; + + ConsoleMFLogger.Info($"RabbitMQ client assembly path={assemblyPath}, version={rabbitClientVersion}"); + } + + private async Task DeclareQueue(string queueName) + { + await _channel.QueueDeclareAsync(queue: queueName, + durable: false, + exclusive: false, + autoDelete: false, + arguments: null); + } + + private async Task BasicPublishMessage(string queueName, string message) + { + + byte[] body = Encoding.UTF8.GetBytes(message); + BasicProperties props = new BasicProperties { Headers = UserHeaders }; + + await _channel.BasicPublishAsync(exchange: "", + routingKey: queueName, + mandatory: true, + basicProperties: props, + body: body); + } + + private async Task BasicGetMessage(string queueName) + { + BasicGetResult basicGetResult = await _channel.BasicGetAsync(queueName, true); + + if (basicGetResult != null) + { + VerifyHeaders(basicGetResult.BasicProperties.Headers); + + string receiveMessage = Encoding.UTF8.GetString(basicGetResult.Body.ToArray()); + + return receiveMessage; + } + + return null; + } + + private async Task EventingConsumerGetMessage(string queueName) + { + using (ManualResetEventSlim manualResetEvent = new ManualResetEventSlim(false)) + { + string receivedMessage = string.Empty; + AsyncEventingBasicConsumer consumer = new AsyncEventingBasicConsumer(_channel); + consumer.ReceivedAsync += handler; + + await _channel.BasicConsumeAsync(queueName, true, consumer); + manualResetEvent.Wait(); + return receivedMessage; + + async Task handler(object ch, BasicDeliverEventArgs basicDeliverEventArgs) + { + receivedMessage = Encoding.UTF8.GetString(basicDeliverEventArgs.Body.ToArray()); + + VerifyHeaders(basicDeliverEventArgs.BasicProperties.Headers); + + await InstrumentedChildMethod(); // to verify we're getting a child span + + manualResetEvent.Set(); + } + } + } + + [Trace] + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] + private async Task InstrumentedChildMethod() + { + await Task.Delay(100); + } + + private void VerifyHeaders(IDictionary headers) + { + foreach (KeyValuePair userHeader in UserHeaders) + { + object objectFromMessageHeaders; + if (headers.TryGetValue(userHeader.Key, out objectFromMessageHeaders)) + { + bool headerValuesMatch = true; + if (objectFromMessageHeaders.GetType() == typeof(byte[])) + { + //RabbitMQ encodes strings as byte arrays when sending messages + string decodedString = Encoding.UTF8.GetString((byte[])objectFromMessageHeaders); + headerValuesMatch = (string)userHeader.Value == decodedString; + } + else + { + headerValuesMatch = userHeader.Value.Equals(objectFromMessageHeaders); + } + if (!headerValuesMatch) + { + throw new Exception("Header value in received message does not match expected value."); + } + } + else + { + throw new Exception("Did not find expected user header value in received message."); + } + } + } + } +} +#endif diff --git a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqDistributedTracingTests.cs b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqDistributedTracingTests.cs index 32a4b1ef0b..c165ea5553 100644 --- a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqDistributedTracingTests.cs +++ b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqDistributedTracingTests.cs @@ -17,20 +17,32 @@ public abstract class RabbitMqDistributedTracingTestsBase : NewRelicIn { private readonly string _sendReceiveQueue = $"integrationTestQueue-{Guid.NewGuid()}"; private ConsoleDynamicMethodFixture _fixture; + private string _exerciser; public RabbitMqDistributedTracingTestsBase(TFixture fixture, ITestOutputHelper output) : base(fixture) { _fixture = fixture; fixture.TestLogger = output; + // if fixture is FWLatest or CoreLatest, set _exerciser to RabbitMQ7AndNewer else set it to RabbitMQ6AndOlder + if (fixture.GetType().Name.Contains("FWLatest") || fixture.GetType().Name.Contains("CoreLatest")) + { + _exerciser = "RabbitMQ7AndNewer"; + _fixture.AddCommand($"{_exerciser} Initialize"); + } + else + { + _exerciser = "RabbitMQ6AndOlder"; + } + // RabbitMQ SendRecieve uses the BasicGet method to receive, which does not process incoming tracing payloads - _fixture.AddCommand($"RabbitMQ SendReceive {_sendReceiveQueue} TestMessage"); + _fixture.AddCommand($"{_exerciser} SendReceive {_sendReceiveQueue} TestMessage"); // RabbitMQ SendRecieveWithEventingConsumer uses the HandleBasicDeliverWrapper on the receiving side, which does process incoming tracing headers // We execute the method twice to make sure this issue stays fixed: https://github.com/newrelic/newrelic-dotnet-agent/issues/464 - _fixture.AddCommand($"RabbitMQ SendReceiveWithEventingConsumer {_sendReceiveQueue} EventingConsumerTestMessageOne"); - _fixture.AddCommand($"RabbitMQ SendReceiveWithEventingConsumer {_sendReceiveQueue} EventingConsumerTestMessageTwo"); + _fixture.AddCommand($"{_exerciser} SendReceiveWithEventingConsumer {_sendReceiveQueue} EventingConsumerTestMessageOne"); + _fixture.AddCommand($"{_exerciser} SendReceiveWithEventingConsumer {_sendReceiveQueue} EventingConsumerTestMessageTwo"); // This is needed to avoid a hang on shutdown in the test app - _fixture.AddCommand("RabbitMQ Shutdown"); + _fixture.AddCommand($"{_exerciser} Shutdown"); fixture.Actions ( @@ -54,8 +66,8 @@ public void Test() new Assertions.ExpectedMetric { metricName = "Supportability/DistributedTrace/CreatePayload/Success", callCount = 3 }, new Assertions.ExpectedMetric { metricName = "Supportability/TraceContext/Create/Success", callCount = 3 }, new Assertions.ExpectedMetric { metricName = "Supportability/TraceContext/Accept/Success", callCount = 2 }, - new Assertions.ExpectedMetric { metricName = "DotNet/MultiFunctionApplicationHelpers.NetStandardLibraries.RabbitMQ/InstrumentedChildMethod"} , - new Assertions.ExpectedMetric { metricName = "DotNet/MultiFunctionApplicationHelpers.NetStandardLibraries.RabbitMQ/InstrumentedChildMethod", metricScope = "OtherTransaction/Message/RabbitMQ/Queue/Named/integrationTestQueue.*", IsRegexScope = true} + new Assertions.ExpectedMetric { metricName = $"DotNet/MultiFunctionApplicationHelpers.NetStandardLibraries.{_exerciser}/InstrumentedChildMethod"} , + new Assertions.ExpectedMetric { metricName = $"DotNet/MultiFunctionApplicationHelpers.NetStandardLibraries.{_exerciser}/InstrumentedChildMethod", metricScope = "OtherTransaction/Message/RabbitMQ/Queue/Named/integrationTestQueue.*", IsRegexScope = true} }; var metrics = _fixture.AgentLog.GetMetrics(); diff --git a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqTests.cs b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqTests.cs index 7fdf6ed135..c9c30c495e 100644 --- a/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqTests.cs +++ b/tests/Agent/IntegrationTests/UnboundedIntegrationTests/RabbitMq/RabbitMqTests.cs @@ -24,20 +24,33 @@ public abstract class RabbitMqTestsBase : NewRelicIntegrationTest + public abstract class RabbitMqW3cTracingTestBase : NewRelicIntegrationTest where TFixture : ConsoleDynamicMethodFixture { - protected readonly string _metricScopeBase = "OtherTransaction/Custom/MultiFunctionApplicationHelpers.NetStandardLibraries.RabbitMQ"; + protected readonly string _metricScopeBase; + protected readonly TFixture _fixture; + protected readonly string _exerciser; - public RabbitMqW3cTracingTests(ConsoleDynamicMethodFixtureFW471 fixture, ITestOutputHelper output) : base(fixture) + protected RabbitMqW3cTracingTestBase(TFixture fixture, ITestOutputHelper output) : base(fixture) { - fixture.TestLogger = output; - fixture.Actions + _fixture = fixture; + + // if fixture is FWLatest or CoreLatest, set _exerciser to RabbitMQ7AndNewer else set it to {_exerciser} + if (fixture.GetType().Name.Contains("FWLatest") || fixture.GetType().Name.Contains("CoreLatest")) + { + _exerciser = "RabbitMQ7AndNewer"; + _fixture.AddCommand($"{_exerciser} Initialize"); + } + else + { + _exerciser = "RabbitMQ6AndOlder"; + } + _metricScopeBase = $"OtherTransaction/Custom/MultiFunctionApplicationHelpers.NetStandardLibraries.{_exerciser}"; + + + _fixture.TestLogger = output; + _fixture.Actions ( setupConfiguration: () => { @@ -36,20 +52,16 @@ public RabbitMqW3cTracingTests(ConsoleDynamicMethodFixtureFW471 fixture, ITestOu } } - public class RabbitMqW3cTracingBasicTest : RabbitMqW3cTracingTests + public abstract class RabbitMqW3cTracingBasicTestBase : RabbitMqW3cTracingTestBase where TFixture : ConsoleDynamicMethodFixture { - private ConsoleDynamicMethodFixtureFW471 _fixture; - private readonly string _sendReceiveQueue = $"integrationTestQueue-{Guid.NewGuid()}"; - public RabbitMqW3cTracingBasicTest(ConsoleDynamicMethodFixtureFW471 fixture, ITestOutputHelper output) + protected RabbitMqW3cTracingBasicTestBase(TFixture fixture, ITestOutputHelper output) : base(fixture, output) { - _fixture = fixture; - - _fixture.AddCommand($"RabbitMQ SendReceive {_sendReceiveQueue} TestMessage"); + _fixture.AddCommand($"{_exerciser} SendReceive {_sendReceiveQueue} TestMessage"); // This is needed to avoid a hang on shutdown in the test app - _fixture.AddCommand("RabbitMQ Shutdown"); + _fixture.AddCommand($"{_exerciser} Shutdown"); fixture.Initialize(); } @@ -92,20 +104,52 @@ public void Test() } } - public class RabbitMqW3cTracingEventingConsumerTest : RabbitMqW3cTracingTests + [NetFrameworkTest] + public class RabbitMqW3cTracingBasicTestFW462 : RabbitMqW3cTracingBasicTestBase { - private ConsoleDynamicMethodFixtureFW471 _fixture; + public RabbitMqW3cTracingBasicTestFW462(ConsoleDynamicMethodFixtureFW462 fixture, ITestOutputHelper output) + : base(fixture, output) + { + } + } - private readonly string _sendReceiveQueue = $"integrationTestQueue-{Guid.NewGuid()}"; + [NetFrameworkTest] + public class RabbitMqW3cTracingBasicTestFWLatest : RabbitMqW3cTracingBasicTestBase + { + public RabbitMqW3cTracingBasicTestFWLatest(ConsoleDynamicMethodFixtureFWLatest fixture, ITestOutputHelper output) + : base(fixture, output) + { + } + } - public RabbitMqW3cTracingEventingConsumerTest(ConsoleDynamicMethodFixtureFW471 fixture, ITestOutputHelper output) + [NetCoreTest] + public class RabbitMqW3cTracingBasicTestCoreOldest : RabbitMqW3cTracingBasicTestBase + { + public RabbitMqW3cTracingBasicTestCoreOldest(ConsoleDynamicMethodFixtureCoreOldest fixture, ITestOutputHelper output) : base(fixture, output) { - _fixture = fixture; + } + } + + [NetCoreTest] + public class RabbitMqW3cTracingBasicTestCoreLatest : RabbitMqW3cTracingBasicTestBase + { + public RabbitMqW3cTracingBasicTestCoreLatest(ConsoleDynamicMethodFixtureCoreLatest fixture, ITestOutputHelper output) + : base(fixture, output) + { + } + } - _fixture.AddCommand($"RabbitMQ SendReceiveWithEventingConsumer {_sendReceiveQueue} TestMessage"); + public abstract class RabbitMqW3cTracingEventingConsumerTestBase : RabbitMqW3cTracingTestBase where TFixture: ConsoleDynamicMethodFixture + { + private readonly string _sendReceiveQueue = $"integrationTestQueue-{Guid.NewGuid()}"; + + protected RabbitMqW3cTracingEventingConsumerTestBase(TFixture fixture, ITestOutputHelper output) + : base(fixture, output) + { + _fixture.AddCommand($"{_exerciser} SendReceiveWithEventingConsumer {_sendReceiveQueue} TestMessage"); // This is needed to avoid a hang on shutdown in the test app - _fixture.AddCommand("RabbitMQ Shutdown"); + _fixture.AddCommand($"{_exerciser} Shutdown"); fixture.Initialize(); } @@ -163,4 +207,40 @@ public void Test() Assertions.MetricsExist(expectedMetrics, metrics); } } + + [NetFrameworkTest] + public class RabbitMqW3cTracingEventingConsumerTestFW462 : RabbitMqW3cTracingEventingConsumerTestBase + { + public RabbitMqW3cTracingEventingConsumerTestFW462(ConsoleDynamicMethodFixtureFW462 fixture, ITestOutputHelper output) + : base(fixture, output) + { + } + } + + [NetFrameworkTest] + public class RabbitMqW3cTracingEventingConsumerTestFWLatest : RabbitMqW3cTracingEventingConsumerTestBase + { + public RabbitMqW3cTracingEventingConsumerTestFWLatest(ConsoleDynamicMethodFixtureFWLatest fixture, ITestOutputHelper output) + : base(fixture, output) + { + } + } + + [NetCoreTest] + public class RabbitMqW3cTracingEventingConsumerTestCoreOldest : RabbitMqW3cTracingEventingConsumerTestBase + { + public RabbitMqW3cTracingEventingConsumerTestCoreOldest(ConsoleDynamicMethodFixtureCoreOldest fixture, ITestOutputHelper output) + : base(fixture, output) + { + } + } + + [NetCoreTest] + public class RabbitMqW3cTracingEventingConsumerTestCoreLatest : RabbitMqW3cTracingEventingConsumerTestBase + { + public RabbitMqW3cTracingEventingConsumerTestCoreLatest(ConsoleDynamicMethodFixtureCoreLatest fixture, ITestOutputHelper output) + : base(fixture, output) + { + } + } } From 4df10d66c1dd9eb32ea843e756c7c2a1840c65ed Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Mon, 18 Nov 2024 16:21:06 -0600 Subject: [PATCH 5/8] Add RabbitMQ.Client back to list of monitored packages --- .../scripts/nugetSlackNotifications/packageInfo.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json b/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json index 8d904016ab..2e379a2488 100644 --- a/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json +++ b/.github/workflows/scripts/nugetSlackNotifications/packageInfo.json @@ -116,11 +116,7 @@ "packageName": "nservicebus" }, { - "packageName": "rabbitmq.client", - "ignorePatch": true, - "ignoreMinor": true, - "ignoreMajor": true, - "ignoreReason": "Breaking major update. See https://github.com/newrelic/newrelic-dotnet-agent/issues/2885" + "packageName": "rabbitmq.client" }, { "packageName": "restsharp" From a11746f8bf76db47e3e1315b06c3d6a26312a9c2 Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:38:14 -0600 Subject: [PATCH 6/8] Refactoring and cleanup for .NET 9 --- .../MultiFunctionApplicationHelpers.csproj | 2 +- .../NetStandardLibraries/{ => RabbitMQ}/RabbitMQ6AndOlder.cs | 2 +- .../NetStandardLibraries/{ => RabbitMQ}/RabbitMQ7AndNewer.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/{ => RabbitMQ}/RabbitMQ6AndOlder.cs (99%) rename tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/{ => RabbitMQ}/RabbitMQ7AndNewer.cs (99%) diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj index 2d9179261f..ef4333af6f 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/MultiFunctionApplicationHelpers.csproj @@ -1,4 +1,4 @@ - + net8.0;net9.0;net462;net471;net48;net481 diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ6AndOlder.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs similarity index 99% rename from tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ6AndOlder.cs rename to tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs index a14f958ab7..6eca9f9abf 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ6AndOlder.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs @@ -1,7 +1,7 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -#if !NET8_0 && !NET481 // .NET 8 and FW481 are tested by RabbitMQ6AndNewer +#if !NET9_0 && !NET481 // .NET 8 and FW481 are tested by RabbitMQ6AndNewer // See this project's .csproj file for target framework => RabbitMQ.Client version mappings #if NET48_OR_GREATER || NET6_0_OR_GREATER diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ7AndNewer.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ7AndNewer.cs similarity index 99% rename from tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ7AndNewer.cs rename to tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ7AndNewer.cs index a9702e2591..f2fbc61a64 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ7AndNewer.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ7AndNewer.cs @@ -1,7 +1,7 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -#if NET8_0 || NET481 // Other TFMs are tested in RabbitMQ6AndOlder +#if NET9_0 || NET481 // Other TFMs are tested in RabbitMQ6AndOlder using System; using System.Collections.Generic; using System.Collections.ObjectModel; From f61690162d0c7cd30333fc427031086063b4e607 Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:38:43 -0600 Subject: [PATCH 7/8] cleanup --- .../NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs index 6eca9f9abf..7cf39ab27d 100644 --- a/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs +++ b/tests/Agent/IntegrationTests/SharedApplications/Common/MultiFunctionApplicationHelpers/NetStandardLibraries/RabbitMQ/RabbitMQ6AndOlder.cs @@ -1,7 +1,7 @@ // Copyright 2020 New Relic, Inc. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -#if !NET9_0 && !NET481 // .NET 8 and FW481 are tested by RabbitMQ6AndNewer +#if !NET9_0 && !NET481 // .NET 9 and FW481 are tested by RabbitMQ6AndNewer // See this project's .csproj file for target framework => RabbitMQ.Client version mappings #if NET48_OR_GREATER || NET6_0_OR_GREATER From e66819ade37efc86ff2b51795887b49bda03758c Mon Sep 17 00:00:00 2001 From: Marty Tippin <120425148+tippmar-nr@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:51:17 -0600 Subject: [PATCH 8/8] Trying to handle ValueTask in publish wrapper. Not going so well. --- .../Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs index 0a493e289a..206d536aaf 100644 --- a/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs +++ b/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/BasicPublishWrapper.cs @@ -41,9 +41,12 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins RabbitMqHelper.InsertDTHeaders(instrumentedMethodCall, transaction, BasicPropertiesIndex); - // TODO: probably need to do something special for v7 since the return type is ValueTask + // TODO: Can we handle ValueTask return type somehow? Without it, we can't properly manage a message broker segment that wraps the publish call return instrumentedMethodCall.IsAsync ? - Delegates.GetAsyncDelegateFor(agent, segment) + Delegates.GetAsyncDelegateFor(agent, segment, false, (_) => + { + segment.End(); // TODO: this never gets called because the delegate is never invoked + }) : Delegates.GetDelegateFor(segment); } }