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

Introduce common messaging.consumer.group.name and messaging.destination.subscription.name instead of per-system ones #815

Merged

Conversation

lmolkova
Copy link
Contributor

@lmolkova lmolkova commented Mar 14, 2024

Changes

Addresses consumer group and subscription name part of #797

Merge requirement checklist

@lmolkova lmolkova marked this pull request as ready for review March 14, 2024 01:37
@lmolkova lmolkova requested review from a team March 14, 2024 01:37
docs/messaging/rocketmq.md Outdated Show resolved Hide resolved
@lmolkova lmolkova force-pushed the messaging-consumer-group-common branch from 506dcc8 to d0f0cfc Compare March 14, 2024 01:45
@lmolkova
Copy link
Contributor Author

It sounds like we have two popular terms - "consumer group" and "subscription" to describe load/balancing + fan-out. Some systems even use both notions (RocketMQ).

I'm totally open to other naming suggestions depending on the feedback.

@lmolkova lmolkova changed the title Introduce common messaging.consumer.group.name instead of per-system ones Introduce common messaging.consumer.group.name instead of per-system ones Mar 14, 2024
.chloggen/815.yaml Outdated Show resolved Hide resolved
@lmolkova lmolkova marked this pull request as draft March 14, 2024 19:08
Copy link
Contributor

@pyohannes pyohannes left a comment

Choose a reason for hiding this comment

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

This looks fine to me.

I like that we actually not use the term "consumer group", but we have "group" as a sub-namespace of the "consumer" namespace. So this denotes a grouping of consumers, which can also apply to subscriptions.

docs/attributes-registry/messaging.md Outdated Show resolved Hide resolved
@pyohannes pyohannes linked an issue Apr 18, 2024 that may be closed by this pull request
@lmolkova lmolkova force-pushed the messaging-consumer-group-common branch from d0f0cfc to 2810697 Compare April 18, 2024 23:58
@lmolkova lmolkova changed the title Introduce common messaging.consumer.group.name instead of per-system ones Introduce common messaging.consumer.group.name and messaging.destination.subscription.name instead of per-system ones Apr 18, 2024
@lmolkova lmolkova marked this pull request as ready for review April 19, 2024 00:08
@lmolkova lmolkova force-pushed the messaging-consumer-group-common branch from 091ed64 to f9b94d5 Compare April 19, 2024 00:09
docs/messaging/messaging-spans.md Show resolved Hide resolved
model/registry/messaging.yaml Outdated Show resolved Hide resolved
@lmolkova lmolkova force-pushed the messaging-consumer-group-common branch from 8db3110 to b9d1120 Compare May 2, 2024 20:46
@lmolkova lmolkova mentioned this pull request May 2, 2024
3 tasks
Copy link

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 May 18, 2024
@lmolkova lmolkova removed the Stale label May 23, 2024
Copy link
Contributor

@pyohannes pyohannes left a comment

Choose a reason for hiding this comment

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

Thanks @lmolkova, I think that's good to go.

docs/messaging/messaging-spans.md Show resolved Hide resolved
model/registry/messaging.yaml Show resolved Hide resolved
docs/messaging/kafka.md Show resolved Hide resolved
@lmolkova lmolkova force-pushed the messaging-consumer-group-common branch from 22a3723 to 9b65eba Compare June 14, 2024 22:36
…ion.subscription.name instead of per-system ones
@lmolkova lmolkova force-pushed the messaging-consumer-group-common branch from 9b65eba to 0f7fbdb Compare June 19, 2024 19:06
@lmolkova lmolkova merged commit c195520 into open-telemetry:main Jun 19, 2024
12 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
Status: V1 - Stable Semantics
Development

Successfully merging this pull request may close these issues.

Messaging: generalize common attributes to future-proof metrics
5 participants