fix(metrics): addDimensions() documentation and tests

[matteofigus]

Fix: addDimensions() should create a set

Summary

Changes

This PR fixes issue #3777 where addDimensions() was incorrectly adding dimensions to the existing set instead of creating a new dimension set.

Changes made:

1. Fixed the addDimensions() method in Metrics.ts to create a new dimension set rather than adding to existing dimensions
2. Added a dimensionSets array to store multiple dimension sets
3. Updated the metrics serialization logic to handle multiple dimension sets
4. Added unit tests to verify the correct behavior of dimension sets
5. Updated documentation to explain the correct behavior of addDimensions()

Documentation updates:

• Added a new section in the metrics documentation explaining how to use dimension sets
• Created a new code example showing how to use addDimensions() to create multiple dimension sets
• Clarified that addDimensions() creates a new set rather than modifying existing dimensions

Issue number: #3777

---

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

[boring-cyborg]

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

[matteofigus]

Thanks for catching this! I've reverted the types/index.ts file to its original state. This was an unintended change that happened during the implementation. The PR now only contains documentation updates and test fixes.

[dreamorosi]

Hi @matteofigus, I'll pick up the review for this right after the release of Bedrock Agent Functions on Tue 06/03.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
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

[dreamorosi]

Thanks for the changes, and apologies for the delayed review. Got caught between PTO and other launches.

I've left a few comments, I think we're getting closer but we need to tweak the logic to align with the addDimension (singular) method.

I've also answered to your questions in the linked issue, I hope it'll clarify the doubts - apologies for the confusion there.

[dreamorosi]

This implementation doesn't match the addDimension method (nor the docstring above).

We are not checking if the dimension value is valid like we do in the other method, and when overwriting a dimension we don't let customers know.

Other than that I think it could work. Here's a suggestion:

const newDimensionSet: Dimensions = {};
for (const [key, value] of Object.entries(dimensions)) {
  if (!value) {
    this.#logger.warn(
      `The dimension ${key} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
    );
    continue;
  }
  if (
    Object.hasOwn(this.dimensions, key) ||
    Object.hasOwn(this.defaultDimensions, key) ||
    Object.hasOwn(newDimensionSet, key)
  ) {
    this.#logger.warn(
      `Dimension "${key}" has already been added. The previous value will be overwritten.`
    );
  }
  newDimensionSet[key] = value;
  if (MAX_DIMENSION_COUNT <= this.getCurrentDimensionsCount()) {
    throw new RangeError(
      `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
    );
  }
}
this.dimensionSets.push(dimensions);

[dreamorosi]

We should also update the this.getCurrentDimensionsCount() method so that it factors in the this.dimensionSets - even if in a set, these dimensions still count against the total 30 allowed.

[dreamorosi]

Unsure about this change, was it intentional?

[dreamorosi]

Can we put back the spread operator (...)? With current implementation we're instantiating a new object (with Object.assing()) only to immediately spread it.