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

Remove decode_message span #2078

Closed
balegas opened this issue Dec 2, 2024 · 3 comments · Fixed by #2082
Closed

Remove decode_message span #2078

balegas opened this issue Dec 2, 2024 · 3 comments · Fixed by #2082

Comments

@balegas
Copy link
Contributor

balegas commented Dec 2, 2024

Remove decode_message span and store decode execution time into parent span. so we later can find outliers.

We currently create a decode_message span for every transaction we handle, which is generating loads of spans

@balegas balegas added the tracing label Dec 2, 2024
@balegas balegas added this to the Production Readiness milestone Dec 2, 2024
@kevin-dp
Copy link
Contributor

kevin-dp commented Dec 2, 2024

I'm wondering whether we want to add this as span attributes or events.
Here's an explanation on when to use attributes vs events: https://opentelemetry.io/docs/concepts/signals/traces/#when-to-use-span-events-versus-span-attributes

It would make sense to me to add an event at the start of decoding the message and an event at the end of the decoding and from those timestamps we could compute the duration. Or we could just add it as an attribute as suggested in this issue. Not sure what's best.

@balegas
Copy link
Contributor Author

balegas commented Dec 2, 2024

Sounds good to me, but cc.ing @KyleAMathews who has more experience with OTEL.

@KyleAMathews
Copy link
Contributor

Events are useful if we want to record info specifically about the event e.g. normal log stuff like errors w/ stack traces or "this odd thing happened that we want to make a note of" etc.

add an event at the start of decoding the message and an event at the end of the decoding and from those timestamps we could compute the duration

This definitely should be an attribute as it's not trivial (or perhaps possible in this case — I'd have to investigate) to calculate stuff across events like this. It's much easier/neater to just have the code do the computation and put it in as an attribute.

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