Bug: Unexpected behavior of Metrics.addDimensions function

[Akshay314]

Expected Behavior

Hi,

The typescript implementation of addDimensions diverge from python implementation where in former, you are creating a new dimensionSet this.#dimensionsStore.addDimensionSet(newDimensions); while in python, it appends to dimensions to existing set.

Also, the method documentation does not specify this clearly. Ideally as per name suggests, it should append new dimensions to existing dimensionSet.

Current Behavior

AddDimensions function is creating a new dimension set for given set of metrics instead of appending to existing set.

Code snippet

public addDimensions(dimensions: Dimensions): void {
  const newDimensions = this.#sanitizeDimensions(dimensions);
  const currentCount = this.#dimensionsStore.getDimensionCount();
  const newSetCount = Object.keys(newDimensions).length;

  if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) {
    throw new RangeError(
      `The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
    );
  }

  this.#dimensionsStore.addDimensionSet(newDimensions);
}

Steps to Reproduce

Call addDimensions twice with different set of key value pairs before publishing metrics. It would result in two different dimension sets.

Possible Solution

Loop over given dimensions and add each pair to existing set.
newDimensions.forEach((k,v) -> this.#dimensionsStore.addDimension(name, value);)

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

22.x

Packaging format used

npm

Execution logs

[boring-cyborg]

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

[dreamorosi]

Hi @Akshay314, thank you for opening this issue.

We've made changes to the internal implementation of the method in two occasions in the past few months with the two most significant changes being in #4181 and #4663, however I am unsure whether these had any impact on this.

Another workstream that might have had an impact on this is #3777, specifically the discussion involved all runtimes and I am unsure whether the discrepancy between Python & TypeScript is intentional or not.

With that said, I'll assign the issue to @sdangol so he can take a look at the discussions and implementations and assess.

We'll report back as soon as we have more info.

[sdangol]

Hi @Akshay314, I've looked into it and the addDimensions method is supposed to create a new dimension set. The implementation in Python doesn't have the concept for creating new arrays with dimensions.

If you're looking to append to an existing dimensionSet, you should use the singular addDimension method instead

[dreamorosi]

I agree that the discrepancy is kind of confusing, but I don't know if it's intended - @leandrodamascena what do you think?

[phipag]

I remember we had a discussion here aws-powertools/powertools-lambda-python#6198

Based on this issue (please correct me if I am wrong) it seems that the behavior in Python should be adapted to TypeScript: Do not append to the dimension set.

[Akshay314]

Hi folks, thanks a lot for prompt replies. Here are my two suggestions if it make sense:

1. There is no harm in keeping both methods, to append dimensions, and to create a new dimensionSet. If its feasible to support both that would be ideal.
2. Whether you agree with first proposal or not, the method name/documentation of addDimensions should be updated to clarify this behavior. I had no way to know about it unless i read the code myself. Even chatGPT hallucinated while answering this.

You can see the struggle every dev have to do because this limitation:

Object.entries(dimensions).forEach(([key, value]) => metrics.addDimension(key, value))