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

Update createMetricSearch type to allow optional dimensions map #599

Open
iamgerg opened this issue Nov 14, 2024 · 0 comments
Open

Update createMetricSearch type to allow optional dimensions map #599

iamgerg opened this issue Nov 14, 2024 · 0 comments
Labels
feature-request New feature

Comments

@iamgerg
Copy link

iamgerg commented Nov 14, 2024

Feature scope

MetricFactory

Describe your suggested feature

The MetricFactory#createMetricSearch method explicitly calls out being able to pass in an object where the values are undefined for the dimensionsMap argument so that searches are expanded to "any value" for that dimension. However the call signature uses the DimensionsMap type from aws-cloudwatch which marks all values as required forcing users to type their values as undefined as unknown as string. This is cumbersome and not quite clear for users unfamiliar with this intended behavior.

The workaround is even documented in the doc comment for the parameter itself:

* @param dimensionsMap dimensions, further narrowing the search results; use `undefined` if you want to represent "any value" (in TS: `undefined as any as string`)

More specifically, I have a use case where I compose my queries using an object that has optional values that I perform some transformations on, but in order to appease the typings I must typecast all undefineds when I do so.

Example

const dimensions: cw.DimensionsMap = Object.fromEntries(
  Object.entries(props.dimensions ?? {}).map(([name, value]) => [
    name,
    value?.equals || undefined as unknown as string, // <- weird that I have to cast this, but I must use `undefined` so the values get filtered out in the query
  ]),
);

factory.createMetricSearch(`MetricName=SomeMetric`, dimensions, MetricStatistic.AVERAGE);

Ideal

const dimensions: cw.DimensionsMap = Object.fromEntries(
  Object.entries(props.dimensions ?? {}).map(([name, value]) => [
    name,
    value?.equals || undefined,
  ]),
);

factory.createMetricSearch(`MetricName=SomeMetric`, dimensions, MetricStatistic.AVERAGE);

Proposal

I think instead of using the cloudwatch DimensionsMap this library can expose a new type like OptionalDimensionsMap, that can be used for the createMetricSearch and possibly createMetric methods. Something along the lines of:

// interface and explicit undefined should be jsii compatible
interface OptionalDimensionsMap {
  [key: string]: string | undefined
}

I'd be happy to submit a PR if this is something you'd like to add.

@iamgerg iamgerg added the feature-request New feature label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature
Projects
None yet
Development

No branches or pull requests

1 participant