Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change OTel attribute messaging.operation to messaging.operation.type #1716

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Change OTel attribute messaging.operation to messaging.operation.type #1716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d708a8
Change OTel attribute messaging.operation to messaging.operation.type
iinuwa Oct 30, 2024
f71273b
Update span operation names to comply with OTel semantic conventions
iinuwa Oct 30, 2024
c4d1045
Use const strings for OTel operation types
iinuwa Oct 30, 2024
fe9ccbe
Update tests for Activity source operation name and attribute value
iinuwa Oct 30, 2024
e2c246f
docs: Add clone and build steps to contributing guide
iinuwa Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,39 @@ Here's the recommended workflow:
If what you are going to work on is a substantial change, please first
ask the core team for their opinion on the [RabbitMQ users mailing list][rmq-users].

### Building Source

It is good practice to make sure you can build the project before making any
changes to confirm that your development environment is set up correctly.
Verifying that the tests pass is also a good practice (see
[RUNNING_TESTS.md](/RUNNING_TESTS.md)).

All together, this looks like:

* Linux

```shell
git clone --recurse-submodules https://github.com/rabbitmq/rabbitmq-dotnet-client
cd rabbitmq-dotnet-client

# On any Linux distribution with Docker installed
./.ci/ubuntu/gha-setup.sh toxiproxy pull

make build
make test
```

* Windows

Note that this will _NOT_ run toxiproxy tests.

```powershell
git clone --recurse-submodules https://github.com/rabbitmq/rabbitmq-dotnet-client
cd rabbitmq-dotnet-client
.\.ci\windows\gha-setup.ps1 # On Windows. Note that this installs RabbitMQ
.\build.ps1 -RunTests:$true
```

### Running Tests

See [RUNNING_TESTS.md](/RUNNING_TESTS.md).
Expand Down
25 changes: 14 additions & 11 deletions projects/RabbitMQ.Client/Impl/RabbitMQActivitySource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ public static class RabbitMQActivitySource
// https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/#messaging-attributes
internal const string MessageId = "messaging.message.id";
internal const string MessageConversationId = "messaging.message.conversation_id";
internal const string MessagingOperation = "messaging.operation";
internal const string MessagingOperationType = "messaging.operation.type";
internal const string MessagingOperationTypeSend = "send";
internal const string MessagingOperationTypeProcess = "process";
internal const string MessagingOperationTypeReceive = "receive";
internal const string MessagingSystem = "messaging.system";
internal const string MessagingDestination = "messaging.destination.name";
internal const string MessagingDestinationRoutingKey = "messaging.rabbitmq.destination.routing_key";
Expand Down Expand Up @@ -63,14 +66,14 @@ public static class RabbitMQActivitySource

Activity? activity = linkedContext == default
? s_publisherSource.StartRabbitMQActivity(
UseRoutingKeyAsOperationName ? $"{routingKey} publish" : "publish",
UseRoutingKeyAsOperationName ? $"{routingKey} {MessagingOperationTypeSend}" : MessagingOperationTypeSend,
Copy link

Choose a reason for hiding this comment

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

span name should be {operation name} {destination} - https://github.com/open-telemetry/semantic-conventions/blob/main/docs/messaging/messaging-spans.md#span-name.

I.e. publish stays (as rabbit-friendly operation name), but the order has changed and should now be publish {routingKey}

Or if you want to call it basic_publish instead of publish or anything else (language-agnostic) - go for it - happy to update otel semconv with your preference.

ActivityKind.Producer)
: s_publisherSource.StartLinkedRabbitMQActivity(
UseRoutingKeyAsOperationName ? $"{routingKey} publish" : "publish",
UseRoutingKeyAsOperationName ? $"{routingKey} {MessagingOperationTypeSend}" : MessagingOperationTypeSend,
ActivityKind.Producer, linkedContext);
if (activity != null && activity.IsAllDataRequested)
{
PopulateMessagingTags("publish", routingKey, exchange, 0, bodySize, activity);
PopulateMessagingTags(MessagingOperationTypeSend, routingKey, exchange, 0, bodySize, activity);
Copy link

Choose a reason for hiding this comment

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

we'll need to set operation name too - it's required now.

}

return activity;
Expand All @@ -85,12 +88,12 @@ public static class RabbitMQActivitySource
}

Activity? activity = s_subscriberSource.StartRabbitMQActivity(
UseRoutingKeyAsOperationName ? $"{queue} receive" : "receive",
UseRoutingKeyAsOperationName ? $"{queue} {MessagingOperationTypeReceive}" : MessagingOperationTypeReceive,
Copy link

Choose a reason for hiding this comment

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

Is this activity created for BasicGet ? If so, let's pick a rabbitMQ-friendly operation name for it.

See my comment on publish.

Also, the order has changed - it's now {operation_name} {destination}

ActivityKind.Consumer);
if (activity != null && activity.IsAllDataRequested)
{
activity
.SetTag(MessagingOperation, "receive")
.SetTag(MessagingOperationType, MessagingOperationTypeReceive)
Copy link

Choose a reason for hiding this comment

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

Suggested change
.SetTag(MessagingOperationType, MessagingOperationTypeReceive)
.SetTag(MessagingOperationName, "something that we'll pick for `BasicGet``")

.SetTag(MessagingDestination, "amq.default");
}

Expand All @@ -107,11 +110,11 @@ public static class RabbitMQActivitySource

// Extract the PropagationContext of the upstream parent from the message headers.
Activity? activity = s_subscriberSource.StartLinkedRabbitMQActivity(
UseRoutingKeyAsOperationName ? $"{routingKey} receive" : "receive", ActivityKind.Consumer,
UseRoutingKeyAsOperationName ? $"{routingKey} {MessagingOperationTypeReceive}" : MessagingOperationTypeReceive, ActivityKind.Consumer,
Copy link

Choose a reason for hiding this comment

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

same here

ContextExtractor(readOnlyBasicProperties));
if (activity != null && activity.IsAllDataRequested)
{
PopulateMessagingTags("receive", routingKey, exchange, deliveryTag, readOnlyBasicProperties,
PopulateMessagingTags(MessagingOperationTypeReceive, routingKey, exchange, deliveryTag, readOnlyBasicProperties,
bodySize, activity);
}

Expand All @@ -128,11 +131,11 @@ public static class RabbitMQActivitySource

// Extract the PropagationContext of the upstream parent from the message headers.
Activity? activity = s_subscriberSource.StartLinkedRabbitMQActivity(
UseRoutingKeyAsOperationName ? $"{routingKey} deliver" : "deliver",
UseRoutingKeyAsOperationName ? $"{routingKey} {MessagingOperationTypeProcess}" : MessagingOperationTypeProcess,
Copy link

Choose a reason for hiding this comment

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

this one is probably for consume, so let's do consume|basic_consume|... {routing_key} and add the operation name tag with the name we'll pick

ActivityKind.Consumer, ContextExtractor(basicProperties));
if (activity != null && activity.IsAllDataRequested)
{
PopulateMessagingTags("deliver", routingKey, exchange,
PopulateMessagingTags(MessagingOperationTypeProcess, routingKey, exchange,
deliveryTag, basicProperties, bodySize, activity);
}

Expand Down Expand Up @@ -174,7 +177,7 @@ private static void PopulateMessagingTags(string operation, string routingKey, s
ulong deliveryTag, int bodySize, Activity activity)
{
activity
.SetTag(MessagingOperation, operation)
.SetTag(MessagingOperationType, operation)
.SetTag(MessagingDestination, string.IsNullOrEmpty(exchange) ? "amq.default" : exchange)
.SetTag(MessagingDestinationRoutingKey, routingKey)
.SetTag(MessagingBodySize, bodySize);
Expand Down
4 changes: 2 additions & 2 deletions projects/Test/SequentialIntegration/TestActivitySource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ private static ActivityListener StartActivityListener(List<Activity> activities)
private void AssertActivityData(bool useRoutingKeyAsOperationName, string queueName,
List<Activity> activityList, bool isDeliver = false)
{
string childName = isDeliver ? "deliver" : "receive";
string childName = isDeliver ? "process" : "receive";
Activity[] activities = activityList.ToArray();
Assert.NotEmpty(activities);

Expand All @@ -418,7 +418,7 @@ private void AssertActivityData(bool useRoutingKeyAsOperationName, string queueN
}

Activity sendActivity = activities.First(x =>
x.OperationName == (useRoutingKeyAsOperationName ? $"{queueName} publish" : "publish") &&
x.OperationName == (useRoutingKeyAsOperationName ? $"{queueName} send" : "send") &&
x.GetTagItem(RabbitMQActivitySource.MessagingDestinationRoutingKey) is string routingKeyTag &&
routingKeyTag == $"{queueName}");
Activity receiveActivity = activities.Single(x =>
Expand Down
4 changes: 2 additions & 2 deletions projects/Test/SequentialIntegration/TestOpenTelemetry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ public async Task TestPublisherAndBasicGetActivityTags(bool useRoutingKeyAsOpera
private void AssertActivityData(bool useRoutingKeyAsOperationName, string queueName,
List<Activity> activityList, bool isDeliver = false, string baggageGuid = null)
{
string childName = isDeliver ? "deliver" : "receive";
string childName = isDeliver ? "process" : "receive";
Activity[] activities = activityList.ToArray();
Assert.NotEmpty(activities);
foreach (var item in activities)
Expand All @@ -354,7 +354,7 @@ private void AssertActivityData(bool useRoutingKeyAsOperationName, string queueN
}

Activity sendActivity = activities.First(x =>
x.OperationName == (useRoutingKeyAsOperationName ? $"{queueName} publish" : "publish") &&
x.OperationName == (useRoutingKeyAsOperationName ? $"{queueName} send" : "send") &&
x.GetTagItem(RabbitMQActivitySource.MessagingDestinationRoutingKey) is string routingKeyTag &&
routingKeyTag == $"{queueName}");
Activity receiveActivity = activities.Single(x =>
Expand Down
Loading