-
Notifications
You must be signed in to change notification settings - Fork 224
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
feat(SwingSet): Echo kernel console output to the slog #10925
base: master
Are you sure you want to change the base?
Conversation
Deploying agoric-sdk with
|
Latest commit: |
8cfe644
|
Status: | ✅ Deploy successful! |
Preview URL: | https://b46807e8.agoric-sdk.pages.dev |
Branch Preview URL: | https://gh-10776-kernel-console-slog.agoric-sdk.pages.dev |
1565d62
to
8cfe644
Compare
}); | ||
doneRegistering(`Unrecognized makeSlogger slogCallbacks names:`); | ||
// @ts-expect-error xxx | ||
done('Unused makeSlogger slogCallbacks method names'); |
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 considering reducing this even further:
const unusedWrapperMsg = 'Unused makeSlogger slogCallbacks method names';
const wrappedMethods = addSlogCallbacks(slogCallbacks, unusedWrapperMsg, {
provideVatSlogger,
vatConsole: (vatID, ...args) =>
provideVatSlogger(vatID).vatSlog.vatConsole(...args),
),
startup: (vatID, ...args) =>
provideVatSlogger(vatID).vatSlog.startup(...args),
),
…
});
return harden({ ...wrappedMethods, write: safeWrite });
const noopFinisher = harden(() => {}); | ||
|
||
/** @typedef {(...finishArgs: unknown[]) => unknown} AnyFinisher */ | ||
/** @typedef {Partial<Record<Exclude<keyof KernelSlog, 'write'>, (methodName: string, args: unknown[], finisher: AnyFinisher) => unknown>>} SlogWrappers */ |
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.
Would it be worth indicating that each SlogWrapper method can return a finisher, even though that information would be subsumed in many places?
/** @typedef {Partial<Record<Exclude<keyof KernelSlog, 'write'>, (methodName: string, args: unknown[], finisher: AnyFinisher) => unknown>>} SlogWrappers */ | |
/** @typedef {Partial<Record<Exclude<keyof KernelSlog, 'write'>, (methodName: string, args: unknown[], finisher: AnyFinisher) => (AnyFinisher | unknown)>>} SlogWrappers */ |
provideVatSlogger(vatID).vatSlog.startup(...args), | ||
), | ||
// TODO: Remove this seemingly dead code. | ||
replayVatTranscript, |
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't find any use of this method: https://github.com/search?q=repo%3AAgoric%2Fagoric-sdk%20replayVatTranscript&type=code
sourcedConsole[level](source, ...args); | ||
// TODO: Just use "liveslots" rather than "ls"? | ||
if (source === 'ls') source = 'liveslots'; |
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.
It's weird that we map source "ls" to "liveslots" in the slog but not in console output. For that matter, it's weird that we use an abbreviation just to reverse it anyway.
Fixes #10776
Description
After a lot of refactoring, makes the relatively simple change of endowing the compartment in which the kernel runs with a console whose methods are augmented to write slog entries. Best reviewed by commit.
Security Considerations
Security posture should not be affected.
Scaling Considerations
The slog will gain new entries, but kernel logging tends to be reasonably small AFAIK.
Documentation Considerations
Anything consuming slogfiles (such as for telemetry reporting) should be prepared to encounter entries with type "console" and source "kernel".
Testing Considerations
Manual confirmation per #10776:
slog excerpts
Upgrade Considerations
This tweaks some typing in packages/swingset-liveslots/src/liveslots.js and adds some utilities to package "internal", but everything else is kernel code that is not subject to and does not affect consensus.
Release verification should look for slog entries like the above (type "console" and source "kernel").