Skip to content

fix(metrics): addDimensions() documentation and tests #3964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

matteofigus
Copy link
Member

@matteofigus matteofigus commented May 22, 2025

Fix: addDimensions() should create a set

Summary

Changes

This PR fixes issue #3777 by modifying the addDimensions() method to create a new dimension set rather than concatenating dimensions into the existing set. The implementation follows the EMF specification which supports multiple dimension sets within the Dimensions array.

Issue number: #3777

Description of changes

Key changes:

  • Modified addDimensions() to create a new dimension set that includes default dimensions
  • Added an alias method addDimensionSet() for consistency with other Powertools for AWS Lambda implementations
  • Updated serializeMetrics() to include all dimension sets in the EMF output
  • Updated clearDimensions() to clear both individual dimensions and dimension sets
  • Updated tests to verify the new behavior
  • Added documentation for the new functionality

Example

const metrics = new Metrics({
  namespace: 'serverlessAirline',
  serviceName: 'orders',
});

// Add a single dimension
metrics.addDimension('environment', 'prod');

// Add a new dimension set
metrics.addDimensions({
  dimension1: "1",
  dimension2: "2"
});

// This will create two dimension sets in the EMF output:
// [["service", "environment"]] and [["service", "dimension1", "dimension2"]]
metrics.addMetric('successfulBooking', MetricUnit.Count, 1);
metrics.publishStoredMetrics();

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 boring-cyborg bot added documentation Improvements or additions to documentation metrics This item relates to the Metrics Utility tests PRs that add or change tests labels May 22, 2025
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label May 22, 2025
Copy link

boring-cyborg bot commented May 22, 2025

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

This comment was marked as outdated.

@github-actions github-actions bot added do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels May 22, 2025

This comment was marked as outdated.

@github-actions github-actions bot added the bug Something isn't working label May 22, 2025
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit unsure about the changes in this entire file. It seems that we re-declared all the types that were previously in the packages/metrics/src/types/Metrics.ts file for no apparent reason.

This seems to break the compilation (see failed CI) - can we revert?

@dreamorosi dreamorosi removed do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels May 22, 2025
@matteofigus
Copy link
Member Author

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.

Copy link

@dreamorosi dreamorosi linked an issue May 22, 2025 that may be closed by this pull request
@dreamorosi dreamorosi changed the title fix: addDimensions() documentation and tests fix(metrics): addDimensions() documentation and tests May 22, 2025
@matteofigus matteofigus marked this pull request as draft May 23, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation metrics This item relates to the Metrics Utility size/L PRs between 100-499 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: addDimensions() should create a set
2 participants