Update semantic conventions in opentelemetry_oban #528
Conversation
|
I'd love to see this PR approved. 🙏 |
| [ | ||
| event( | ||
| name: "exception", | ||
| name: :exception, |
| [ | ||
| event( | ||
| name: "exception", | ||
| name: :exception, |
| [ | ||
| event( | ||
| name: "exception", | ||
| name: :exception, |
| :"oban.job.scheduled_at" => DateTime.to_iso8601(scheduled_at) | ||
| } | ||
|
|
||
| span_name = "#{worker} process" |
There was a problem hiding this comment.
Seems like we also need to update the span name per https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/#span-name?
| MessagingAttributes.messaging_client_id() => worker, | ||
| MessagingAttributes.messaging_destination_name() => queue, | ||
| MessagingAttributes.messaging_operation_type() => | ||
| MessagingAttributes.messaging_operation_type_values().process, |
There was a problem hiding this comment.
Does the warning message displayed in the top of https://opentelemetry.io/docs/specs/semconv/messaging/ means that the migration needs to provide a OTEL_SEMCONV_STABILITY_OPT_IN env var that allows for previous or newer "messaging" conventions?
| Trace.messaging_destination_kind() => :queue, | ||
| MessagingAttributes.messaging_system() => :oban, | ||
| MessagingAttributes.messaging_client_id() => worker, | ||
| MessagingAttributes.messaging_destination_name() => queue, |
There was a problem hiding this comment.
I see here that you're keeping the existing interpretation that a "Destination" is an Oban queue.
For what is worth and for the record, noting that this other pull request (#436 (comment)) is making a different interpretation in that a "Destination" is the Oban Worker and the "Consumer group" is the Oban queue.
3 participants
What does this PR do?
Updates the opentelemetry_semantic_conventions dependency of opentelemetry_oban to 1.27.
I wasn't sure exactly what the replacement is for the "messaging_destination_kind" key, so I deleted it. Please correct me if this was the wrong decision.messaging destination kind was indeed deleted, as they didn't find a use case for it.