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.

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

Adding messaging.rabbitmq.message.delivery_tag to the list of RabbitMQ specific tags #433

Merged
merged 6 commits into from
Feb 22, 2024

Conversation

stebet
Copy link
Contributor

@stebet stebet commented Oct 20, 2023

Changes

As requested by @lmolkova here, I'm adding a new RabbitMQ specific attribute: messaging.rabbitmq.delivery_tag. This tag and it's purpose is explained here: https://www.rabbitmq.com/confirms.html#consumer-acks-delivery-tags

Merge requirement checklist

Copy link
Member

@AlexanderWert AlexanderWert left a comment

Choose a reason for hiding this comment

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

@stebet please do not modify the markdown tables manually!

Instead, please add the new attribute in the corresponding model yaml file:

- id: messaging.rabbitmq

Then, run make table-generation to generate the markdown table automatically.

@stebet
Copy link
Contributor Author

stebet commented Oct 23, 2023

@stebet please do not modify the markdown tables manually!

Instead, please add the new attribute in the corresponding model yaml file:

- id: messaging.rabbitmq

Then, run make table-generation to generate the markdown table automatically.

Awesome. Missed this step when I was reading over the docs. Will fix :)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 23, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

docs/messaging/rabbitmq.md Outdated Show resolved Hide resolved
Copy link
Member

@arminru arminru left a comment

Choose a reason for hiding this comment

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

@pyohannes
Copy link
Contributor

@stebet Could you take a stab at #433 (comment)?

Then I think this would be ready to be merged.

@stebet
Copy link
Contributor Author

stebet commented Nov 9, 2023

@stebet Could you take a stab at #433 (comment)?

Then I think this would be ready to be merged.

Will do

model/trace/messaging.yaml Outdated Show resolved Hide resolved
model/trace/messaging.yaml Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Feb 3, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Feb 3, 2024
@joaopgrassi
Copy link
Member

Hi @stebet !

We changed how the CHANGELOG.md is managed. Please take a look at https://github.com/open-telemetry/semantic-conventions/blob/main/CONTRIBUTING.md#adding-a-changelog-entry to see what needs to be done. Sorry for the disruption.

@lmolkova
Copy link
Contributor

@stebet I took a liberty to update your PR to address comments, I hope you don't mind :)

@lmolkova
Copy link
Contributor

Also, I moved delivery_tag to message namespace, so it's messaging.rabbitmq.message.delivery_tag now. It feels wrong to have it in the destination attributes since it identifies delivery of a specific message.

@lmolkova
Copy link
Contributor

@open-telemetry/semconv-messaging-approvers could you please take another look?

@lmolkova lmolkova changed the title Adding messaging.rabbitmq.delivery_tag to the list of RabbitMQ specific tags Adding messaging.rabbitmq.message.delivery_tag to the list of RabbitMQ specific tags Feb 22, 2024
@stebet
Copy link
Contributor Author

stebet commented Feb 22, 2024

@stebet I took a liberty to update your PR to address comments, I hope you don't mind :)

Thanks! I've been super busy with other things, so thank you.

@joaopgrassi joaopgrassi merged commit 096596b into open-telemetry:main Feb 22, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants