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 all fields to span name #1

Closed
wants to merge 3 commits into from

Conversation

superdump
Copy link

I'm not sure if this is desirable in its current form, but this is super useful for me. I'm open to input for how to improve it.

@thoren-d
Copy link
Owner

thoren-d commented Nov 5, 2020

Thanks for the contribution!

I was thinking I wanted to add fields at some point and I like that using FormattedFields makes this code simpler.

Is it important that the field values be in the name of each span for your application? I was thinking we could put them in the "args" object for each span enter and exit event. This is where I put the file and line number for each span.

Also, an option to enable/disable including fields would be good, as they can make the resulting file much larger.

@superdump
Copy link
Author

I was motivated to make this PR because of a problem with naming Spans in the bevy game engine. It schedules a bunch of functions to run at different times based on structs that are derived for functions. These structs contain the name of the function, obtained by std::any::type_name<T>() -> &'static str where T is the type of the function, and it's stored in a Cow<'static, str>. As tracing Span names must be constants (it seems), I have had to add the name of the function being executed as a field on the span. That then leads to this PR because a lot of the interesting Spans added to bevy's code have a name field and visualising the traces is only really useful by adding the field into the name, otherwise all the spans are called 'system' or 'stage'. So, I'm trying to see if there's a compile-time way to automatically instrument all systems.

@thoren-d
Copy link
Owner

thoren-d commented Dec 1, 2020

@superdump I've just published 0.3.0 with a new name_fn option which should enable your use case. Let me know if it works for you.

@superdump
Copy link
Author

Works like a charm! Thanks!

@superdump superdump closed this Dec 2, 2020
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.

2 participants