Skip to content

chore(envoy-client): log command and event payloads#4751

Merged
NathanFlurry merged 1 commit intomainfrom
sleep-cleanup/11-info-log-envoy-commands-events
Apr 27, 2026
Merged

chore(envoy-client): log command and event payloads#4751
NathanFlurry merged 1 commit intomainfrom
sleep-cleanup/11-info-log-envoy-commands-events

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Summary

  • log each decoded envoy command wrapper at info level when commands arrive
  • log each event wrapper at info level when events are sent or resent

Verification

  • cargo check -p rivet-envoy-client

@claude
Copy link
Copy Markdown

claude Bot commented Apr 24, 2026

PR Review: chore(envoy-client): log command and event payloads

PR #4751 — Draft | adds per-command and per-event tracing::info! log lines using the existing stringify.rs module.


Overview

Adds diagnostic logging to handle_commands, handle_send_events, and resend_unacknowledged_events so each individual protocol message is logged. Builds on top of the already-existing stringify.rs module.


Issues

Log level is too high for per-item messages

All new lines use tracing::info!. In any environment with real traffic, this will emit one log line per command and one per event — on top of the existing batch-count lines that already exist at info. This will flood logs in production and increase storage costs significantly.

Recommendation: use tracing::debug! (or tracing::trace!) for the per-item lines. The batch-count lines (received commands, sending events, resending unacknowledged events) are appropriate at info.

// Keep count at info:
tracing::info!(command_count = commands.len(), "received commands");
// Move per-item to debug:
for command_wrapper in &commands {
    tracing::debug!(
        command = %stringify_command_wrapper(command_wrapper),
        "received command"
    );
}

Eager string allocation regardless of log level

stringify_command_wrapper and stringify_event_wrapper return owned Strings and are called unconditionally — the tracing macro evaluates all field expressions before checking whether the level is enabled. In a production binary with info as the minimum level, moving to debug! would let the subscriber short-circuit before the strings are built. If these must stay at info, guard with:

if tracing::enabled!(tracing::Level::INFO) {
    tracing::info!(command = %stringify_command_wrapper(w), "received command");
}

actor_id and generation should be separate structured fields

The CLAUDE.md convention is to use discrete structured fields (e.g. actor_id = %..., generation = ...) rather than embedding them inside a formatted string. Separate fields make it possible to filter/query logs by actor in structured log sinks.

tracing::debug!(
    actor_id = %wrapper.checkpoint.actor_id,
    generation = wrapper.checkpoint.generation,
    index = wrapper.checkpoint.index,
    command = %stringify_command(&wrapper.inner),
    "received command"
);

Extra iteration in handle_commands

The logging loop iterates over &commands and then the processing loop iterates again. This is harmless in practice (commands are small batches), but logging inline during processing would avoid the double pass.


Minor

  • events.rs adds a new tracing::info!(event_count = ..., "sending events") line that didn't exist before — good addition at the right level.
  • The stringify.rs module itself is well-structured and handles all protocol variants cleanly.

Summary

The stringify.rs foundation is solid. The main ask before merging: drop the per-item log lines from info to debug, which fixes both the production noise concern and the eager-allocation concern in one change.

Copy link
Copy Markdown
Member Author

NathanFlurry commented Apr 24, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

Preview packages published to npm

Install with:

npm install rivetkit@pr-4751

All packages published as 0.0.0-pr.4751.f6ab1d3 with tag pr-4751.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-f6ab1d3
docker pull rivetdev/engine:full-f6ab1d3
Individual packages
npm install rivetkit@pr-4751
npm install @rivetkit/react@pr-4751
npm install @rivetkit/rivetkit-napi@pr-4751
npm install @rivetkit/workflow-engine@pr-4751

Base automatically changed from sleep-cleanup/16-minor-fixes to main April 27, 2026 07:13
@NathanFlurry NathanFlurry merged commit bac1e2d into main Apr 27, 2026
35 of 42 checks passed
@NathanFlurry NathanFlurry deleted the sleep-cleanup/11-info-log-envoy-commands-events branch April 27, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant