Skip to content

Commit 413f71d

Browse files
trondntrondn
committed
MB-35930: Add support for providing stat args
Allow the STAT call to contain a value containing a JSON payload containing "arguments and options" to the given STAT call. It is up to the stat call to define the schema for the JSON provided to each stat group. Change-Id: I8e88cddbcb8e7c9ceea22e368f5cc71238618f20 Reviewed-on: http://review.couchbase.org/114585 Tested-by: Build Bot <[email protected]> Reviewed-by: Jim Walker <[email protected]>
1 parent 3a6d750 commit 413f71d

27 files changed

+177
-102
lines changed

‎daemon/mcbp_validators.cc‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,12 +1190,36 @@ static Status delete_validator(Cookie& cookie) {
11901190
}
11911191

11921192
static Status stat_validator(Cookie& cookie) {
1193-
return McbpValidator::verify_header(cookie,
1194-
0,
1195-
ExpectedKeyLen::Any,
1196-
ExpectedValueLen::Zero,
1197-
ExpectedCas::NotSet,
1198-
PROTOCOL_BINARY_RAW_BYTES);
1193+
auto ret = McbpValidator::verify_header(
1194+
cookie,
1195+
0,
1196+
ExpectedKeyLen::Any,
1197+
ExpectedValueLen::Any,
1198+
ExpectedCas::NotSet,
1199+
PROTOCOL_BINARY_RAW_BYTES | PROTOCOL_BINARY_DATATYPE_JSON);
1200+
if (ret != Status::Success) {
1201+
return ret;
1202+
}
1203+
1204+
const auto& req = cookie.getRequest(Cookie::PacketContent::Full);
1205+
auto value = req.getValue();
1206+
if (!value.empty()) {
1207+
// The value must be JSON
1208+
if ((uint8_t(req.getDatatype()) & PROTOCOL_BINARY_DATATYPE_JSON) == 0) {
1209+
cookie.setErrorContext("Datatype must be JSON");
1210+
return Status::Einval;
1211+
}
1212+
1213+
// Validate that the value is JSON
1214+
try {
1215+
nlohmann::json::parse(value);
1216+
} catch (const std::exception&) {
1217+
cookie.setErrorContext("value is not valid JSON");
1218+
return Status::Einval;
1219+
}
1220+
}
1221+
1222+
return Status::Success;
11991223
}
12001224

12011225
static Status arithmetic_validator(Cookie& cookie) {

‎daemon/protocol/mcbp/engine_wrapper.cc‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,9 +337,14 @@ ENGINE_ERROR_CODE bucket_flush(Cookie& cookie) {
337337

338338
ENGINE_ERROR_CODE bucket_get_stats(Cookie& cookie,
339339
cb::const_char_buffer key,
340+
cb::const_byte_buffer value,
340341
const AddStatFn& add_stat) {
341342
auto& c = cookie.getConnection();
342-
auto ret = c.getBucketEngine()->get_stats(&cookie, key, add_stat);
343+
auto ret = c.getBucketEngine()->get_stats(
344+
&cookie,
345+
key,
346+
{reinterpret_cast<const char*>(value.data()), value.size()},
347+
add_stat);
343348
if (ret == ENGINE_DISCONNECT) {
344349
LOG_WARNING("{}: {} bucket_get_stats return ENGINE_DISCONNECT",
345350
c.getId(),

‎daemon/protocol/mcbp/engine_wrapper.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ ENGINE_ERROR_CODE bucket_flush(Cookie& cookie);
127127

128128
ENGINE_ERROR_CODE bucket_get_stats(Cookie& cookie,
129129
cb::const_char_buffer key,
130+
cb::const_byte_buffer value,
130131
const AddStatFn& add_stat);
131132

132133
/**

‎daemon/protocol/mcbp/stats_context.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,8 @@ static ENGINE_ERROR_CODE stat_tracing_executor(const std::string& arg,
701701

702702
static ENGINE_ERROR_CODE stat_all_stats(const std::string& arg,
703703
Cookie& cookie) {
704-
auto ret = bucket_get_stats(cookie, arg, appendStatsFn);
704+
auto value = cookie.getRequest().getValue();
705+
auto ret = bucket_get_stats(cookie, arg, value, appendStatsFn);
705706
if (ret == ENGINE_SUCCESS) {
706707
ret = server_stats(appendStatsFn, cookie);
707708
}
@@ -710,7 +711,8 @@ static ENGINE_ERROR_CODE stat_all_stats(const std::string& arg,
710711

711712
static ENGINE_ERROR_CODE stat_bucket_stats(const std::string& arg,
712713
Cookie& cookie) {
713-
return bucket_get_stats(cookie, arg, appendStatsFn);
714+
auto value = cookie.getRequest().getValue();
715+
return bucket_get_stats(cookie, arg, value, appendStatsFn);
714716
}
715717

716718
/***************************** STAT HANDLERS *****************************/

‎docs/BinaryProtocol.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1586,7 +1586,7 @@ Request:
15861586

15871587
* MUST NOT have extras.
15881588
* MAY have key.
1589-
* MUST NOT have value.
1589+
* MAY have value.
15901590

15911591
Response:
15921592

@@ -1601,6 +1601,12 @@ statistical item and the body contains the value in ASCII format). The
16011601
sequence of return packets is terminated with a packet that contains no key
16021602
and no value.
16031603

1604+
If a value is present it must be a JSON payload (and the JSON datatype set)
1605+
which adds additional parameters and arguments to the stats call. It is up
1606+
to each stat subgroup to define the schema. As long as the provided payload
1607+
is _valid json_, the server will silently (from the clients perspective)
1608+
ignore unknown elements in the provided JSON.
1609+
16041610
#### Example
16051611

16061612
The following example requests all statistics from the server

‎engines/crash_engine/crash_engine.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ class CrashEngine : public EngineIface {
110110

111111
ENGINE_ERROR_CODE get_stats(gsl::not_null<const void*> cookie,
112112
cb::const_char_buffer key,
113+
cb::const_char_buffer value,
113114
const AddStatFn& add_stat) override;
114115

115116
void reset_stats(gsl::not_null<const void*> cookie) override;
@@ -264,9 +265,10 @@ ENGINE_ERROR_CODE CrashEngine::unlock(gsl::not_null<const void*> cookie,
264265
return ENGINE_FAILED;
265266
}
266267

267-
ENGINE_ERROR_CODE CrashEngine::get_stats(gsl::not_null<const void*> cookie,
268-
cb::const_char_buffer key,
269-
const AddStatFn& add_stat) {
268+
ENGINE_ERROR_CODE CrashEngine::get_stats(gsl::not_null<const void*>,
269+
cb::const_char_buffer,
270+
cb::const_char_buffer,
271+
const AddStatFn&) {
270272
return ENGINE_FAILED;
271273
}
272274

‎engines/default_engine/default_engine.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ ENGINE_ERROR_CODE default_engine::unlock(gsl::not_null<const void*> cookie,
454454

455455
ENGINE_ERROR_CODE default_engine::get_stats(gsl::not_null<const void*> cookie,
456456
cb::const_char_buffer key,
457+
cb::const_char_buffer value,
457458
const AddStatFn& add_stat) {
458459
ENGINE_ERROR_CODE ret = ENGINE_SUCCESS;
459460

‎engines/default_engine/default_engine_internal.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ struct default_engine : public EngineIface {
171171

172172
ENGINE_ERROR_CODE get_stats(gsl::not_null<const void*> cookie,
173173
cb::const_char_buffer key,
174+
cb::const_char_buffer value,
174175
const AddStatFn& add_stat) override;
175176

176177
void reset_stats(gsl::not_null<const void*> cookie) override;

‎engines/ep/src/ep_engine.cc‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ auto makeExitBorderGuard = [](auto&& wrapped) {
357357
ENGINE_ERROR_CODE EventuallyPersistentEngine::get_stats(
358358
gsl::not_null<const void*> cookie,
359359
cb::const_char_buffer key,
360+
cb::const_char_buffer value,
360361
const AddStatFn& add_stat) {
361362
// The AddStatFn callback may allocate memory (temporary buffers for
362363
// stat data) which will be de-allocated inside the server, after the
@@ -367,10 +368,8 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::get_stats(
367368
// the input add_stat function.
368369
auto addStatExitBorderGuard = makeExitBorderGuard(std::cref(add_stat));
369370

370-
return acquireEngine(this)->getStats(cookie,
371-
key.data(),
372-
gsl::narrow_cast<int>(key.size()),
373-
addStatExitBorderGuard);
371+
return acquireEngine(this)->getStats(
372+
cookie, key, value, addStatExitBorderGuard);
374373
}
375374

376375
ENGINE_ERROR_CODE EventuallyPersistentEngine::store(
@@ -4178,13 +4177,18 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::doScopeStats(
41784177

41794178
ENGINE_ERROR_CODE EventuallyPersistentEngine::getStats(
41804179
const void* cookie,
4181-
const char* stat_key,
4182-
int nkey,
4180+
cb::const_char_buffer request_stat_key,
4181+
cb::const_char_buffer value,
41834182
const AddStatFn& add_stat) {
41844183
ScopeTimer2<HdrMicroSecStopwatch, TracerStopwatch> timer(
41854184
HdrMicroSecStopwatch(stats.getStatsCmdHisto),
41864185
TracerStopwatch(cookie, cb::tracing::TraceCode::GETSTATS));
41874186

4187+
// @todo cleanup this method to avoid copying the data to a new
4188+
// buffer, and start adding support for the provided json value
4189+
auto* stat_key = request_stat_key.data();
4190+
int nkey = gsl::narrow_cast<int>(request_stat_key.size());
4191+
41884192
const std::string statKey(stat_key, nkey);
41894193

41904194
if (statKey.size()) {

‎engines/ep/src/ep_engine.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
181181

182182
ENGINE_ERROR_CODE get_stats(gsl::not_null<const void*> cookie,
183183
cb::const_char_buffer key,
184+
cb::const_char_buffer value,
184185
const AddStatFn& add_stat) override;
185186

186187
void reset_stats(gsl::not_null<const void*> cookie) override;
@@ -473,8 +474,8 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
473474
}
474475

475476
ENGINE_ERROR_CODE getStats(const void* cookie,
476-
const char* stat_key,
477-
int nkey,
477+
cb::const_char_buffer stat_key,
478+
cb::const_char_buffer value,
478479
const AddStatFn& add_stat);
479480

480481
void resetStats();

0 commit comments

Comments
 (0)