wu-sheng commented on PR #139:
URL:
https://github.com/apache/skywalking-nodejs/pull/139#issuecomment-4824549227
Thanks for the update. The previous single-start alias issue is fixed:
`agent.start({ nvmMetricsReporterActive: false })` now sets
`runtimeMetricsReporterActive` to `false`, and the new config test covers that.
I found one remaining stale-config edge around restart.
`normalizeDeprecatedRuntimeMetricOptions()` still returns the deprecated alias
keys when it promotes them to canonical keys. After the first start, those
alias keys stay on the singleton exported `config`, so they can override a
later canonical option during a `destroy()` / `start()` cycle.
```ts
// First start: deprecated alias is promoted, but nvmMetricsReporterActive
// remains on the singleton config.
agent.start({
nvmMetricsReporterActive: false,
});
agent.destroy();
// BUG: this canonical option should win on the second start, but the stale
// config.nvmMetricsReporterActive=false from the first start is still
present.
agent.start({
runtimeMetricsReporterActive: true,
});
```
I verified locally:
```text
first start runtimeMetricsReporterActive false stale alias false
second start runtimeMetricsReporterActive false stale alias false
```
The cause is this combination:
```ts
// normalizeDeprecatedRuntimeMetricOptions() promotes the alias...
if (reporterActive !== undefined) {
normalized.runtimeMetricsReporterActive = reporterActive;
}
// ...but keeps normalized.nvmMetricsReporterActive on the object.
Object.assign(config, normalizeDeprecatedRuntimeMetricOptions(options));
// Later finalizeConfig() sees the stale alias and overwrites the canonical
value.
if (config.nvmMetricsReporterActive !== undefined) {
config.runtimeMetricsReporterActive = config.nvmMetricsReporterActive;
}
```
Suggestion: after promoting deprecated aliases, delete the deprecated fields
from the normalized options and from the singleton config after
`applyDeprecatedRuntimeMetricConfig()` consumes them. Add a regression test for:
```ts
agent.start({ nvmMetricsReporterActive: false });
agent.destroy();
agent.start({ runtimeMetricsReporterActive: true });
expect(config.runtimeMetricsReporterActive).toBe(true);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
