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

[Traces Documentation] Clarification on events vs span attributes #4184

Closed
benh4ckett opened this issue Mar 20, 2024 · 4 comments · Fixed by #4185
Closed

[Traces Documentation] Clarification on events vs span attributes #4184

benh4ckett opened this issue Mar 20, 2024 · 4 comments · Fixed by #4185

Comments

@benh4ckett
Copy link

I'll start with the motivating example. Consider a span that is responsible for publishing a bunch of messages. Should the number of failed publishes and successful publishes be attributes on the span, or should there be an event (named something like FinishedAllPublishAttempts) with those attributes attached?

More generally, can the documentation at https://opentelemetry.io/docs/concepts/signals/traces/#attributes clarify if it's best practice for span attributes to only include information known at the start of the span? Or is it fine if the span contains attributes with information known only after the span is almost complete?

@cartermp
Copy link
Contributor

Using your example, I'll say that:

  • If the success or failure of message publishing has a timestamp that's meaningful -- i.e., you need to know the specific point in time that it attempted, succeeded, failed, etc. -- then a Span Event is appropriate to capture data about that operation
  • If these sub-operations don't have any meaning in their timestamps, then they should just be attributes on the span

And yeah, attributes can absolutely contain data you don't know during span creation. The downside to that is you can't use those attribute vales when making SDK sampling decisions but that's pretty much it.

@cartermp
Copy link
Contributor

Okay, I created #4185

@benh4ckett
Copy link
Author

@cartermp Awesome, thanks so much for the help!! Super appreciated. That all makes sense to me. And for what it's worth, the pull request looks good to me (but I assume I can't approve :P). (Though, do let me know if there's something I can do to help get the pull request approved). Thanks again!

@cartermp
Copy link
Contributor

Great, glad to hear it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants