-
Notifications
You must be signed in to change notification settings - Fork 663
Add reducer name to database log records #3274
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
Conversation
|
Output from $ spacetime logs -s local quickstart-chat
Finished `release` profile [optimized] target(s) in 0.15s
Finished `release` profile [optimized] target(s) in 0.14s
Running `/home/phoebe/clockworklabs/SpacetimeDB/target/x86_64-unknown-linux-gnu/release/spacetimedb-cli logs -s local quickstart-chat`
2025-09-23T14:40:18.218267Z INFO: __spacetimedb__ __spacetimedb__: Creating table `message`
2025-09-23T14:40:18.218327Z INFO: __spacetimedb__ __spacetimedb__: Creating table `user`
2025-09-23T14:40:18.218479Z INFO: __spacetimedb__ __spacetimedb__: Invoking `init` reducer
2025-09-23T14:40:18.218567Z INFO: __spacetimedb__ __spacetimedb__: Database initialized
2025-09-23T14:40:31.077171Z INFO: set_name modules/quickstart-chat/src/lib.rs:30: User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2 sets name to phoebe
2025-09-23T14:40:44.593588Z INFO: send_message modules/quickstart-chat/src/lib.rs:55: User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2: Hey!
2025-09-23T14:55:33.213621Z INFO: send_message modules/quickstart-chat/src/lib.rs:55: User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2: Hey!Note that The raw JSON records are: {"level":"Info","ts":1758638418218267,"target":"__spacetimedb__","filename":"__spacetimedb__","function":"__spacetimedb__","message":"Creating table `message`"}
{"level":"Info","ts":1758638418218327,"target":"__spacetimedb__","filename":"__spacetimedb__","function":"__spacetimedb__","message":"Creating table `user`"}
{"level":"Info","ts":1758638418218479,"target":"__spacetimedb__","filename":"__spacetimedb__","function":"__spacetimedb__","message":"Invoking `init` reducer"}
{"level":"Info","ts":1758638418218567,"target":"__spacetimedb__","filename":"__spacetimedb__","function":"__spacetimedb__","message":"Database initialized"}
{"level":"Info","ts":1758638431077171,"target":"quickstart_chat_module","filename":"modules/quickstart-chat/src/lib.rs","line_number":30,"function":"set_name","message":"User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2 sets name to phoebe"}
{"level":"Info","ts":1758638444593588,"target":"quickstart_chat_module","filename":"modules/quickstart-chat/src/lib.rs","line_number":55,"function":"send_message","message":"User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2: Hey!"}
{"level":"Info","ts":1758639333213621,"target":"quickstart_chat_module","filename":"modules/quickstart-chat/src/lib.rs","line_number":55,"function":"send_message","message":"User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2: Hey!"} |
NIT: I would reverse the order of function and target since the function is within the target. NIT: Is it possible to just not print anything instead of |
Happy to reverse these; the order I chose was arbitrary. I don't think it's necessarily true that the function is within the target module, strictly speaking, though. What function is executing is a dynamic property, whereas the target is a static property. It is possible to get two logs with the same target, filename and line number but different functions, and/or with a function where the defining form for the listed reducer is defined somewhere entirely unrelated to the target, filename and line number. But again, happy to reverse these, I don't care about the order.
Sure, that's easy. Two different ways we could do this:
Do you have a preference? |
|
WRT point 1: Ah I see, yeah then I'd just leave it as is. WRT point 2: Well I suppose they logically don't have a target (that the user created) nor a function (that the user created). A sentinel makes a little more sense for a target kinda? I slightly favor option 2, but I don't have a strong opinion. |
Also add a tiny bit of logic to not print the `: ` separator if there's nothing before it.
Done. |
For the record, I filter these sentinel values in the log explorer of my dashboard: |
|
Log output now looks like: spacetime logs -s local quickstart-chat
Compiling spacetimedb-cli v1.4.0 (/home/phoebe/clockworklabs/SpacetimeDB/crates/cli)
Finished `release` profile [optimized] target(s) in 9.53s
Finished `release` profile [optimized] target(s) in 0.14s
Running `/home/phoebe/clockworklabs/SpacetimeDB/target/x86_64-unknown-linux-gnu/release/spacetimedb-cli logs -s local quickstart-chat`
2025-09-23T14:40:18.218267Z INFO: Creating table `message`
2025-09-23T14:40:18.218327Z INFO: Creating table `user`
2025-09-23T14:40:18.218479Z INFO: Invoking `init` reducer
2025-09-23T14:40:18.218567Z INFO: Database initialized
2025-09-23T14:40:31.077171Z INFO: set_name modules/quickstart-chat/src/lib.rs:30: User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2 sets name to phoebe
2025-09-23T14:40:44.593588Z INFO: send_message modules/quickstart-chat/src/lib.rs:55: User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2: Hey!
2025-09-23T14:55:33.213621Z INFO: send_message modules/quickstart-chat/src/lib.rs:55: User c2004cbe351992901ea1f3ecd09a36d318bd5289f4eb17c7fad36c025fd0eba2: Hey!Raw JSON is unchanged. |
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 comment on SpacetimeDB internals but otherwise LGTM
You'll have to switch this to filter on |
cloutiertyler
left a comment
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.
LGTM
Description of Changes
Extends log records with a new field,
function, which stores the name of the reducer being executed when the log was produced. I have chosen to name this fieldfunctionrather thanreducerbecause we will soon be adding procedures, which are not reducers but will also be valid values for this field.While making this change, I noticed inconsistent values for injected logs. Previously, we injected logs in three places, with different values for the record fields:
SystemLogger(used when publishing and updating) setfilename: Some("spacetimedb")andtarget: None.log_reducer_error(used for reducer error returns) setfilename: Noneandtarget: Some(reducer), withreducerbeing the name of the reducer.ModuleHost::inject_logs(used for calls to nonexistent reducers and calls with ill-typed arguments) setfilename: Some("external")andtarget: None.With this change, I have decided that injected logs universally have
filename: Some("__spacetimedb__")andtarget: Some("__spacetimedb__"). I have chosen the double-underscore convention for reserved names to avoid confusion should a user name a source file or reducerspacetimedb. I am not terribly attached to usingspacetimedbhere, and would be happy to change to some other string, likeinternalorsystem.I have further decided that
log_reducer_errorandinject_logsboth have access to a sensible non-Nonevalue forfunctionand so should set it to the name of the reducer. Thetargetfield is not used for this. Note that in cases where a client attempts to call a non-existent reducer, thefunctionfield of the log record will be set to that client-supplied non-existent reducer name.API and ABI breaking changes
Changes our log record format. This is at least backwards-compatible (as in, newer consumers can read older logs) because the new field is optional. I am unsure if it is forwards-compatible (as in, older consumers reading newer logs) because I don't know if
serde_jsonwill ignore unknown fields or will error.Expected complexity level and risk
1
Testing
quickstart-chat(modified to log messages and name changes) and saw sensible output.