RabbitMQ: NR agent version 8.39.2 overwrites message headers

[witoldsz]

Description
After upgrading NewRelic agent 8.37.0 to 8.39.2 our services started to fail due to missing headers in RabbitMQ messages.

Expected Behavior
New Relic should not overwrite application's headers with it's own.

Steps to Reproduce
Application written in F# 5, publishing messages looks like this (simplified)

let ch = conn.CreateModel()
let props = ch.CreateBasicProperties()
props.Persistent <- true
props.ContentType <- "application/json"
props.Headers <-
    Map.ofList [ "trace", trace |> Array.map (fun it -> it.ToString()) :> obj
                 "publisher", appName :> obj
                 "ts", now |> Utils.instantToISOString :> obj ]
ch.BasicPublish(exchange, routingKey, props, ReadOnlyMemory(Encoding.UTF8.GetBytes body))

Your Environment

• Linux 5.4
• dotnet 5.0
• RabbitMQ 3.6 server
• Nuget: "RabbitMQ.Client" Version="6.2.1"
• newrelic-netcore20-agent_8.39.2.0_amd64.tar.gz

Additional context
Nothing yet, work in progress…

[witoldsz]

I think I have some lead. Take a look at the RabbitMQ API:
https://rabbitmq.github.io/rabbitmq-dotnet-client/api/RabbitMQ.Client.IBasicProperties.html#RabbitMQ_Client_IBasicProperties_Headers
IDictionary<string, object> Headers { get; set; } this is IDictionary, not Dictionary).

However in:
https://github.com/newrelic/newrelic-dotnet-agent/blob/main/src/Agent/NewRelic/Agent/Extensions/Providers/Wrapper/RabbitMq/RabbitMqHelper.cs
there are expressions like:

Dictionary<string, object> headers = carrier.Headers as Dictionary<string, object>;
if (headers == null) …

Correct me if I am wrong, but it will always result in null unless the System.Collections.Generic.Dictionary<TKey,TValue> class is used by an application.

Now, in my case, the props.Headers <- Map.ofList [ … ] ← not a Dictionary, but still valid IDictionary instance.

[witoldsz]

What's surprising is: why does it work in 8.37.0… So, when I have upgraded NR to 8.39.2 – the only headers our application messages had: X-NewRelic-ID and X-NewRelic-Transaction, when using 8.37.0 the only headers are our own, i.e. publisher, trace and ts.

It looks like NR 8.37.0 does not intercept the:
BasicPublish(this IModel model, string exchange, string routingKey, IBasicProperties basicProperties, ReadOnlyMemory<byte> body)
at all and this is why our application does work with older version of the agent, and crashes with the newer.

[lstachowiak]

I have the same issue. Thanks for reporting! I hope it will be fixed soon! 🤞

[JcolemanNR]

Thank you for the bug report and investigation @witoldsz! As you identified, we did recently add instrumentation to some RabbitMQ calls.

The team discussed this issue in the standup today and we are looking at a fix.

[nr-ahemsath]

Thanks for writing this up @witoldsz. I agree with your analysis of what the issue is. Hopefully we can get a fix out soon.

[nr-ahemsath]

@witoldsz @lstachowiak We just merged a PR that should fix this issue, which auto-closed this issue. The fix will be available in the next release. I am not sure when we plan to do the next release, it might be as early as next week. Thanks again for the report and the analysis.

[lstachowiak]

thank you for such a quick response!!!

[witoldsz]

@nr-ahemsath thank you. I saw the PR and as far as I can tell, it will crash our application: #648 (comment)
However, I have never written a line of code in C# in my life, so I might be wrong on that. Please check it. Thanks!

[tehbio]

@witoldsz You're right - the FSharpMap dictionary is read-only. I've pushed some extra checks to handle this case in a new PR here.