Skip to content

Conversation

aaguilartablada
Copy link
Contributor

Spans generated from this library don't have any tags to identify the source queue and other useful information. With this PR 4 tags are included:

  • source queue.
  • amqp channel.
  • body size
  • requeue count header

@aaguilartablada
Copy link
Contributor Author

aaguilartablada commented Apr 29, 2025

hello @vivekjilla @aloiva @manikantanallagatla

could you take a look at this pull request, please?

thanks!

@aloiva
Copy link
Contributor

aloiva commented May 2, 2025

Hi @aaguilartablada thank you for your contribution. Could you provide a little more description and use cases? Curious to know how this change effects telemetry.

@aaguilartablada
Copy link
Contributor Author

Hi @aaguilartablada thank you for your contribution. Could you provide a little more description and use cases? Curious to know how this change effects telemetry.

Of course. Here you can see a trace from Grafana Tempo:

image

As you can see, an application publishes a message and another application using this library consumes the message generating a "Trigger" span. The point is that "Span Attributes" is empty. The idea of this pull request is filling span attributes with some useful information.

@@ -107,6 +107,9 @@ public Task StartAsync(CancellationToken cancellationToken)
async Task ReceivedHandler(object model, BasicDeliverEventArgs args)
{
using Activity activity = RabbitMQActivitySource.StartActivity(args.BasicProperties);
activity?.AddTag("amqp.queue", this.queue);
activity?.AddTag("amqp.channel", this.channel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.channel is not a string so it cannot be used here. maybe we can have args.DeliveryTag instead.

@@ -118,6 +121,7 @@ async Task ReceivedHandler(object model, BasicDeliverEventArgs args)
args.BasicProperties.Headers ??= new Dictionary<string, object>();
args.BasicProperties.Headers.TryGetValue(RequeueCountHeaderName, out object headerValue);
int requeueCount = Convert.ToInt32(headerValue, CultureInfo.InvariantCulture) + 1;
activity?.AddTag("amqp.requeueCount", requeueCount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds vague since this would show requeueCount=1 even when the is message not requeued. i suggest modifying to

activity?.AddTag("amqp.currentRequeueCount", requeueCount-1);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants