-
Notifications
You must be signed in to change notification settings - Fork 18
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
melpomene: tracing improvements #2
Merged
Merged
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
Currently, the `tracing` behavior in Melpomene is controlled by a pair of feature flags, "trace-modality" and "trace-fmt". These select between emitting traces to stdout using `tracing_subscriber::fmt`, and emitting to Modality using the `tracing-modality` crate. However, these feature flags are not additive. If they are both enabled, the code will not compile, since there are two implementations of the `setup_tracing` function, which are conditionally compiled based on whether those feature flags are enabled. If both are enabled, both functions will be present, resulting in a compiler error. This commit changes this code so that there is a single implementation of the `setup_tracing` function, which will construct a `tracing` subscriber that includes either `fmt`, Modality, or _both_, depending on the feature combination. Now, Melpomene should build even with `--all-features` or similar. Now that the `fmt` layer is built manually rather than using `fmt::init()` "easy mode" helper, I've also changed the filter environment variable from "RUST_LOG" to "MELPOMENE_TRACE". I thought this was nicer, but we can continue using the default env var if that's preferable.
This branch makes the tracing configuration slightly nicer (IMO). Depends on #1.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Totally works for me! Thanks for the cleanup :) |
jamesmunns
added a commit
that referenced
this pull request
Jun 4, 2023
containers: fix missing and wrong `Send` + `Sync` impls
jamesmunns
added a commit
that referenced
this pull request
Jun 4, 2023
These are the "non-`disk`" changes made for #2, without the actual "add the disk" support. I'm opening this as a PR to hopefully allow @hawkw to merge it into her branches while I'm still working on disk branch. Uncontroversial changes: * Makes EntryKind non-exhaustive - could be used for robustness checking * Adds a single method to allocation a dictionary entry, used to remove some copy/paste code * adds `b@` and `b!` operators for byte-aligned (instead of cell-aligned) load/stores Potentially controversial changes: * Adds a `ptr_data: isize` union variant to the cell type, to be used when mixing "i32" data from the stack with "ptr" data from the stack * ONLY `add` and `sub` actually use the `ptr_data` variant At some point, we might want to pass and declare which operators ARE "pointer safe", and which ones aren't, or add specific ptr-safe operators. That being said, I'm not sure whether any operations OTHER than add or sub on a pointer are likely to be meaningful. * Unrelated changes made during `disk` changes * Update src/leakbox.rs * Rename for consistency Co-authored-by: Eliza Weisman <eliza@buoyant.io>
jamesmunns
added
platform: melpomene
Specific to the Melpomene simulator platform
area: tools & build
Related to host developer tools, including tracing, Crowtty and build processes
labels
Jul 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area: tools & build
Related to host developer tools, including tracing, Crowtty and build processes
platform: melpomene
Specific to the Melpomene simulator platform
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.
(This includes changes from #1 because that PR was opened from a fork
and I can't nicely stack PRs when one branch is on a fork)
This branch makes several small improvements to
tracing
in Melpomene:melpomene: make
tracing
feature flags additiveCurrently, the
tracing
behavior in Melpomene is controlled by apair of feature flags, "trace-modality" and "trace-fmt". These select
between emitting traces to stdout using
tracing_subscriber::fmt
,and emitting to Modality using the
tracing-modality
crate. However,these feature flags are not additive. If they are both enabled, the
code will not compile, since there are two implementations of the
setup_tracing
function, which are conditionally compiled based onwhether those feature flags are enabled. If both are enabled, both
functions will be present, resulting in a compiler error.
This commit changes this code so that there is a single
implementation of the
setup_tracing
function, which will constructa
tracing
subscriber that includes eitherfmt
, Modality, orboth, depending on the feature combination. Now, Melpomene should
build even with
--all-features
or similar.Now that the
fmt
layer is built manually rather than usingfmt::init()
"easy mode" helper, I've also changed the filterenvironment variable from "RUST_LOG" to "MELPOMENE_TRACE". I thought
this was nicer, but we can continue using the default env var if
that's preferable.
melpomene: minor improvements to tracing configuration
melpomene: add some spans to the kernel/userspace threads
kernel: change the "initialized heap" event to format the addr in hex
Closes #1