Skip to content

Commit b9e97a5

Browse files
John Huggmeta-codesync[bot]
authored andcommitted
Always echo Artillery trace context on memcache-protocol replies
Summary: UCache's memcache-protocol requests show up as INFERRED_BLOCKs in Artillery traces because the server never echoes trace context back — the echo was gated behind the expensive logging path (cost tracking + scribe flush) that UCache may not want to enable for cost reasons. This adds a lightweight echo-only mode that returns trace context on replies without activating the logging path. A new gflag (`asmcs_trace_always_echo_context`) controls it, bridged into UCache via a live-observed folly setting for runtime kill-switch capability. Defaults off; existing behavior unchanged. Questions for reviewer: Is this the best way to gate something like this? Is there a different preferred way? Reviewed By: ghostonhuang Differential Revision: D112132643 fbshipit-source-id: 7ef698073c92356bfa274118a9e6339b6b4eaad8
1 parent be45cfa commit b9e97a5

3 files changed

Lines changed: 48 additions & 14 deletions

File tree

mcrouter/lib/network/CarbonRequestHandler.h

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,14 @@ class CarbonRequestHandler : public facebook::memcache::CarbonMessageDispatcher<
7272
const facebook::memcache::CaretMessageInfo* headerInfo,
7373
const folly::IOBuf* reqBuf,
7474
std::true_type) {
75-
if (FOLLY_UNLIKELY(
76-
!req.traceContext().empty() &&
77-
facebook::mcrouter::traceCheckRateLimit())) {
78-
onRequestImplWithTracingEnabled(
79-
std::move(ctx), std::move(req), headerInfo, reqBuf);
80-
return;
75+
if (FOLLY_UNLIKELY(!req.traceContext().empty())) {
76+
const bool loggingEnabled = facebook::mcrouter::traceCheckRateLimit();
77+
if (loggingEnabled ||
78+
facebook::mcrouter::traceShouldAlwaysEchoContext()) {
79+
onRequestImplWithTracingEnabled(
80+
std::move(ctx), std::move(req), headerInfo, reqBuf, loggingEnabled);
81+
return;
82+
}
8183
}
8284
callOnRequest(
8385
std::move(ctx),
@@ -104,17 +106,29 @@ class CarbonRequestHandler : public facebook::memcache::CarbonMessageDispatcher<
104106
facebook::memcache::McServerRequestContext&& ctx,
105107
Request&& req,
106108
const facebook::memcache::CaretMessageInfo* headerInfo,
107-
const folly::IOBuf* reqBuf) {
109+
const folly::IOBuf* reqBuf,
110+
[[maybe_unused]] bool loggingEnabled) {
108111
#ifndef LIBMC_FBTRACE_DISABLE
109112
folly::RequestContextScopeGuard requestContextGuard;
110-
auto tracingData = facebook::mcrouter::traceRequestReceived(
111-
req.traceContext(), Request::name);
112-
if (tracingData != nullptr) {
113-
// Mark the context as being traced by Artillery
113+
if (loggingEnabled) {
114+
auto tracingData = facebook::mcrouter::traceRequestReceived(
115+
req.traceContext(), Request::name);
116+
if (tracingData != nullptr) {
117+
// Mark the context as being traced by Artillery
118+
markContextAsTraced(ctx);
119+
facebook::mcrouter::extractAndSetCallerIdentities(
120+
*tracingData, ctx.getThriftRequestContext());
121+
tracingData->startCounters();
122+
}
123+
} else if (facebook::mcrouter::traceEchoOnlyRequestReceived(
124+
req.traceContext())) {
125+
// Echo-only: the reply will carry the received trace context back, but
126+
// no cost tracking or scribe flush happens. markContextAsTraced only
127+
// sets ctx.isTraced_, whose sole reader is replyImpl2 -- it stamps the
128+
// echoed context onto the reply via getCurrentTracer()->sendResponse()
129+
// (the tracer set up by traceEchoOnlyRequestReceived above). It has no
130+
// other side effects: no counters, logging, or scribe I/O.
114131
markContextAsTraced(ctx);
115-
facebook::mcrouter::extractAndSetCallerIdentities(
116-
*tracingData, ctx.getThriftRequestContext());
117-
tracingData->startCounters();
118132
}
119133
#endif
120134
callOnRequest(

mcrouter/lib/network/FBTrace-inl.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ inline bool traceCheckRateLimit() {
2626
return false;
2727
}
2828

29+
inline bool traceShouldAlwaysEchoContext() {
30+
return false;
31+
}
32+
2933
inline uint64_t traceGetCount() {
3034
return 0;
3135
}
@@ -37,6 +41,11 @@ inline std::nullptr_t traceRequestReceived(
3741
return nullptr;
3842
}
3943

44+
inline bool traceEchoOnlyRequestReceived(const std::string& traceContext) {
45+
// Do nothing by default.
46+
return false;
47+
}
48+
4049
#else
4150

4251
// Fwd declaration
@@ -48,6 +57,13 @@ std::shared_ptr<TracingData> traceRequestReceived(
4857
const std::string& traceContext,
4958
folly::StringPiece requestType);
5059

60+
// Set up a tracer for reply-side echo only, without the heavyweight logging
61+
// path (no cost counters, no scribe flush). Stores the tracer under the
62+
// response-context key so getCurrentTracer()/replyImpl echoes it back. Returns
63+
// true if a tracer was set up.
64+
// NOTE: this function does not exist if LIBMC_FBTRACE_DISABLE is defined.
65+
bool traceEchoOnlyRequestReceived(const std::string& traceContext);
66+
5167
#endif
5268

5369
} // namespace mcrouter

mcrouter/lib/network/FBTrace.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ namespace mcrouter {
1919
// Returns true if a rate limiting check passes and tracing can proceed.
2020
bool traceCheckRateLimit();
2121

22+
// Returns true if received trace context should always be echoed back on the
23+
// reply, independent of the rate limit (which gates only the logging path).
24+
bool traceShouldAlwaysEchoContext();
25+
2226
// Returns the cumulative number of traces logged.
2327
uint64_t traceGetCount();
2428

0 commit comments

Comments
 (0)