Add consumer_id to identify the consumer
#1810
Conversation
| @@ -57,6 +57,14 @@ groups: | |||
| The [conversation ID](#conversations) identifying the conversation to which the message belongs, | |||
| represented as a string. Sometimes called "Correlation ID". | |||
| examples: 'MyConversationId' | |||
| - id: consumer_id | |||
There was a problem hiding this comment.
Should this be expanded to client_id to include producerId as well?
There was a problem hiding this comment.
Maybe we should consider that we currently have the used queue/topic/... as destination/destination_kind. Which also is not named perfectly, but the other way round.
I think client_id would make sense here, even if it only applicable for consumers in Kafka. But no strong opinion.
There was a problem hiding this comment.
Of course, the description should be updated accordingly to also allow a "sender ID".
There was a problem hiding this comment.
Does a "sender ID" provide any differentiation that isn't already present?
My understanding is a single message can only be sent from a single sender or producer, which enables the process detail attributes to be used for uniquely identifying where the message came from.
With consumption, the same message can be consumed by many different consumers, either in near real-time or significantly delayed. It's for this reason I'd only focused on the consumption side for a unique identifier as it's possible to use the same process/service to consume the message and they are identified differently in terms of Kafka, or other brokers.
There was a problem hiding this comment.
which enables the process detail attributes to be used for uniquely identifying where the message came from.
That makes sense, but doesn't the same apply, conceptually, to the consumer side? Or is it more of a "consumption" ID than a "consumer" ID?
There was a problem hiding this comment.
This is currently in the general messaging semantic conventions that apply to both consumers and producers. But I assume that consumer_id only applies to consumer spans, not producer spans. So I'd put in the messaging.consumer semantic convention group, a bit down below in the YAML file (https://github.com/open-telemetry/opentelemetry-specification/pull/1810/files#diff-6ea24809c25647f7e2f067dfc8fa080983ab936471e9eda83b26f190181bdeb4R104)
There was a problem hiding this comment.
Thanks! I see what you mean now
There was a problem hiding this comment.
Would the attribute change to id, or it stays what it currently is and becomes messaging.consumer.consumer_id?
There was a problem hiding this comment.
I think I've figured it out, so will update and get feedback.
Thanks for all the pointers
There was a problem hiding this comment.
The messaging.consumer semantic convention still only has "messaging." as prefix, so the final string to use as key in the semantic conventions will still be just "messaging.consumer_id".
|
@kenfinnigan Please also add a Changelog entry for the new attribute. |
|
@Oberon00 I will update the changelog, but I'm not sure what to do regarding the suggested change to |
There is only one approval yet, it's no big deal if you still change it (if you agree with @iNikem's and my comments). Just re-request the review from @carlosalberto afterwards. |
kenfinnigan
force-pushed
the
trace-messaging-consumer-id
branch
from
0622868 to
6cbb75b
Compare
|
I think this is good, but would appreciate a re-review to confirm |
| type: string | ||
| brief: > | ||
| The identifier for the consumer receiving a message. For Kafka, set it to | ||
| `{messaging.kafka.consumer_group} - {messaging.kafka.client_id}`, if both are present, or only |
There was a problem hiding this comment.
The only thing I'm not sure about in this PR is this format. Isn't there some standard "fully qualified client ID" format for Kafka?
There was a problem hiding this comment.
I don't believe so, as https://jaceklaskowski.gitbooks.io/apache-kafka/content/kafka-properties-client-id.html describes how it's an optional identifier passed to the broker, so it could be anything
There was a problem hiding this comment.
I meant the combined format, where we now put " - " in-between. I wondered if there is some existing notation to combine consumer group and client_id into a single identifier string.
There was a problem hiding this comment.
Not that I've found unfortunately
There was a problem hiding this comment.
Are client_ids unique across consumer groups? Maybe it would be better then to just use the client_id here.
There was a problem hiding this comment.
They must be unique within a Kafka Broker, but they're not required. It's possible for a consumer to only have the consumer group id set and not the client id.
|
Any other comments on this, or is it good? |
|
Any feedback/comments? I'd like to have this merged soon if possible so I can implement it in our frameworks |
kenfinnigan
force-pushed
the
trace-messaging-consumer-id
branch
from
6cbb75b to
60ee36f
Compare
|
Not sure why these checks failed, it almost appears they didn't even run. Is there a problem with CI checks at present? |
|
I re-ran the CI. |
|
Thanks @Oberon00, seems ok now |
|
Is there additional feedback on this? |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Any additional concerns with this, or can it be merged? |
|
@Oberon00 Is this ok to be merged or is there more needed? |
|
I'd like to see this completed soon, what are the next steps for this to happen? |
|
I can't merge it. There needs to be one more approval and then a maintainer (= TC member) needs to merge it. |
|
@Oberon00 Ok. What can I do to help that process along? |
|
Thanks @carlosalberto and @Oberon00! |
Resolves #1796
Changes
Adds
consumer_idas a label for messaging spans to, as best as possible, uniquely identify the consumer of a message.This is especially beneficial when the same message is "replayed" to different consumers, ensuring the spans are differentiated between the usages.