cloudwatch: make metricName field public

[rantoniuk]

Describe the feature

In the following code:

    const sigkillMetricFilter = new MetricFilter(this, 'SigkillMetricFilter', {
      filterPattern: FilterPattern.literal('SIGKILL'),
      logGroup: this.logGroup,
      metricNamespace,
      metricName: 'SigkillCount',
      metricValue: '1',
    });

    // Create a metric using the filter
    const sigkillMetric = new Metric({
      namespace: metricNamespace,
      metricName: 'SigkillCount',
      statistic: 'Sum',
      period: Duration.minutes(5),
    });

the metricName field is duplicated in two constructs, because it's not possible to do:

    const sigkillMetric = new Metric({
      namespace: metricNamespace,
      metricName: sigkillMetricFilter.metricName, // this field is private
      statistic: 'Sum',
      period: Duration.minutes(5),
    });

This doesn't allow for proper object dependency/relationship.

Use Case

As above.

Proposed Solution

Make the field public.

Other Information

No response

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

CDK version used

[email protected]

Environment details (OS name and version, etc.)

MacOS

[ashishdhingra]

@rantoniuk Good morning. Thanks for opening the issue. Although it's debatable, shouldn't the above code written like this:

const sigkillMetric = new Metric({
      namespace: metricNamespace,
      metricName: SigkillCount,
      statistic: 'Sum',
      period: Duration.minutes(5),
    });

const sigkillMetricFilter = new MetricFilter(this, 'SigkillMetricFilter', {
      filterPattern: FilterPattern.literal('SIGKILL'),
      logGroup: this.logGroup,
      metricNamespace,
      metricName: sigkillMetric.metricName,
      metricValue: '1',
    });

In other words, we should define metric first and then use it's object to reference meticName property.

Thanks,
Ashish

[rantoniuk]

Yes, that's perfectly fine, thanks for pointing that out!

However, I think I'm missing something here. See the below code:

 const sigkillMetric = new Metric({
      namespace: 'reportgen',
      metricName: 'SigkillCount',
      statistic: 'Sum',
      period: Duration.minutes(5),
    });

    const sigkillMetricFilter = new MetricFilter(this, 'SigkillMetricFilter', {
      filterPattern: FilterPattern.literal('SIGKILL'),
      logGroup: this.logGroup,
      metricNamespace: sigkillMetric.namespace,
      metricName: sigkillMetric.metricName,
      metricValue: '1',
    });

    const sigkillAlarm = new Alarm(this, 'SigkillAlarm', {
      alarmName: 'SIGKILL-Alarm',
      metric: sigkillMetric,
      threshold: 1,
      evaluationPeriods: 1,
      comparisonOperator: ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
      treatMissingData: TreatMissingData.NOT_BREACHING,
    });

Now, the above code does what I want it to do, i.e. sets up alarm for SUM, 5min. However, you can notice that the alarm refers directly to the metric and not the metric filter. This is fine, I'm just curious why the below doesn't do the same:

 const sigkillAlarm = new Alarm(this, 'SigkillAlarm', {
      alarmName: 'SIGKILL-Alarm',
      metric: sigkillMetricFilter.metric(), // CHANGE
      threshold: 1,
      evaluationPeriods: 1,
      comparisonOperator: ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
      treatMissingData: TreatMissingData.NOT_BREACHING,
    });

When I do this, cdk diff shows:

[~] AWS::CloudWatch::Alarm SigkillAlarm SigkillAlarmF9E70DD1
 └─ [~] Statistic
     ├─ [-] Sum
     └─ [+] Average

Is that expected? it should return a ref to the metric object that has sum defined, right?

[ashishdhingra]

@rantoniuk Good morning. Could you please share self-contained code to troubleshoot the issue? Also share the output of cdk synth when using Metric and MetricFilter respectively?

Thanks,
Ashish

[rantoniuk]

You can find the code already above. As requested, I did a template diff between

  const sigkillAlarm = new Alarm(this, 'SigkillAlarm', {
      alarmName: 'SIGKILL-Alarm',
      metric: sigkillMetric,
      threshold: 1,
      evaluationPeriods: 1,
      comparisonOperator: ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
      treatMissingData: TreatMissingData.NOT_BREACHING,
    });

and

    const sigkillAlarm = new Alarm(this, 'SigkillAlarm', {
      alarmName: 'ReportGen-SIGKILL-Alarm',
      metric: sigkillMetricFilter.metric(),
      threshold: 1,
      evaluationPeriods: 1,
      comparisonOperator: ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
      treatMissingData: TreatMissingData.NOT_BREACHING,
    });

Result:

diff no-ref.json with-ref.json 
367c367
<     "Statistic": "Sum",
---
>     "Statistic": "Average",

I think the problem (CDK-wise) is that the MetricFilter does not have an object reference to Metric - instead it only relies on the properties:

    const sigkillMetricFilter = new MetricFilter(this, 'SigkillMetricFilter', {
      filterPattern: FilterPattern.literal('SIGKILL'),
      logGroup: this.logGroup,
      metricNamespace: sigkillMetric.namespace, <-----
      metricName: sigkillMetric.metricName, <----
      metricValue: '1',
    });