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

Add span IDs #2495

Closed
wants to merge 1 commit into from
Closed

Add span IDs #2495

wants to merge 1 commit into from

Conversation

goodspark
Copy link

Motivation

I think this will address #1481

Having a general way to add span ID information to events being output as logs will enable trace-to-log linking in various tracing services.

@davidbarsky
Copy link
Member

I'm sorry, we're not going to accept this PR. tracing's span IDs are generally meant to be opaque identifiers that are unique within a process for currently open spans. They are not meant to be globally unique identifiers like UUIDs or an OpenTelemetry span ID. If you'd like to read more, take a look at this documentation.

While I think we should make it easier to integrate different layers, I think whatever approach is done approach requires care. tracing needs to be generic over different external integrations and that's pretty hard. I think it's fair to say that we don't have the time to design such an approach and recording a given distributed tracing ID field as an field in a span is a good enough approach for now.

@davidbarsky davidbarsky closed this Mar 3, 2023
@hawkw
Copy link
Member

hawkw commented Mar 3, 2023

Hmm, I think it would be fine to have the option to include tracing span IDs in the JSON format --- although they are not globally unique, they can be used to e.g. visually distinguish two distinct spans with the same name and fields. I think it would be okay to have a formatter configuration to include them, as long as the constraints of tracing's IDs are clearly documented in the API for enabling that configuration...

@goodspark
Copy link
Author

Does this mean #1481 should be closed as a wont-do?

@davidbarsky
Copy link
Member

Does this mean #1481 should be closed as a wont-do?

No. We should probably keep it open and document the workaround.

@goodspark
Copy link
Author

What's the workaround? It wasn't clear to me. Unless you mean manually adding it to every place we emit an event. Bc that seems too painful a user experience. If this were behind config options with caveat docs as hawkw said, would it be acceptable?

@davidbarsky
Copy link
Member

Sorry, it's:

I think it's fair to say that we don't have the time to design such an approach and recording a given distributed tracing ID field as an field in a span is a good enough approach for now.

Most layers/formatters will include information from spans when printing events.

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 this pull request may close these issues.

None yet

3 participants