cdk-monitoring-constructs icon indicating copy to clipboard operation
cdk-monitoring-constructs copied to clipboard

Update createMetricSearch type to allow optional dimensions map

Open iamgerg opened this issue 11 months ago • 1 comments

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:

https://github.com/cdklabs/cdk-monitoring-constructs/blob/1f8ca082a122af8775f01be2d30c92c3de1344a1/lib/common/metric/MetricFactory.ts#L131

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 avatar Nov 14 '24 15:11 iamgerg