Skip to content

Commit 34972b9

Browse files
committed
agents: fix statsdTags in reconfigure response
The gRPC reconfigure response dropped `statsdTags` because `PopulateReconfigureEvent()` looked up the wrong config key. Harden the gRPC reconfigure test to assert response fields directly and to fail when expected config keys are missing from the response body.` Signed-off-by: Santiago Gimeno <santiago.gimeno@gmail.com> PR-URL: #495 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent a0aee02 commit 34972b9

2 files changed

Lines changed: 35 additions & 6 deletions

File tree

agents/grpc/src/grpc_agent.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ void PopulateReconfigureEvent(grpcagent::ReconfigureEvent* reconfigure_event,
388388
if (it != config.end()) {
389389
body->set_statsdbucket(it->get<std::string>());
390390
}
391-
it = config.find("statsdtags");
391+
it = config.find("statsdTags");
392392
if (it != config.end()) {
393393
body->set_statsdtags(it->get<std::string>());
394394
}

test/agents/test-grpc-reconfigure.mjs

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,38 @@ function checkReconfigureData(reconfigure, metadata, requestId, agentId, nsolidC
3232
// Normalize the configuration objects for comparison
3333
const normalizedReconfigBody = {};
3434
const normalizedNsolidConfig = {};
35+
const configKeys = [
36+
'blockedLoopThreshold',
37+
'interval',
38+
'pauseMetrics',
39+
'promiseTracking',
40+
'redactSnapshots',
41+
'statsd',
42+
'statsdBucket',
43+
'statsdTags',
44+
'tags',
45+
'tracingEnabled',
46+
'tracingModulesBlacklist',
47+
'contCpuProfile',
48+
'assetsEnabled',
49+
'traceSampleRate',
50+
];
3551

3652
// Process reconfigure.body - remove properties starting with underscore
3753
for (const [key, value] of Object.entries(reconfigure.body)) {
3854
if (!key.startsWith('_')) {
39-
// Convert string numbers to actual numbers for comparison
40-
if (typeof value === 'string' && !Number.isNaN(Number(value))) {
55+
const expectedValue = nsolidConfig[key];
56+
if (typeof expectedValue === 'number' && typeof value === 'string') {
4157
normalizedReconfigBody[key] = Number(value);
4258
} else {
4359
normalizedReconfigBody[key] = value;
4460
}
4561
}
4662
}
4763

48-
// Process nsolidConfig - include only keys that exist in normalizedReconfigBody
49-
for (const key of Object.keys(normalizedReconfigBody)) {
64+
// Compare against the expected reconfigure keys, not only the keys already
65+
// present in the response, so missing fields fail the test.
66+
for (const key of configKeys) {
5067
if (key in nsolidConfig) {
5168
normalizedNsolidConfig[key] = nsolidConfig[key];
5269
}
@@ -148,7 +165,19 @@ tests.push({
148165
checkReconfigureData(data.msg, data.metadata, requestId, agentId, nsolidConfig);
149166

150167
// Verify that the specific field has been updated correctly
151-
console.log(`Checking if ${key} was updated to ${val}`);
168+
console.log('Checking if ' + key + ' was updated to ' + val);
169+
170+
const bodyVal = data.msg.body[key];
171+
const normalizedBodyVal =
172+
typeof val === 'number' && typeof bodyVal === 'string' ?
173+
Number(bodyVal) :
174+
bodyVal;
175+
176+
assert.deepStrictEqual(
177+
normalizedBodyVal,
178+
val,
179+
'Expected reconfigure body ' + key + ' to be ' + val + ', but got ' + bodyVal,
180+
);
152181

153182
// Compare the values
154183
assert.deepStrictEqual(

0 commit comments

Comments
 (0)