Skip to content

Bug: dimension value not being validated for empty strings #3302

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

Closed
dreamorosi opened this issue Nov 8, 2024 · 3 comments · Fixed by #3310
Closed

Bug: dimension value not being validated for empty strings #3302

dreamorosi opened this issue Nov 8, 2024 · 3 comments · Fixed by #3310
Assignees
Labels
bug Something isn't working confirmed The scope is clear, ready for implementation good-first-issue Something that is suitable for those who want to start contributing help-wanted We would really appreciate some support from community for this one metrics This item relates to the Metrics Utility

Comments

@dreamorosi
Copy link
Contributor

Expected Behavior

When adding a dimension to my metrics, I should get a warning if the dimension value is invalid aka an empty string ("") or undefined/null, and the dimension should not be added to the EMF blobs emitted by the utility.

Current Behavior

Currently the utility doesn't validate the input but only enforces types. In practice this means it's unlikely customers can pass undefined or null unless they're suppressing the warning, but can pass an empty string and generate an EMF blob that will have the invalid dimension.

Code snippet

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

const metrics = new Metrics();

export const handler = async () => {
  metrics.addDimension('FunctionName', '');
  metrics.addMetric('CustomMetric', 'Count', 1);
  metrics.publishStoredMetrics();
};

Steps to Reproduce

  1. Run the code above
  2. Observe that the EMF blob emitted has an empty dimension
  3. Verify that the processed metric in CloudWatch does not include any dimensions

Possible Solution

We should: 1/ validate the input when adding a dimension, 2/ emit a warning if invalid, and 3/ don't add the dimension to the EMF blob

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

20.x

Packaging format used

npm

Execution logs

{
  "_aws": {
    "Timestamp": 1731086204245,
    "CloudWatchMetrics": [
      {
        "Namespace": "default_namespace",
        "Dimensions": [
          [
            "service",
            "FunctionName"
          ]
        ],
        "Metrics": [
          {
            "Name": "CustomMetric",
            "Unit": "Count"
          }
        ]
      }
    ]
  },
  "service": "service_undefined",
  "FunctionName": "",
  "CustomMetric": 1
}
@dreamorosi dreamorosi added bug Something isn't working confirmed The scope is clear, ready for implementation help-wanted We would really appreciate some support from community for this one metrics This item relates to the Metrics Utility labels Nov 8, 2024
@dreamorosi dreamorosi moved this from Triage to Backlog in Powertools for AWS Lambda (TypeScript) Nov 11, 2024
@dreamorosi
Copy link
Contributor Author

From the BelieveInServerless Discord server, a customer was having issues while passing a value typed as any which at runtime was a number and that caused the same behavior described above.

Another +1 for runtime validation.

@dreamorosi dreamorosi added the good-first-issue Something that is suitable for those who want to start contributing label Nov 11, 2024
@arnabrahman
Copy link
Contributor

Ok, let's do it 🙂

@GusToughSon
Copy link

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

class EnhancedMetrics extends Metrics {
addDimension(name: string, value: string | undefined | null): void {
if (!value || value.trim() === '') {
console.warn(
Invalid dimension value provided for "${name}". Skipping dimension.
);
return; // Skip adding the invalid dimension
}
super.addDimension(name, value);
}
}

const metrics = new EnhancedMetrics();

export const handler = async () => {
metrics.addDimension('FunctionName', ''); // Invalid, will be skipped
metrics.addDimension('ServiceName', 'MyService'); // Valid, will be added
metrics.addMetric('CustomMetric', 'Count', 1);
metrics.publishStoredMetrics();
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed The scope is clear, ready for implementation good-first-issue Something that is suitable for those who want to start contributing help-wanted We would really appreciate some support from community for this one metrics This item relates to the Metrics Utility
Projects
3 participants