Skip to content

Commit 7923488

Browse files
committed
admin : added a new admin endpoint - /logs to fetch log files + related bugfix
- Implement `GET /logs?type=<mlog|qlog>&connection_id=<hex>` route. - Offload blocking file I/O (`std::ifstream::read`) to `folly::getGlobalCPUExecutor()` to avoid stalling the admin event loop. - Bugfix: create log dir if not exists
1 parent 4915b55 commit 7923488

8 files changed

Lines changed: 378 additions & 1 deletion

File tree

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ add_library(moqx_core STATIC
158158
src/admin/BuiltinRoutes.cpp
159159
src/admin/CachePurgeHandler.cpp
160160
src/admin/ConfigHandler.cpp
161+
src/admin/ConnectionLogsHandler.cpp
161162
src/admin/MetricsHandler.cpp
162163
src/admin/TrackMetricsHandler.cpp
163164
src/admin/StateHandler.cpp
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
/*
2+
* Copyright (c) OpenMOQ contributors.
3+
* This source code is licensed under the Apache 2.0 license found in the
4+
* LICENSE file in the root directory of this source tree.
5+
*/
6+
7+
#include "admin/ConnectionLogsHandler.h"
8+
9+
#include <cctype>
10+
#include <string>
11+
#include <string_view>
12+
13+
#include <fcntl.h>
14+
#include <sys/stat.h>
15+
16+
#include <folly/CancellationToken.h>
17+
#include <folly/File.h>
18+
#include <folly/FileUtil.h>
19+
#include <folly/coro/Invoke.h>
20+
#include <folly/coro/Task.h>
21+
#include <folly/coro/WithCancellation.h>
22+
#include <folly/executors/GlobalExecutor.h>
23+
#include <folly/io/IOBuf.h>
24+
#include <folly/io/async/EventBaseManager.h>
25+
#include <folly/logging/xlog.h>
26+
#include <proxygen/httpserver/ResponseBuilder.h>
27+
#include <proxygen/lib/http/HTTPMessage.h>
28+
29+
#include "admin/AdminResponse.h"
30+
#include "admin/AdminServer.h"
31+
32+
namespace openmoq::moqx::admin {
33+
34+
namespace {
35+
36+
constexpr size_t kMaxDownloadBytes = 512ULL * 1024 * 1024; // 512 MB hard cap
37+
38+
// Normalize a raw connection ID string:
39+
// - strip 0x/0X prefix
40+
// - lowercase hex digits
41+
// - validate hex-only, 1–40 chars
42+
std::optional<std::string> normalizeConnectionId(std::string_view raw) {
43+
if (raw.size() >= 2 && raw[0] == '0' && (raw[1] == 'x' || raw[1] == 'X')) {
44+
raw.remove_prefix(2);
45+
}
46+
std::string result;
47+
result.reserve(raw.size());
48+
for (char c : raw) {
49+
if (!std::isxdigit(static_cast<unsigned char>(c)))
50+
return std::nullopt;
51+
result += static_cast<char>(std::tolower(static_cast<unsigned char>(c)));
52+
}
53+
if (result.empty() || result.size() > 40)
54+
return std::nullopt;
55+
return result;
56+
}
57+
58+
// Read an entire file into an IOBuf. Returns nullptr if the file cannot be
59+
// opened, is empty, or exceeds maxBytes.
60+
std::unique_ptr<folly::IOBuf> readFileToIOBuf(const std::string& path, size_t maxBytes) {
61+
folly::File file;
62+
try {
63+
file = folly::File(path, O_RDONLY);
64+
} catch (const std::exception&) {
65+
return nullptr;
66+
}
67+
68+
struct stat st{};
69+
if (::fstat(file.fd(), &st) != 0 || !S_ISREG(st.st_mode))
70+
return nullptr;
71+
const auto size = static_cast<size_t>(st.st_size);
72+
if (size == 0 || size > maxBytes)
73+
return nullptr;
74+
75+
auto content = std::make_unique<std::string>();
76+
if (!folly::readFile(file.fd(), *content, size) || content->size() != size)
77+
return nullptr;
78+
79+
auto* data = content->data();
80+
const auto len = content->size();
81+
return folly::IOBuf::takeOwnership(
82+
data,
83+
len,
84+
[](void*, void* userData) { delete static_cast<std::string*>(userData); },
85+
content.release()
86+
);
87+
}
88+
89+
// Reads the requested log file (on the global CPU pool) and writes the
90+
// HTTP response. This is a plain, explicitly-typed coroutine function
91+
// rather than an immediately-invoked generic ("auto"-parameter) lambda:
92+
// that pattern, combined with a by-value parameter used after a co_await
93+
// suspension point, is a known trigger for a GCC 11 coroutine codegen bug
94+
// that manifests as a double-free of the parameter's backing storage under
95+
// ASAN (observed for both `path` and `name` here). Using a named function
96+
// with concrete parameter types avoids the pattern entirely.
97+
folly::coro::Task<void> serveLogFile(
98+
std::string path,
99+
std::string name,
100+
proxygen::ResponseHandler* ds,
101+
folly::CancellationToken token
102+
) {
103+
if (token.isCancellationRequested())
104+
co_return;
105+
106+
// Read the file on the global CPU pool to avoid blocking the admin
107+
// event-loop thread. Use co_awaitTry (folly::Try) instead of a
108+
// try/catch around the co_await to avoid exception unwinding through
109+
// the coroutine frame (see MoqxRelay.cpp, UpstreamProvider.cpp for the
110+
// same established pattern).
111+
auto readResult = co_await folly::coro::co_awaitTry(folly::coro::co_withExecutor(
112+
folly::getGlobalCPUExecutor(),
113+
folly::coro::co_invoke(
114+
[path = std::move(path),
115+
maxBytes = kMaxDownloadBytes]() -> folly::coro::Task<std::unique_ptr<folly::IOBuf>> {
116+
co_return readFileToIOBuf(path, maxBytes);
117+
}
118+
)
119+
));
120+
if (readResult.hasException()) {
121+
XLOG(ERR) << "ConnectionLogsHandler: file read threw: " << readResult.exception().what();
122+
if (!token.isCancellationRequested()) {
123+
sendError(ds, 500, "internal error\n");
124+
}
125+
co_return;
126+
}
127+
128+
if (token.isCancellationRequested())
129+
co_return;
130+
131+
std::unique_ptr<folly::IOBuf> fileBuf = std::move(readResult.value());
132+
if (!fileBuf) {
133+
sendError(ds, 404, "log file not found or exceeds size limit\n");
134+
co_return;
135+
}
136+
137+
proxygen::ResponseBuilder(ds)
138+
.status(200, proxygen::HTTPMessage::getDefaultReason(200))
139+
.header("Content-Type", "application/json")
140+
.header("Content-Disposition", "attachment; filename=\"" + name + "\"")
141+
.body(std::move(fileBuf))
142+
.sendWithEOM();
143+
}
144+
145+
} // namespace
146+
147+
void registerConnectionLogsRoutes(
148+
AdminServer& adminServer,
149+
const std::optional<config::LoggingConfig>& logging
150+
) {
151+
std::string mlogDir, qlogDir;
152+
if (logging) {
153+
if (logging->mlog && !logging->mlog->dir.empty()) {
154+
mlogDir = logging->mlog->dir;
155+
}
156+
if (logging->qlog && !logging->qlog->dir.empty()) {
157+
qlogDir = logging->qlog->dir;
158+
}
159+
}
160+
161+
// ── GET /logs?connection_id=<hex>&type=mlog|qlog
162+
//
163+
// Path is constructed directly as {dir}/{normalized_cid}.{ext}
164+
adminServer.addRoute(
165+
"GET",
166+
"/logs",
167+
[mlogDir = std::move(mlogDir), qlogDir = std::move(qlogDir)](
168+
std::unique_ptr<proxygen::HTTPMessage> req,
169+
std::unique_ptr<folly::IOBuf> /*body*/,
170+
proxygen::ResponseHandler* downstream,
171+
folly::CancellationToken cancelToken
172+
) {
173+
// Resolve type → directory, file extension, Content-Type.
174+
const auto& typeStr = req->getQueryParam("type");
175+
const std::string* dir = nullptr;
176+
const char* ext = nullptr;
177+
if (typeStr == "mlog") {
178+
dir = &mlogDir;
179+
ext = ".mlog";
180+
} else if (typeStr == "qlog") {
181+
dir = &qlogDir;
182+
ext = ".qlog";
183+
} else {
184+
sendError(downstream, 400, "type must be 'mlog' or 'qlog'\n");
185+
return;
186+
}
187+
188+
if (dir->empty()) {
189+
sendError(downstream, 503, "that log type is not configured\n");
190+
return;
191+
}
192+
193+
const auto& rawCid = req->getQueryParam("connection_id");
194+
if (rawCid.empty()) {
195+
sendError(downstream, 400, "missing connection_id\n");
196+
return;
197+
}
198+
199+
auto normCid = normalizeConnectionId(rawCid);
200+
if (!normCid) {
201+
sendError(downstream, 400, "invalid connection_id\n");
202+
return;
203+
}
204+
205+
// {dir}/{normalizedCid}.{ext}
206+
auto filePath = *dir + "/" + *normCid + ext;
207+
auto fileName = *normCid + ext;
208+
209+
auto* evb = folly::EventBaseManager::get()->getEventBase();
210+
folly::coro::co_withCancellation(
211+
cancelToken,
212+
folly::coro::co_withExecutor(
213+
evb,
214+
serveLogFile(std::move(filePath), std::move(fileName), downstream, cancelToken)
215+
)
216+
)
217+
.start();
218+
}
219+
);
220+
}
221+
222+
} // namespace openmoq::moqx::admin

src/admin/ConnectionLogsHandler.h

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright (c) OpenMOQ contributors.
3+
* This source code is licensed under the Apache 2.0 license found in the
4+
* LICENSE file in the root directory of this source tree.
5+
*/
6+
7+
#pragma once
8+
9+
#include <optional>
10+
11+
#include "config/Config.h"
12+
13+
namespace openmoq::moqx::admin {
14+
15+
class AdminServer;
16+
17+
// Registers GET /logs on the admin server.
18+
//
19+
// GET /logs?connection_id=<hex>&type=mlog|qlog
20+
// Resolves the file path as {log_dir}/{normalized_cid}.{ext} and streams
21+
// the file directly. No index or disk scan is required — files written
22+
// after startup are immediately reachable.
23+
// Responds 400 for missing/invalid params, 503 if the requested type is
24+
// not configured, 404 if the file does not exist.
25+
void registerConnectionLogsRoutes(
26+
AdminServer& adminServer,
27+
const std::optional<config::LoggingConfig>& logging
28+
);
29+
30+
} // namespace openmoq::moqx::admin

src/config/ConfigResolver.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1240,7 +1240,11 @@ folly::Expected<ResolvedConfig, std::string> resolveConfig(const ParsedConfig& c
12401240
if (!mlogConfig.dir.empty()) {
12411241
std::error_code ec;
12421242
const auto st = std::filesystem::status(mlogConfig.dir, ec);
1243-
if (ec) {
1243+
// A missing directory is not an error here: LogSetup creates it
1244+
// (via create_directories) before logging starts. Only reject
1245+
// genuine access failures (e.g. permission denied on a parent
1246+
// directory).
1247+
if (ec && st.type() != std::filesystem::file_type::not_found) {
12441248
return folly::makeUnexpected(
12451249
"Failed to access mlog directory '" + mlogConfig.dir + "': " + ec.message()
12461250
);

src/main.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "admin/BuiltinRoutes.h"
1111
#include "admin/CachePurgeHandler.h"
1212
#include "admin/ConfigHandler.h"
13+
#include "admin/ConnectionLogsHandler.h"
1314
#include "admin/MetricsHandler.h"
1415
#include "admin/StateHandler.h"
1516
#include "admin/TrackMetricsHandler.h"
@@ -204,6 +205,7 @@ int main(int argc, char* argv[]) {
204205
}
205206
admin::registerTrackMetricsRoute(adminServer, context, trackLimits);
206207
admin::registerConfigRoute(adminServer, std::make_shared<const cfg::Config>(config));
208+
admin::registerConnectionLogsRoutes(adminServer, config.logging);
207209

208210
// === 8. Start serving ===
209211
for (auto& server : servers) {

test/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,10 @@ add_test(
348348
NAME admin_config_endpoint
349349
COMMAND bash ${PROJECT_SOURCE_DIR}/test/test_admin_config.sh $<TARGET_FILE:moqx>
350350
)
351+
add_test(
352+
NAME admin_connection_logs_endpoint
353+
COMMAND bash ${PROJECT_SOURCE_DIR}/test/test_admin_connection_logs.sh $<TARGET_FILE:moqx>
354+
)
351355
add_test(
352356
NAME admin_cache_purge_concurrency_test
353357
COMMAND bash ${PROJECT_SOURCE_DIR}/test/test_admin_cache_purge_race.sh $<TARGET_FILE:moqx>

0 commit comments

Comments
 (0)