fix(metrics): detect key collisions between dimensions, metrics, and metadata

[Zelys-DFKH]

Summary

Changes

Closes #5208. In serializeMetrics(), EMF output is assembled by spreading default dimensions, dimensions, dimension sets, metric values, and metadata in that order. When keys match across those layers, later values win silently. Two of those collisions break the EMF schema:

• A metric name matching a dimension key replaces the string value with a number, so CloudWatch drops the dimension.
• A metadata key matching a metric name replaces the number with a string, so CloudWatch fails to parse the metric.

What changed

• Metrics.ts: Added a guard in storeMetric() that throws when the metric name matches any current dimension key (flat dimensions, default dimensions, or dimension sets), and a guard in addMetadata() that throws when the metadata key matches any current metric name. Both error messages include the key name.
• dimensions.test.ts: Four new tests covering a regular dimension conflict, a default dimension conflict, a dimension-set conflict (via addDimensions), and the built-in service dimension.
• metadata.test.ts: One new test covering a metadata-metric name conflict.

Issue number: Closes #5208

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[svozza]

Thanks for tackling this! Two pieces of feedback:

1. Only throw on type changes

Same-type collisions (dimension ↔ metadata, both strings) overwrite silently but don't corrupt the EMF schema — CloudWatch only rejects when a key flips between string and number. Throwing on same-type clashes is stricter than the spec requires and inconsistent with addDimension, which already handles dimension-on-dimension overwrites with a warn. I'd keep the throw for type changes only and leave same-type collisions on the existing warn path.

2. Centralise the check in serializeMetrics()

The current guards in storeMetric() and addMetadata() only catch two of the four type-change directions. They miss:

• addDimension/addDimensions/setDefaultDimensions after addMetric (num → str)
• addMetric after addMetadata (str → num)

Reverse the call order in either of the existing tests and the corruption comes back. Rather than adding guards to every mutator, serializeMetrics() already has all the sources in scope right before the spread that causes the overwrite — one pass there catches every combination regardless of call order:

// inside serializeMetrics(), just before the return
type Source = 'dimension' | 'default dimension' | 'dimension set' | 'metadata';
const stringKeys = new Map<string, Source>();
for (const k of Object.keys(defaultDimensions)) stringKeys.set(k, 'default dimension');
for (const k of Object.keys(dimensions)) stringKeys.set(k, 'dimension');
for (const set of dimensionSets) for (const k of Object.keys(set)) stringKeys.set(k, 'dimension set');
for (const k of Object.keys(this.#metadataStore.getAll())) stringKeys.set(k, 'metadata');

for (const name of Object.keys(metricValues)) {
  const source = stringKeys.get(name);
  if (source) {
    throw new Error(
      `EMF key collision on "${name}": registered as both a metric (number) and a ${source} (string)`
    );
  }
}