-
Notifications
You must be signed in to change notification settings - Fork 41
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 oxlog tool and library #4810
Conversation
This still needs a bit of work before it is ready to go.
} | ||
} | ||
|
||
fn load_svc_logs(dir: Utf8PathBuf, logs: &mut BTreeMap<ServiceName, SvcLogs>) { |
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.
There is some similar logic here for finding logs during zone-bundling, it might be a good idea to share it somehow. Calling into this library in the running_zone
mod seems reasonable.
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.
I can certainly add a helper function to do this. However, right now oxlog only looks at logs with the oxide-
prefix. There are a lot more with the system-
prefix. Should I make that a separate type of retrieval?
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.
I'm not sure. The system-illumos-
prefix was included because there are some of our services that still use the original naming of the SMF services, like svc:/system/illumos/propolis-server
. (I think some of the networking daemons are still named this way too.)
Searching for that prefix becomes more reasonable when we're only looking for logs that come from a specific, named service, which is the case of the zone bundling process. That might be too permissive for oxlog
, I'm really not sure. If that's true, then maybe we can commonize the portion of the checks that make sense (say, for SMF services named like svc:/oxide/blah
). It seems like you might also be trying to capture all log files for any service, not just the "primary" Oxide services in zone. In that case, the commonization might be less helpful.
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.
Searching for that prefix becomes more reasonable when we're only looking for logs that come from a specific, named service, which is the case of the zone bundling process. That might be too permissive for oxlog, I'm really not sure.
I think this is likely to be a common use case, but we also capture all logs in a zone. This means that we'll suck up all 'system-' prefixed logs unless we hardcode a list of services that can be named like this. I've so far avoiding hardcoding the names of any services and deducing service names by the names of the log-files. It turns out that strategy falls apart because of this dichotomy in naming. It seems to me that we would be better off changing the prefix for things like propolis to be oxide-
, but in the meantime I can suck up only files that match a hardcoded set of services so we don't miss them. I specifically do not want to include all system service files as there are so many and mostly we won't want to dig through them in the common case. Again, though, we likely will want to be able to retrieve those files in some debugging scenarios though. Getting them through sled-agent integration via oxlog is preferable than having to ssh in. I will add some support for this as well.
It seems like you might also be trying to capture all log files for any service, not just the "primary" Oxide services in zone. In that case, the commonization might be less helpful.
I'm trying to gather any oxide specific service log in a zone, ideally without knowing the name of the service ahead of time. That may be too ambitious.
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.
I'm trying to gather any oxide specific service log in a zone, ideally without knowing the name of the service ahead of time. That may be too ambitious.
So the way we did that in zone-bundling is to list all SMF FMRIs matching the patterns svc:/oxide/*
or svc:/system/illumos/*
, and then listing the logfiles associated with each of those (with svcs -L
).
Based on your answer, it does really sound like you want to use the same logic as in the zone bundling, or at least, use that logic possibly in addition to some other. Note that the prefix used in the zone-bundling is system-illumos-*
not system-*
. The latter is definitely very permissive, and will get you a bunch of stuff unrelated to Oxide services. If the goal is Oxide-specific log files, then I think the logic can be shared with zone-bundling :)
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.
Note that the prefix used in the zone-bundling is system-illumos-* not system-*. The latter is definitely very permissive, and will get you a bunch of stuff unrelated to Oxide services. If the goal is Oxide-specific log files, then I think the logic can be shared with zone-bundling :)
Aha! Alright, so far the only dependency for oxlog is using the filesystem. But I can definitely parse using system-illumos-*
and include those files. Then we can share the logic.
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.
I added this support in c38d7e4
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.
illumos-utils
now uses the method which I copied into oxlog from oxlog as of caf3cd5
dev-tools/oxlog/src/bin/oxlog.rs
Outdated
|
||
/// Print only the archive log files | ||
#[arg(short, long)] | ||
archived: bool, |
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.
I think there is a third class, rotated log files that have not yet been archived. These probably don't live very long, since the archiver runs frequently. But we may want to consider including them and a CLI switch to filter them.
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.
These are actually included with the archived files right now. I"m happy to separate them out though if you think that makes sense.
dev-tools/oxlog/src/lib.rs
Outdated
|
||
impl Pools { | ||
pub fn read() -> anyhow::Result<Pools> { | ||
let internal = read_dir("/pool/int/") |
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.
We don't actually read anything from the internal_pools, but I don't think it hurts to keep this type around for now. We may end up reading if we expand this tool to grab other files.
This was one of the first bits of code I wrote for this PR and I left it through all the iterations. Happy to change/remove if desired though.
pub use super::oxide_smf_service_name_from_log_file_name; | ||
|
||
#[test] | ||
fn test_is_oxide_smf_log_file() { |
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.
I copied this from the illumos_utils
code along with the related method. I added an extra test at the end to ensure we don't also collect system-*
along with system-illumos-*
files.
I also need to clean up the packaging into the global zone as @smklein pointed out: #4840 (comment) |
Implemented in 09ad1b5 |
No description provided.