logging: operator guide + --logging/--log-handler multi-flag support#437
logging: operator guide + --logging/--log-handler multi-flag support#437gmarzot wants to merge 9 commits into
Conversation
Two changes to PR #437: 1. Operator-facing shorthand: --logging=DBG2 now works as a synonym for --logging=.=DBG2 (the long form keeps working). Composes naturally with category specs and handler-settings blocks: --logging=DBG2 # root at DBG2 --logging=DBG2,moxygen=DBG4 # root + per-category bump --logging=WARN; default:async=false # root + handler tweak folly's own LogConfigParser requires <category>=<level> form and would throw on a bare token, so the shim runs in moqx main() *before* folly::Init and rewrites argv slots. Stable storage via std::list keeps the new pointers valid past gflag parsing. New files: src/LoggingShortcut.{h,cpp}, test/LoggingShortcutTest.cpp. Wired into main.cpp:74 immediately before folly::Init. Added to moqx_core SRCS so other targets can reuse if they ever need it. 2. Doc fixes in docs/logging.md: several examples used `;` between category specs, which is actually incorrect folly grammar. `;` is the single boundary between the categories block and the handler block; commas separate items within each block. Affected examples: - Typical multi-category config (was: `.=INFO; moqx=DBG4; ...`) - Per-stack QUIC examples (was: `quic.mvfst=DBG2; quic.picoquic=DBG2`) - Flaky-session combine-layers (was: `.=INFO; moqx.MoqxRelay=...`) - Quiet-noisy-layer (was: `.=INFO; quic=WARN`) - FOLLY_LOGGING env var example (was: `.=INFO; moqx=DBG2`) Added a 'Delimiter rules' subsection with a table to keep this from biting the next reader, and a 'Bare-level shorthand' subsection covering the new ergonomics. Smoke: 8/8 unit tests pass; full moqx binary rebuilds clean; `moqx --logging=WARN` flows through folly's parser without error.
mondain
left a comment
There was a problem hiding this comment.
@mondain reviewed 7 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on afrind, akash-a-n, peterchave, suhasHere, and TimEvens).
afrind
left a comment
There was a problem hiding this comment.
I find the guide to be too long, with redundant information and excessive detail cluttering things. Maybe have the agent make another pass to crisp it up?
@afrind reviewed 7 files and all commit messages, and made 8 comments.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on akash-a-n, peterchave, suhasHere, and TimEvens).
docs/logging.md line 3 at r1 (raw file):
# Logging moqx and every layer of its stack (moxygen, fizz, wangle, mvfst, picoquic transport, and over time proxygen and folly itself) emit logs through **folly's XLOG framework**. A single CLI flag controls the entire stack uniformly:
Suggestion:
moqx and every layer of its stack (moxygen, fizz, wangle, mvfst, picoquic transport, and over time proxygen and folly itself) emit debug logs through **folly's XLOG framework**. A single CLI flag controls the entire stack uniformly:docs/logging.md line 23 at r1 (raw file):
| Crank everything (firehose) | `moqx … --logging=DBG4` | | Production: log to file, WARN baseline | `moqx … --logging=WARN --log-handler=default:async=true,sync_level=WARN` | | Hot-path tracing without losing throughput | `moqx … --logging=moqx=DBG4 --log-handler=default:async=true,sync_level=WARN` (async on by default already) |
I was wondering how this was different from "Crank everything". Is the answer: because any --logging string automatically nukes async:true? That is probably unexpected, so maybe we should add async:true unless the log string has async:false?
docs/logging.md line 164 at r1 (raw file):
moxygen.MoQFramer Wire format encode/decode moxygen.MoQCodec Per-stream framing moxygen.MoQForwarder Track-level multiplex to subscribers
Is it not moxygen.relay.MoQForwarder?
docs/logging.md line 165 at r1 (raw file):
moxygen.MoQCodec Per-stream framing moxygen.MoQForwarder Track-level multiplex to subscribers moxygen.MoQCache Forward-cache (object/group lookup)
We don't use MoQCache
docs/logging.md line 342 at r1 (raw file):
- **XLOG** is the operator/dev surface — uniform across the stack, async, controlled by `--logging=`. - **qlog** is the IETF-standard structured QUIC log — JSON files for tooling like [qvis](https://qvis.quictools.info/) to analyze packet timelines, congestion control, and stream events.
This can also be an operator/dev surface. The distinction is the
a) standard, structured format can be consumed by external tools
b) moqx (will soon) support enabling mlog/qlog on a per-session basis, including random sampling
--logging is good for "what just went wrong" and clues embedded in the code
mlog is good for "I want a trace of a single session"
src/LoggingMultiFlag.cpp line 16 at r1 (raw file):
namespace { void joinNonEmpty(std::string& out, char sep, const std::vector<folly::StringPiece>& values) {
I think std::string_view is preferred to StringPiece unless needed for compat now?
src/main.cpp line 79 at r1 (raw file):
// any multiple instances into a single composite before folly::Init // sees them — lets operators write `--logging=A --logging=B` instead // of having to shell-quote `--logging='A;B'`.
consider condensing comment to 1 or maybe 2 lines
|
Also the PR title is somewhat misleading because there's C++ code here to do the log arg consolidation |
afrind's review: doc trim — 396 → 178 lines: - Drop duplicate compile-time-baseline section; consolidate into one block that also notes folly::Init uses LoggerDB::updateConfig (merge, not replace) so runtime --logging=DBG4 doesn't clobber the baseline async/sync_level — addresses afrind's question on the TL;DR rows - TL;DR cleaned up: drop the rows that were just restating the baseline with extra --log-handler that didn't actually change anything - Practical-examples section collapsed from 9 subsections of prose into one table (Common operator patterns) - 'Async behavior' standalone section folded into the baseline block - qlog section reframed per afrind's note: not 'XLOG good, qlog niche' but 'XLOG for what just went wrong; qlog for trace one session' with the per-session-enable-and-sample plan mentioned doc fixes: - s/emit logs/emit debug logs/ (afrind's wording suggestion) - moxygen.MoQForwarder → moxygen.relay.MoQForwarder (correct path) - Drop moxygen.MoQCache from the hierarchy — moqx doesn't link moxygen_moq_cache (it has its own moqx_cache) code: - LoggingMultiFlag: folly::StringPiece → std::string_view per afrind's preference; folly types only used where the API requires them (folly::parseLogConfig still takes StringPiece, but we never expose it) - main.cpp: condense the call-site comment from 4 lines → 2 All 17 LoggingMultiFlag tests still pass; format check clean.
Add docs/logging.md — a comprehensive operator-facing guide for the
folly XLOG framework that now governs every layer of the stack
(moxygen, fizz, wangle, mvfst, picoquic transport bridge, with
proxygen and folly itself to follow).
Covers:
- Single --logging=<config-string> CLI for the whole stack
- TL;DR table for the most common operational scenarios
- Three-layer config composition (compile-time baseline → env vars →
CLI flag) and how they override
- Category hierarchy (moqx.*, moxygen.*, quic.{mvfst,picoquic}.*)
with the multi-QUIC-stack disambiguation pattern
- Severity ladder (DBG9..DBG0, INFO, WARN, ERR, CRITICAL, FATAL) with
per-level conventions for what each layer emits
- Async behavior + sync_level escalation (default: async=true,
sync_level=WARN — important events flush synchronously)
- ~10 worked operator examples (production logging, hot-path tracing,
per-class filtering, qlog coexistence)
- Migration table from the legacy glog flags
- Troubleshooting section for the most common 'why is nothing being
logged' / 'log file doesn't exist' / 'category isn't matching'
scenarios
This doc is intentionally a living reference — additional findings
during end-to-end vetting of the XLOG bridge (moxygen#256, just
landed) will accumulate here as PRs against this same file.
Two changes to PR #437: 1. Operator-facing shorthand: --logging=DBG2 now works as a synonym for --logging=.=DBG2 (the long form keeps working). Composes naturally with category specs and handler-settings blocks: --logging=DBG2 # root at DBG2 --logging=DBG2,moxygen=DBG4 # root + per-category bump --logging=WARN; default:async=false # root + handler tweak folly's own LogConfigParser requires <category>=<level> form and would throw on a bare token, so the shim runs in moqx main() *before* folly::Init and rewrites argv slots. Stable storage via std::list keeps the new pointers valid past gflag parsing. New files: src/LoggingShortcut.{h,cpp}, test/LoggingShortcutTest.cpp. Wired into main.cpp:74 immediately before folly::Init. Added to moqx_core SRCS so other targets can reuse if they ever need it. 2. Doc fixes in docs/logging.md: several examples used `;` between category specs, which is actually incorrect folly grammar. `;` is the single boundary between the categories block and the handler block; commas separate items within each block. Affected examples: - Typical multi-category config (was: `.=INFO; moqx=DBG4; ...`) - Per-stack QUIC examples (was: `quic.mvfst=DBG2; quic.picoquic=DBG2`) - Flaky-session combine-layers (was: `.=INFO; moqx.MoqxRelay=...`) - Quiet-noisy-layer (was: `.=INFO; quic=WARN`) - FOLLY_LOGGING env var example (was: `.=INFO; moqx=DBG2`) Added a 'Delimiter rules' subsection with a table to keep this from biting the next reader, and a 'Bare-level shorthand' subsection covering the new ergonomics. Smoke: 8/8 unit tests pass; full moqx binary rebuilds clean; `moqx --logging=WARN` flows through folly's parser without error.
While adding tests, the empirical diagnostic 'FollyTreatsBareLevelAsRoot'
showed that folly::parseLogConfig("DBG2") and folly::parseLogConfig(
".=DBG2") produce the IDENTICAL LogConfig structure. Folly's parser
already accepts a bare level token as a root-category set.
That makes the LoggingShortcut shim — argv preprocessor that prepended
'.=' to bare levels before folly::Init — strictly redundant. Removing:
- src/LoggingShortcut.{h,cpp} (deleted)
- test/LoggingShortcutTest.cpp (deleted)
- The wire-up in main.cpp and CMakeLists.txt / test/CMakeLists.txt
The doc work stays — the value the previous commit added was the
delimiter-rules clarification (\`;\` vs \`,\` is the most common
trip-up), the corrected examples, and the no-spaces convention. The
Bare-level shorthand section now correctly attributes the behavior to
folly itself rather than claiming it's moqx-side ergonomics.
Other doc cleanups in this commit:
- Strip spaces after \`;\` in all code-block examples (consistent
no-spaces style)
- Update FOLLY_INIT_LOGGING_CONFIG default in main.cpp to match the
no-spaces convention (.=INFO;default:async=true,sync_level=WARN)
- Drop the misleading 'this fails' example for the \`;\`-between-
categories form — folly actually silently misparses it, not errors,
but the cleaner teach is to just show the right form
Replace the single `--logging=<cats>;<handlers>` form (which requires
shell-quoting the `;` block boundary) with two flags, each with one job:
--logging category specs; multiple instances joined with `,`
--log-handler handler configs; multiple instances joined with `;`
Both can be passed repeatedly without any shell-quoting:
moqx --logging=INFO --logging=moxygen=DBG4 \
--log-handler=default:async=true,sync_level=WARN
Internally, an argv preprocessor (combineLoggingArgs, runs before
folly::Init) composes all flag occurrences into a single folly-grammar
composite string and rewrites every `--logging` and `--log-handler`
argv slot to `--logging=<composite>`. gflags last-wins then picks the
composite regardless of which flag was last. folly never sees the
unknown `--log-handler` name because the rewrite renames it.
The doc is updated throughout to lead with the bare-level form
(`--logging=INFO`) for global-level settings — preferred over the
longer `--logging=.=INFO` — and to use the two-flag form in every
example that previously had an embedded `;`. The migration table,
troubleshooting section, and production file-logging example all
switch to the new style.
Tests:
- combineLoggingValues (pure helper): cats-only, handlers-only, both,
empty-input dropping, leading-`;` for handlers-only shape, full-shape
property test against folly's actual parseLogConfig
- combineLoggingArgs (argv mutator): single-flag bail, equals form,
separated form, mixed forms, --log-handler-alone, --logging +
--log-handler composition, unrelated flags left alone, trailing
flag-without-value safety
Smoke: `moqx --logging=INFO --logging=moxygen=DBG4
--log-handler=default:async=false --config=...` parses through gflags
cleanly; reaches config-file parsing (which is the expected next step).
17/17 tests pass via scripts/build.sh.
The previous version had it as a one-line bullet under 'How configuration layers compose', which didn't actually explain what the baseline is or when you'd touch it. Add a 'The compile-time baseline in detail' subsection covering: - What FOLLY_INIT_LOGGING_CONFIG does and where it lives - A line-by-line read of the current baseline string - Realistic reasons to change it for different binaries (debug, locked- down prod, test) - How to change it (edit main.cpp directly, or drive via CMake target_compile_definitions) - The 'what it is NOT' clarification — the baseline only sets a runtime default; it does not strip call sites at compile time - Cross-reference to FOLLY_XLOG_STRIP_PREFIXES which IS a true compile-time string rewrite (category-name derivation), to disambiguate the two uses of 'compile-time' in the doc
afrind's review: doc trim — 396 → 178 lines: - Drop duplicate compile-time-baseline section; consolidate into one block that also notes folly::Init uses LoggerDB::updateConfig (merge, not replace) so runtime --logging=DBG4 doesn't clobber the baseline async/sync_level — addresses afrind's question on the TL;DR rows - TL;DR cleaned up: drop the rows that were just restating the baseline with extra --log-handler that didn't actually change anything - Practical-examples section collapsed from 9 subsections of prose into one table (Common operator patterns) - 'Async behavior' standalone section folded into the baseline block - qlog section reframed per afrind's note: not 'XLOG good, qlog niche' but 'XLOG for what just went wrong; qlog for trace one session' with the per-session-enable-and-sample plan mentioned doc fixes: - s/emit logs/emit debug logs/ (afrind's wording suggestion) - moxygen.MoQForwarder → moxygen.relay.MoQForwarder (correct path) - Drop moxygen.MoQCache from the hierarchy — moqx doesn't link moxygen_moq_cache (it has its own moqx_cache) code: - LoggingMultiFlag: folly::StringPiece → std::string_view per afrind's preference; folly types only used where the API requires them (folly::parseLogConfig still takes StringPiece, but we never expose it) - main.cpp: condense the call-site comment from 4 lines → 2 All 17 LoggingMultiFlag tests still pass; format check clean.
0f7cf1f to
7da1659
Compare
afrind
left a comment
There was a problem hiding this comment.
@afrind reviewed 5 files and all commit messages, and made 2 comments.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on akash-a-n, peterchave, suhasHere, and TimEvens).
docs/logging.md line 121 at r2 (raw file):
| `DBG6`–`DBG9` | Byte-level, executor tasks, queue stats | Forensic | If you see `ERR` in steady state, file a bug. Steady `WARN` hints at an edge case the system is handling.
Oof - ok
docs/logging.md line 170 at r2 (raw file):
Independent channels — enabling one doesn't suppress the other. ## Migration from the old (glog) flags
Do we need this section? It's not like we have a giant user base.
End-to-end testing revealed two factual errors in the doc that would
mislead operators:
1. Per-category targeting is currently inconsistent. Empirically:
- --logging=moxygen.MoQServer=DBG1 fires zero moxygen.* lines
- the actual category for MoQServer.cpp is the full absolute
build-time path (home.<...>.deps.moxygen.moxygen.MoQServer)
because moxygen's FOLLY_XLOG_STRIP_PREFIXES references moqx's
CMAKE_SOURCE_DIR rather than moxygen's own, so the strip never
matches moxygen source paths
- the same prefix issue affects the QUIC stacks
- moqx's own sources DO collapse to moqx.* when
-fmacro-prefix-map=src=moqx is applied
Replace the (inaccurate) full category hierarchy with an honest
explanation: root-level and coarse-subtree mutes (e.g. quic=WARN)
work today; targeting individual source files is fragile until the
cross-project strip-prefix story is normalized as a follow-up to
issue #339.
2. folly's 'file' handler type is NOT auto-registered. From the folly
source: 'intentionally do not register FileHandlerFactory, since
this allows arbitrary file writes via the config string'. The doc
previously showed --log-handler=default=file:path=... as working; it
does not. Replace those examples with shell/systemd/Docker stderr
redirection, and add a 'File output' section explaining the security
rationale plus the registerHandlerFactory snippet for apps that need
in-process file output.
Verified via timeout-bound moqx runs against config.example.yaml; the
17 LoggingMultiFlag unit tests still pass and CI's check-format is
clean.
moqx is a young project with no legacy deployments pinning glog flags like --minloglevel or --vmodule. The migration table was doc tax for a user base that doesn't exist — exactly the kind of 'thoroughness' content afrind flagged as excessive in the first review pass. If a legacy-migration story ever emerges (e.g. a Meta-internal binary flipping over), the mapping is a one-line answer in a comment or Stack Overflow — not foundational doc material.
Summary
Adds docs/logging.md — a comprehensive operator-facing guide for the folly XLOG framework that now governs every layer of moqx's stack (moxygen, fizz, wangle, mvfst, picoquic transport bridge, with proxygen and folly itself to follow).
What's in it
moqx.*,moxygen.*,quic.{mvfst,picoquic}.*and the multi-QUIC-stack disambiguation patternDBG9..DBG0,INFO,WARN,ERR,CRITICAL,FATALwith conventions for what each layer emits per levelasync=true, sync_level=WARN; important events still flush synchronouslyWhy now
The stack-wide migration to folly XLOG is now real on
main:fizz/wangle/mvfstselector PRs landed (feat: Add TrackFilterLoadTest (live-relay blackbox load test) #168/refactor: remove MoQForwarder unit tests from MoqxRelayTest #252/#456 upstream) and standalone: folly XLOG backend for bundled deps + override knobs moxygen#246 wires them upThis doc gives operators a single place to learn the new logging model. It's intentionally a living reference — additional findings during end-to-end vetting of the bridge will accumulate here as follow-up PRs against this same file.
Test plan
This change is