Tracing: add useful tags to Trigger spans #253
Conversation
aaguilartablada
requested review from
vivekjilla,
aloiva and
manikantanallagatla
as code owners
|
hello @vivekjilla @aloiva @manikantanallagatla could you take a look at this pull request, please? thanks! |
|
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:
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); | |||
There was a problem hiding this comment.
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); | |||
There was a problem hiding this comment.
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);
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: