Use MloggerFactory within the MoQServer

Summary:
Previously, we just had one `MLogger` for the whole `MoQServer`. Instead, we'd like to have one `MLogger` per `MoQSession`, which is similar to how we have one `QLogger` for each QUIC connection in mvfst.

In order to do this, I'm creating an `MLoggerFactory` member of the `MoQServer` that would allow us to create a new `MLogger` whenever a new session is created.

Reviewed By: sandarsh

Differential Revision: D89326321

fbshipit-source-id: 3b2b6990f764f98a5a5fbdf09c1937c73ba00b7f

Author: Aman Sharma

moxygen/MoQServer.cpp

@@ -115,6 +115,10 @@ void MoQServer::createMoQQuicSession(
   auto evb = qevb->getTypedEventBase<quic::FollyQuicEventBase>()
                  ->getBackingEventBase();
   auto moqSession = createSession(std::move(wt), getOrCreateExecutor(evb));
+  if (mLoggerFactory_) {
+    auto logger = createLogger();
+    moqSession->setLogger(logger);
+  }
 
   // Configure session based on negotiated ALPN
   if (alpn) {
@@ -283,8 +287,8 @@ folly::Try<ServerSetup> MoQServer::onClientSetup(
   };
 
   // Log Server Setup
-  if (logger_) {
-    logger_->logServerSetup(serverSetup);
+  if (auto logger = session->getLogger()) {
+    logger->logServerSetup(serverSetup);
   }
 
   return folly::Try<ServerSetup>(serverSetup);
@@ -351,6 +355,10 @@ void MoQServer::Handler::onHeadersComplete(
   clientSession_ = server_.createSession(
       folly::MaybeManagedPtr<proxygen::WebTransport>(wt),
       server_.getOrCreateExecutor(evb));
+  if (server_.mLoggerFactory_) {
+    auto logger = server_.createLogger();
+    clientSession_->setLogger(logger);
+  }
 
   // Configure session based on negotiated WebTransport protocol
   if (negotiatedProtocol) {
@@ -360,12 +368,15 @@ void MoQServer::Handler::onHeadersComplete(
   co_withExecutor(evb, server_.handleClientSession(clientSession_)).start();
 }
 
-void MoQServer::setLogger(std::shared_ptr<MLogger> logger) {
-  logger_ = std::move(logger);
+void MoQServer::setMLoggerFactory(std::shared_ptr<MLoggerFactory> factory) {
+  mLoggerFactory_ = std::move(factory);
 }
 
-std::shared_ptr<MLogger> MoQServer::getLogger() const {
-  return logger_;
+std::shared_ptr<MLogger> MoQServer::createLogger() const {
+  if (mLoggerFactory_) {
+    return mLoggerFactory_->createMLogger();
+  }
+  return nullptr;
 }
 
 void MoQServer::setQuicStatsFactory(
@@ -384,10 +395,11 @@ std::shared_ptr<MoQExecutor> MoQServer::getOrCreateExecutor(
 std::shared_ptr<MoQSession> MoQServer::createSession(
     folly::MaybeManagedPtr<proxygen::WebTransport> wt,
     std::shared_ptr<MoQExecutor> executor) {
-  return std::make_shared<MoQSession>(
+  auto session = std::make_shared<MoQSession>(
       folly::MaybeManagedPtr<proxygen::WebTransport>(std::move(wt)),
       *this,
       std::move(executor));
+  return session;
 }
 
 } // namespace moxygen

moxygen/MoQServer.h

@@ -7,7 +7,7 @@
 #pragma once
 
 #include <proxygen/httpserver/samples/hq/HQServer.h>
-#include <moxygen/mlog/MLogger.h>
+#include <moxygen/mlog/MLoggerFactory.h>
 
 #include <folly/init/Init.h>
 #include <folly/io/async/EventBaseLocal.h>
@@ -48,8 +48,7 @@ class MoQServer : public MoQSession::ServerSetupCallback {
     return hqServer_->getWorkerEvbs();
   }
 
-  void setLogger(std::shared_ptr<MLogger> logger);
-  std::shared_ptr<MLogger> getLogger() const;
+  void setMLoggerFactory(std::shared_ptr<MLoggerFactory> factory);
 
   // QUIC stats factory setter
   void setQuicStatsFactory(
@@ -104,6 +103,9 @@ class MoQServer : public MoQSession::ServerSetupCallback {
   // Register ALPN handlers for direct QUIC connections (internal use)
   void registerAlpnHandler(const std::vector<std::string>& alpns);
 
+  // Create a logger from the factory if one is set
+  std::shared_ptr<MLogger> createLogger() const;
+
  private:
   // AUTHORITY parameter validation methods
   // Not called for HTTP inside proxygen, we leave it to applications.
@@ -183,7 +185,9 @@ class MoQServer : public MoQSession::ServerSetupCallback {
   std::unique_ptr<quic::samples::HQServerTransportFactory> factory_;
   std::unique_ptr<quic::samples::HQServer> hqServer_;
   std::string endpoint_;
-  std::shared_ptr<MLogger> logger_;
+  std::shared_ptr<MLoggerFactory> mLoggerFactory_;
   folly::EventBaseLocal<std::shared_ptr<MoQExecutor>> executorLocal_;
+
+  friend class Handler;
 };
 } // namespace moxygen

moxygen/MoQSession.cpp

@@ -2074,6 +2074,9 @@ MoQSession::MoQSession(
 
 MoQSession::~MoQSession() {
   cleanup();
+  if (logger_) {
+    logger_->outputLogsToFile();
+  }
   XLOG(DBG1) << __func__ << " sess=" << this;
 }
 
@@ -2166,6 +2169,10 @@ void MoQSession::setLogger(const std::shared_ptr<MLogger>& logger) {
   logger_ = logger;
 }
 
+std::shared_ptr<MLogger> MoQSession::getLogger() const {
+  return logger_;
+}
+
 void MoQSession::drain() {
   XLOG(DBG1) << __func__ << " sess=" << this;
   draining_ = true;

moxygen/MoQSession.h

@@ -376,6 +376,7 @@ class MoQSession : public Subscriber,
       RequestID requestID);
 
   void setLogger(const std::shared_ptr<MLogger>& logger);
+  std::shared_ptr<MLogger> getLogger() const;
   RequestID peekNextRequestID() const {
     return nextRequestID_;
   }

moxygen/moqtest/MoQTestServer.h

@@ -59,12 +59,24 @@ class MoQTestServer : public moxygen::Publisher,
  public:
   MoQTestServer(const std::string& cert = "", const std::string& key = "");
 
+  // Server-level logger methods
+  void setLogger(std::shared_ptr<MLogger> logger) {
+    logger_ = std::move(logger);
+  }
+
+  std::shared_ptr<MLogger> getLogger() const {
+    return logger_;
+  }
+
   //  Override onNewSession to set publisher handler to be this object
   virtual void onNewSession(
       std::shared_ptr<MoQSession> clientSession) override {
     clientSession->setPublishHandler(shared_from_this());
-    if (getLogger()) {
-      clientSession->setLogger(getLogger());
+    // Use server-level logger if set, otherwise try factory
+    if (logger_) {
+      clientSession->setLogger(logger_);
+    } else if (auto logger = createLogger()) {
+      clientSession->setLogger(std::move(logger));
     }
   }
 
@@ -139,6 +151,9 @@ class MoQTestServer : public moxygen::Publisher,
   std::shared_ptr<MoQRelaySession> relaySession_;
   std::shared_ptr<Subscriber::AnnounceHandle> announceHandle_;
   std::shared_ptr<MoQFollyExecutorImpl> moqEvb_;
+
+  // Server-level logger
+  std::shared_ptr<MLogger> logger_;
 };
 
 } // namespace moxygen

moxygen/relay/MoQRelayClient.h

@@ -112,6 +112,11 @@ class MoQRelayClient {
       }
     }
     announceHandles_.clear();
+    // Flush mLogs before closing the session, since closeSession() does not
+    // trigger onSessionEnd callback (where logs would normally be flushed)
+    if (moqClient_ && moqClient_->logger_) {
+      moqClient_->logger_->outputLogsToFile();
+    }
     if (moqClient_ && moqClient_->moqSession_) {
       // Break strong references held by the session to the Publisher/Subscriber
       moqClient_->moqSession_->setPublishHandler(nullptr);

moxygen/samples/date/MoQDateServer.cpp

@@ -11,6 +11,7 @@
 #include <moxygen/MoQLocation.h>
 #include <moxygen/MoQServer.h>
 #include <moxygen/MoQWebTransportClient.h>
+#include <moxygen/mlog/FileMLoggerFactory.h>
 #include <moxygen/relay/MoQForwarder.h>
 #include <moxygen/relay/MoQRelayClient.h>
 #include <moxygen/util/InsecureVerifierDangerousDoNotUseInProduction.h>
@@ -130,8 +131,8 @@ class MoQDateServer : public MoQServer,
                                    MoQRelaySession::createRelaySessionFactory(),
                                    verifier)));
 
-    if (getLogger()) {
-      relayClient_->setLogger(getLogger());
+    if (auto logger = createLogger()) {
+      relayClient_->setLogger(logger);
     }
 
     // Default to experimental protocols, override to legacy if flag set
@@ -162,8 +163,11 @@ class MoQDateServer : public MoQServer,
 
   void onNewSession(std::shared_ptr<MoQSession> clientSession) override {
     clientSession->setPublishHandler(shared_from_this());
-    if (getLogger()) {
-      clientSession->setLogger(getLogger());
+  }
+
+  void shutdownRelayClient() {
+    if (relayClient_) {
+      relayClient_->shutdown();
     }
   }
 
@@ -644,10 +648,9 @@ int main(int argc, char* argv[]) {
   }
 
   if (!FLAGS_mlog_path.empty()) {
-    auto logger =
-        std::make_shared<moxygen::MLogger>(moxygen::VantagePoint::SERVER);
-    logger->setPath(FLAGS_mlog_path);
-    server->setLogger(logger);
+    auto factory = std::make_shared<moxygen::FileMLoggerFactory>(
+        FLAGS_mlog_path, moxygen::VantagePoint::SERVER);
+    server->setMLoggerFactory(factory);
   }
 
   folly::SocketAddress addr("::", FLAGS_port);
@@ -656,17 +659,12 @@ int main(int argc, char* argv[]) {
     return 1;
   }
 
-  SigHandler handler(&evb, [&server, &evb](int) {
-    if (server->getLogger()) {
-      server->getLogger()->outputLogsToFile();
-    }
+  SigHandler handler(&evb, [&evb, server](int) {
+    server->shutdownRelayClient();
     evb.terminateLoopSoon();
   });
 
   evb.loopForever();
 
-  if (server->getLogger()) {
-    server->getLogger()->outputLogsToFile();
-  }
   return 0;
 }