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

Replacement for messaging.kafka.source.partition? #300

Closed
trask opened this issue Sep 5, 2023 · 8 comments
Closed

Replacement for messaging.kafka.source.partition? #300

trask opened this issue Sep 5, 2023 · 8 comments
Assignees

Comments

@trask
Copy link
Member

trask commented Sep 5, 2023

Messaging "source" attributes were removed in #100.

#156 introduced replacements for all of the attributes removed in #100, except for messaging.kafka.source.partition.

Ran into this while migrating Java Kafka instrumentation to semconv 1.21.0, and wondering if we should bring this attribute back also, e.g. as messaging.kafka.destination_publish.partition?

@lmolkova
Copy link
Contributor

lmolkova commented Sep 7, 2023

It should be messaging.kafka.destination.partition - both consumers and producers should report partition on destination namespace (which used to be split to source and destination depending on where it's reported).

destination_publish.partition might be used if consumer knowns not just the original destination message was published to, but also a partition. I guess it's quite a rare case.

@trask
Copy link
Member Author

trask commented Sep 7, 2023

thx @lmolkova, this makes sense!

@trask trask closed this as completed Sep 7, 2023
@joaopgrassi
Copy link
Member

joaopgrassi commented Sep 11, 2023

Should we leave it open, so we can track and re-add the attribute for Kafka? I see @pyohannes added to our v1 messaging project.

@trask
Copy link
Member Author

trask commented Sep 11, 2023

Should we leave it open, so we can track and re-add the attribute for Kafka? I see @pyohannes added to our v1 messaging project.

we don't have any Java instrumentation that is able to report both destination and destination_publish. if there had been a destination_publish.partition we might have used it not realizing we shouldn't. maybe better to wait until a use case emerges for it?

@joaopgrassi
Copy link
Member

But was it recording the old attribute before? Because if so, then we should I guess re-introduce the attribute under messaging.kafka.destination.partition. The other case I feel we don't know so fine leaving it out. (destination_publish.partition)

@trask
Copy link
Member Author

trask commented Sep 12, 2023

messaging.kafka.destination.partition already exists: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/messaging/kafka.md#span-attributes

@joaopgrassi
Copy link
Member

joaopgrassi commented Sep 13, 2023

Ah, great then. We could still re-open it and move it out of our V1 Stable semantics board, so we at least have a place holder to discuss it in case the need arrives. What do you think @pyohannes ?

@pyohannes
Copy link
Contributor

I'd suggest to leave it as is for now.

Once a use case arises for it, we can open an issue and discuss. I don't think it's urgent for now, as it can be added later as a non-breaking addition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: V1 - Stable Semantics
Development

No branches or pull requests

5 participants