-
Notifications
You must be signed in to change notification settings - Fork 881
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
Add yaml semantic conventions for Messaging #926
Add yaml semantic conventions for Messaging #926
Conversation
@open-telemetry/specs-approvers please have a look! |
|---|---|---|---|---| | ||
| `messaging.system` | string | A string identifying the messaging system. | `kafka`<br>`rabbitmq`<br>`activemq` | Yes | | ||
| `messaging.destination` | string | The message destination name. This might be equal to the span name but is required nevertheless. | `MyQueue`<br>`MyTopic` | Yes | | ||
| `messaging.destination_kind` | string enum | The kind of message destination | `queue` | Conditional [1] | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not quite clear what does string enum
mean. Perhaps just say string
since it is already clarifies in the footnote that it must be one of the predefined values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this requires changes in the tool, I've opened open-telemetry/build-tools#9
<!-- semconv messaging.consumer --> | ||
| Attribute | Type | Description | Example | Required | | ||
|---|---|---|---|---| | ||
| `messaging.operation` | string enum | A string identifying the kind of message consumption as defined in the [Operation names](#operation-names) section above. If the operation is "send", this attribute MUST NOT be set, since the operation can be inferred from the span kind in that case. | `receive` | No | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, I think simply string
is good enough as the type since possible values are clarified below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I concede that it is redundant but I think it's OK to call this out in the type. Maybe string (fixed)
would be more clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #926 (comment)
Note for triagers: this does not change the meaning of the spec, it should be considered an editorial change, so should be OK to merged after freeze. |
Merging this editorial change which introduces YAML representations for the existing semantic conventions. |
Changes
Updated Messaging semantic convention to the YAML format.
Related issues #547