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

Consider otel span to be a transaction of type=message when some message attribute is set #194

Closed
simitt opened this issue Dec 20, 2023 · 3 comments · Fixed by #204
Closed
Assignees
Labels
bug Something isn't working

Comments

@simitt
Copy link
Contributor

simitt commented Dec 20, 2023

TranslateTransaction only considers events that have message_bus.destination or messaging.destination set to be a transaction of type messaging. However, messaging.system is a required field according to https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/ and should also be considered for deciding the transactions type.

See https://github.com/elastic/apm-data/blob/main/input/otlp/traces.go#L411-L414

@simitt simitt added the bug Something isn't working label Dec 20, 2023
@carsonip
Copy link
Member

  • In TranslateSpan, we are currently storing messageSystem in event.Span.Subtype and messageOperation in event.Span.Action.
  • In TranslateTransaction, messaging.system and messaging.operation are currently set as labels under default in the switch statement.

Question:
We don't have subtype and action in Transaction. Should we still keep the info in a label, after using it to determine transaction type? In that case the change will look like

diff --git a/input/otlp/traces.go b/input/otlp/traces.go
index 94072c7..8b8da65 100644
--- a/input/otlp/traces.go
+++ b/input/otlp/traces.go
@@ -413,6 +413,12 @@ func TranslateTransaction(
                        case "message_bus.destination", semconv.AttributeMessagingDestination:
                                message.QueueName = stringval
                                isMessaging = true
+                       case semconv.AttributeMessagingSystem:
+                               isMessaging = true
+                               modelpb.Labels(event.Labels).Set(k, stringval)
+                       case semconv.AttributeMessagingOperation:
+                               isMessaging = true
+                               modelpb.Labels(event.Labels).Set(k, stringval)
 
                        // rpc.*
                        //

@simitt
Copy link
Contributor Author

simitt commented Jan 26, 2024

Yes let's keep the information set as labels. Eventually the information should be mapped to the messaging.* field, but that would be a larger undertaking as the current apm model for message differs quite a bit from otel semantic conventions for messaging, and the apm model doesn't have attributes for system and operations.

@carsonip
Copy link
Member

Closes via #204

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants