-
Notifications
You must be signed in to change notification settings - Fork 176
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
Format span and trace id as UUIDs in Griptape Cloud #941
Conversation
"trace_id": str(UUID(int=span.context.trace_id)), | ||
"span_id": str(UUID(int=span.context.span_id)), | ||
"parent_id": str(UUID(int=span.parent.span_id)) if span.parent else None, |
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.
This is a bit inconsistent with how we generate uuids in other places. And what does the UUID(int=...)
syntax do? Is it generating a uuid v5 using the span/trace id as the seed?
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.
It's not generating a uuid, its just using str(UUID(int=...))
to format the integer id generated by open telemetry. The range of the integer values supported by UUID are the compatible with the range generated by open telemetry.
We are using this value instead of generating our own to make it easy to get the correct value of parent spans and on events. If we generated our own, then we'd need to keep track of a mapping between open telemetry id and our generated id, but since they are 1:1, there's no reason not to just use the one generated by open telemetry.
span = get_current_span() | ||
if span is INVALID_SPAN: | ||
return None | ||
return str(UUID(int=span.get_span_context().span_id)) |
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 uuid comment here.
Changes:
NOTE: Merging into
release/cloud-unreleased
rather than dev!