Skip to content
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

feat(metrics): warn when overwriting dimension #3352

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

dreamorosi
Copy link
Contributor

Summary

Changes

Please provide a summary of what's being changed

When working with dimensions with Metrics, the default behavior is to overwrite existing dimensions when a new one with the same name/key is added. Customers have asked us to add a warning when this happens so that they can more easily troubleshoot issues rather than missing data.

Before

import { Metrics } from '@aws-lambda-powertools/metrics';

const metrics = new Metrics({ singleMetric: true });

metrics.addDimension('test', 'foo');
metrics.addDimension('test', 'bar'); // "bar" silently replaces "foo" as value

After

import { Metrics } from '@aws-lambda-powertools/metrics';

const metrics = new Metrics({ singleMetric: true });

metrics.addDimension('test', 'foo');
metrics.addDimension('test', 'bar'); // "bar" replaces "foo"

// warning: Dimension "test" has already been added. The previous value will be overwritten.

The PR also consolidates the implementation of addDimension() and addDimensions() with the latter now iterating through the dimensions and calling the former for each one, instead of bringing its own separate implementation. In doing so, it also fixes a bug in which the addDimensions() method didn't consider default dimensions when counting the total of allowed dimensions in EMF (30).

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: closes #3350


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.

@dreamorosi dreamorosi self-assigned this Nov 24, 2024
@dreamorosi dreamorosi requested review from a team as code owners November 24, 2024 19:27
@boring-cyborg boring-cyborg bot added metrics This item relates to the Metrics Utility tests PRs that add or change tests labels Nov 24, 2024
@pull-request-size pull-request-size bot added the size/M PR between 30-99 LOC label Nov 24, 2024
@github-actions github-actions bot added the feature PRs that introduce new features or minor changes label Nov 24, 2024
@leandrodamascena
Copy link
Contributor

I'm adding this one in Python as well. Thanks for working on this @dreamorosi

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Hi @dreamorosi! Congrats on fixing this so quickly! I'll do the same in Python!

APPROVED!

packages/metrics/src/Metrics.ts Show resolved Hide resolved
@dreamorosi dreamorosi merged commit 12f3e44 into main Nov 25, 2024
37 checks passed
@dreamorosi dreamorosi deleted the feat/metrics_warn_overwrite_dimensions branch November 25, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature PRs that introduce new features or minor changes metrics This item relates to the Metrics Utility size/M PR between 30-99 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Warn users when adding multiple values to dimension before flushing
2 participants