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

Messaging: generalize common attributes to future-proof metrics #797

Closed
lmolkova opened this issue Mar 7, 2024 · 3 comments · Fixed by #815 or #798
Closed

Messaging: generalize common attributes to future-proof metrics #797

lmolkova opened this issue Mar 7, 2024 · 3 comments · Fixed by #815 or #798

Comments

@lmolkova
Copy link
Contributor

lmolkova commented Mar 7, 2024

See #760 (comment)

One of the options to minimize future breaking for generic metrics when specific messaging extension decides to add an attribute, is to generalize a few common attributes.

While in general this approach is problematic, there are several common patterns we should consider:

@pyohannes
Copy link
Contributor

Partitions and consumer groups are concepts usually associated with systems that do checkpoint-based settlement (instead of individual message settlement).

Currently, all attributes we've defined for generic metrics apply to all messaging systems independently of the settlement style.

While I'm all in favor of generalizing attribute naming (e. g. to messaging.destination.partition.id), let's further discuss adding such attributes to generic metrics. I'm not yet sure if we should attributes that are of no use at all to certain systems.

@lmolkova
Copy link
Contributor Author

lmolkova commented Mar 7, 2024

I'm not yet sure if we should attributes that are of no use at all to certain systems.

I wonder if we can entertain an idea of messaging.destination.path that would fully identify destination from the consumer side and include subscription name, consumer group, partition or anything else.

Essentially the somewhat catch-all attribute of the highest cardinality, You can still add messaging.pulsar.subscription.name to the same metric if subscription name already appears in the generic messaging.destination.path.

This also pushes a problem to pick a good messaging.destination.path onto the instrumentation (or semconv extension) author and if they didn't put fully qualified thing there, they might need to do breaking changes in the library. Spec in the case remains stable. Sounds not great, but minimizes the blast radius of breaking changes.

@pyohannes
Copy link
Contributor

I wonder if we can entertain an idea of messaging.destination.path that would fully identify destination from the consumer side and include subscription name, consumer group, partition or anything else.

There are two things that should be separated:

  1. A consistent definition of a destination that can apply to both producers and consumers. For checkpoint-based systems, this would involve a destination name (a topic name) and a server address.
  2. A complete definition of a destination for consumers. This involves the destination name (topic name), the server address, the partition id, and a consumer group.

We should be able to separately identify both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment