mlog: Add async I/O and sampling support to MLogger infrastructure

[akash-a-n]

Enhance MoQ-level logging (mlog) with off-thread I/O and per-session sampling:

• Async off-thread disk I/O: FileMLogger now supports optional setWriteExecutor() for asynchronous file writes. Pre-formatting of log events happens on the calling thread (safe on session's event loop), while disk I/O happens on a background executor. Avoids this capture by moving all data into the lambda.

• Path-agnostic factory: Simplified FileMLoggerFactory constructor to accept only VantagePoint. Callers now set path per-logger via setPath(), enabling per-session file naming without factory reconstruction.

• Session sampling: New SamplingMLoggerFactory wraps any MLoggerFactory and applies probabilistic sampling via folly::Random::oneIn() (thread-safe). Enables selective logging (e.g., 1% of sessions) for production deployments.

• moxygen/mlog/FileMLogger.{h,cpp}: Add setWriteExecutor() and async outputLogs()

• moxygen/mlog/FileMLoggerFactory.h: Path-agnostic API, remove path constructor param

• moxygen/mlog/SamplingMLoggerFactory.{h,cpp}: New sampling decorator (generic utility)

• moxygen/mlog/CMakeLists.txt: Add moxygen_mlog_sampling_mlogger_factory target

• moxygen/moqtest/MoQTestServerMain.cpp: Update to use new path-agnostic API

• moxygen/samples/date/MoQDateServer.cpp: Explicit setPath() call for sample code

All data passed to background executor is moved (no references, no this capture):

• Pre-format logs on event thread (where formatLog() is safe)
• Move serialized strings + path into lambda on background executor
• Safe for logger destruction before async task completes

Sampling uses xoshiro256pp PRNG via folly::Random::oneIn(), providing:

• Thread-safe sampling across EventBase threads
• Better statistical properties than modulo-based approaches
• O(1) session creation overhead regardless of sample rate

---

This change is 

[afrind]

The core functionality looks good. I think some of these changes can go upstream -- at least FileMLogger/Factory? If so having a stack of commits like:

FileMLogger changes
SamplingMlogger
Add MLogger to pico

As long as there's a named branch for the upstreamable piece, I can make a PR from it.

@afrind reviewed 11 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on akash-a-n, gmarzot, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

---

moxygen/mlog/FileMLogger.cpp line 18 at r1 (raw file):

  std::string path;
  if (dir_) {
    // Try DCID first, then fall back to server's own CID (srcCid), otherwise

Can we require dcid and remove the fallback logic?

---

moxygen/mlog/FileMLogger.cpp line 49 at r1 (raw file):

  if (writeExecutor_) {
    // Pre-format logs on the calling thread (where formatLog() is safe to

Maybe make this a lamba that's executed in both cases, either inline or via ->add

---

moxygen/mlog/SamplingMLoggerFactory.cpp line 16 at r1 (raw file):

    : inner_(std::move(inner)), sampleRate_(sampleRate) {
  // bucketSize = how many sessions per one logged session, e.g. 0.01 -> 100
  bucketSize_ = static_cast<uint32_t>(1.0f / sampleRate_);

Watch for divide by 0 here? Can you handle the sample rate checks you have in createMLogger here so it's always sane there (would oneIn(0) always return false?

---

moxygen/mlog/test/SamplingMLoggerFactoryTest.cpp line 74 at r1 (raw file):

// Use a large sample to stay within a wide tolerance (30–70%) with
// overwhelming probability, avoiding flakiness.
TEST_F(SamplingMLoggerFactoryTest, HalfRate_ApproximatelyHalfSampled) {

Seems like it's just testing oneIn - not sure it's necessary? Actually that may be more or less true for most of these tests?

[akash-a-n]

Split it into 3 commits as suggested.
created: #236 that contains only FileMLogger changes to be upstreamed

@akash-a-n made 5 comments.
Reviewable status: 6 of 11 files reviewed, 4 unresolved discussions (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

---

moxygen/mlog/FileMLogger.cpp line 18 at r1 (raw file):

Previously, afrind wrote…

Can we require dcid and remove the fallback logic?

Done.

---

moxygen/mlog/FileMLogger.cpp line 49 at r1 (raw file):

Previously, afrind wrote…

Maybe make this a lamba that's executed in both cases, either inline or via ->add

Done.

---

moxygen/mlog/SamplingMLoggerFactory.cpp line 16 at r1 (raw file):

Previously, afrind wrote…

Watch for divide by 0 here? Can you handle the sample rate checks you have in createMLogger here so it's always sane there (would oneIn(0) always return false?

Done.

---

moxygen/mlog/test/SamplingMLoggerFactoryTest.cpp line 74 at r1 (raw file):

Previously, afrind wrote…

Seems like it's just testing oneIn - not sure it's necessary? Actually that may be more or less true for most of these tests?

Done.

[akash-a-n]

@akash-a-n made 1 comment.
Reviewable status: 6 of 11 files reviewed, 4 unresolved discussions (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

---

moxygen/mlog/FileMLogger.cpp line 18 at r1 (raw file):

Previously, akash-a-n wrote…

Done.

@afrind I was doing some more testing today with conformance scripts and noticed that mvfst allows setting 0 len connection-id which is used by all conformance tests.
Is this not a real production use case? Do we still want to require DCID and fail logging if it isn't present?
PS: the current impl has a small bug, it checks for dcid only; where as the dcid_->hex() can be "".

[afrind]

@afrind reviewed 9 files and all commit messages, made 2 comments, and resolved 4 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on gmarzot, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

---

moxygen/mlog/FileMLogger.cpp line 18 at r1 (raw file):

Previously, akash-a-n wrote…

@afrind I was doing some more testing today with conformance scripts and noticed that mvfst allows setting 0 len connection-id which is used by all conformance tests.
Is this not a real production use case? Do we still want to require DCID and fail logging if it isn't present?
PS: the current impl has a small bug, it checks for dcid only; where as the dcid_->hex() can be "".

I guess we could fallback to a hash of the address tuple?

---

moxygen/mlog/FileMLogger.cpp line 20 at r3 (raw file):

    fileObj << jsonLog << std::endl;
  }
  fileObj.close();

Is this implicit in fileObj dtor?