stop rendering empty fields, refactor field conversion #83
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This branch updates the console to truncate fields named `spawn.location` when formatting fields on task spans, if the field's value is a path in the Cargo registry. Paths in the Cargo registry tend to look like this: ``` /home/eliza/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.9.0/src/runtime/basic_scheduler.rs:157:24 ``` We perform the truncation using a regular expression matching strings beginning with a `/` and accepting up to the string `/.cargo/registry/src`, followed by one additional path segment for the `github.com-<SHA>` part. The first match found in the string is replaced with the string `<cargo>`, so that it's clear the path is in the Cargo registry. This could, alternatively, have been implemented by parsing the paths to a `std::path::Path` and actually inspecting path segments. This might be more *semantically* correct than using a regex. However, the issue here is that the paths in the `spawn.location` field are generated in the environment where the instrumented program is _compiled_, rather than in the environment where the console application is running. This might, for example, be on an OS with a different path separator, and `std::path::Path` defaults to using the system's path separator. We could work around this, theoretically, but the regex seemed much simpler. Closes #74
Now that Tokio supports optional user-provided task names (in tokio-rs/tokio#3881), the task spans it generates may contain empty fields (if the user does not provide a name for a spawned task). However, the console always displays the `task.name` field, even when they are empty. This wastes space, and looks kind of ugly. This branch updates the console to skip displaying fields whose value is the empty string. ### Before:  ### After:  Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This factors out the protobuf `Field` -> internal `Field` conversion into a method, and cleans it up a bit. No functional change, except that we now take the proto `Field` type by value, so we don't have to clone strings out of it like we did previously (due to taking it by reference). I also added the metadata ID to the `Metadata` type, so that we can continue asserting that the metadata's ID matches the expected one for the field, without having to pass that in separately. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
seanmonstar
approved these changes
Aug 4, 2021
hawkw
added a commit
that referenced
this pull request
Aug 4, 2021
Depends on #81 (since it touches the same code and would cause merge conflicts). * feat(console): don't display fields with "" values Now that Tokio supports optional user-provided task names (in tokio-rs/tokio#3881), the task spans it generates may contain empty fields (if the user does not provide a name for a spawned task). However, the console always displays the `task.name` field, even when they are empty. This wastes space, and looks kind of ugly. This branch updates the console to skip displaying fields whose value is the empty string. ### Before:  ### After:  Signed-off-by: Eliza Weisman <eliza@buoyant.io> * refac(console): factor out field from pb conversion This factors out the protobuf `Field` -> internal `Field` conversion into a method, and cleans it up a bit. No functional change, except that we now take the proto `Field` type by value, so we don't have to clone strings out of it like we did previously (due to taking it by reference). I also added the metadata ID to the `Metadata` type, so that we can continue asserting that the metadata's ID matches the expected one for the field, without having to pass that in separately. Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #81 (since it touches the same code and would cause merge conflicts).
Now that Tokio supports optional user-provided task names
(in tokio-rs/tokio#3881), the task spans it generates may contain empty
fields (if the user does not provide a name for a spawned task).
However, the console always displays the
task.namefield, even whenthey are empty. This wastes space, and looks kind of ugly.
This branch updates the console to skip displaying fields whose value is
the empty string.
Before:
After:
Signed-off-by: Eliza Weisman eliza@buoyant.io
This factors out the protobuf
Field-> internalFieldconversioninto a method, and cleans it up a bit. No functional change, except that
we now take the proto
Fieldtype by value, so we don't have to clonestrings out of it like we did previously (due to taking it by
reference). I also added the metadata ID to the
Metadatatype, so thatwe can continue asserting that the metadata's ID matches the expected
one for the field, without having to pass that in separately.
Signed-off-by: Eliza Weisman eliza@buoyant.io