Skip to content

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Sep 9, 2025

This implements an alternative to #4965 that outputs to console.log with the prefix strings stderr: and stdout: instead, along with a new configuration option:

struct LoggingOptions {
  structuredLogging @0 :Bool = false;
  # Override of top-level structured logging (only when true).
  # If true, logs will be emitted as JSON for structured logging.
  # When false, logs use the traditional human-readable format.
  # This affects the format of logs from KJ_LOG and exception reporting as well as js logs.
  # This won't work for logs coming from service worker syntax workers with the old module registry.

  stdoutPrefix @1 :Text = "stdout:";
  # Prefix process.stdout output with "stdout: "
  # Can be set to the empty string or a custom prefix to customize.

  stderrPrefix @2 :Text = "stderr:";
  # Prefix process.stderr output with "stderr: "
  # Can be set to the empty string or a custom prefix to customize.
}

which can be used to both customize and disable prefixing.

@guybedford guybedford requested review from a team as code owners September 9, 2025 22:08
@guybedford guybedford requested a review from jasnell September 9, 2025 22:08
@guybedford guybedford force-pushed the gbedford/prefixed-log branch 2 times, most recently from 3638bf4 to ba8e106 Compare September 9, 2025 22:14
@guybedford guybedford force-pushed the gbedford/prefixed-log branch 6 times, most recently from fefa521 to d27954a Compare September 10, 2025 00:26
Copy link
Collaborator

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few nits, but generally LGTM

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried this out locally with Wrangler and the output looks good to me.

Only tiny, non-blocking, thought is whether the configuration option could be to specify the prefixes, rather than just a boolean. This would make it really nice for people to be able to still have prefixes but avoid collisions with other prefixes.

petebacondarwin added a commit to cloudflare/workers-sdk that referenced this pull request Sep 10, 2025
@guybedford guybedford force-pushed the gbedford/prefixed-log branch from 37b98c1 to 6ad99bd Compare September 10, 2025 17:58
Copy link

github-actions bot commented Sep 10, 2025

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

Copy link
Collaborator

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one small nit on the config capnp

@guybedford guybedford force-pushed the gbedford/prefixed-log branch from 304792b to c9025c7 Compare September 10, 2025 18:48
@petebacondarwin
Copy link
Contributor

petebacondarwin commented Sep 10, 2025

We'll still need to modify the Miniflare tests to account for the prefix. I can't put up a PR to relax those tests for now.

Actually since we are no longer changing how console.debug() is passed to stdout, these tests should not need to change.

@guybedford guybedford force-pushed the gbedford/prefixed-log branch 2 times, most recently from bc19e3c to 5934c56 Compare September 10, 2025 22:18
@guybedford guybedford force-pushed the gbedford/prefixed-log branch from 5934c56 to f27a807 Compare September 11, 2025 00:09
@guybedford guybedford force-pushed the gbedford/prefixed-log branch from f27a807 to 808d523 Compare September 11, 2025 00:09
@guybedford guybedford merged commit aa0b29e into main Sep 11, 2025
21 of 22 checks passed
@guybedford guybedford deleted the gbedford/prefixed-log branch September 11, 2025 01:29
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.

4 participants