cdklabs / cdk-monitoring-constructs

Easy-to-use CDK constructs for monitoring your AWS infrastructure
https://constructs.dev/packages/cdk-monitoring-constructs
Apache License 2.0
475 stars 66 forks source link

Function createdAlarms doesn't return alarms created with monitorScope #336

Open PatrykMilewski opened 1 year ago

PatrykMilewski commented 1 year ago

Version

3.0.0

Steps and/or minimal code example to reproduce

  1. Create a stack with lambda and glue resource.
  2. Setup alarms with monitoring.monitorScope(stack, { lambda: { props: <add some alarms here> }}
  3. Setup alarm manually for glue with monitoring.monitorGlueJob( <config> )
  4. Call monitoring.createdAlarms()
  5. It will return only alarms created manually with monitoring.monitorGlueJob( <config> )
  6. Check AWS console, that there are more alarms created

Expected behavior

All of created alarms should be returned

Actual behavior

It doesn't return alarms created with monitorScope function

Other details

No response

bestickley commented 1 year ago

@PatrykMilewski, I'm finding that when I call monitoring.monitorScope on a Stack, no alarms are created (and not returned) but they are added to the dashboard. Have you experience this behavior? Or is your only issue that the alarms aren't being returned? Could you provide example code?

PatrykMilewski commented 1 year ago

From my experience they are created but not returned

echeung-amzn commented 9 months ago

Looking into this, it seems to be due to the nature of Aspects running during the prepare stage, and can be reproduced in a simple test like:

test("ACM", () => {
  // GIVEN
  const stack = new Stack();
  const facade = createDummyMonitoringFacade(stack);

  new acm.Certificate(stack, "DummyCertificate", {
    domainName: "www.monitoring.cdk",
  });

  // WHEN
  facade.monitorScope(stack, {
    ...defaultAspectProps,
    acm: {
      props: {
        addDaysToExpiryAlarm: {
          Warning: {
            minDaysToExpiry: 14,
          },
        },
      },
    },
  });

  // THEN
  expect(Template.fromStack(stack)).toMatchSnapshot();

  // If this happens before the previous line that synths it, it fails
  expect(facade.createdAlarms()).toHaveLength(1); 
});

I'm not actually sure if there's a great way to handle that. @ayush987goyal Not sure if you have any ideas?

JCKortlang commented 9 months ago

I will say this absence of functionality has been very frustrating. I felt like I was going insane. The AlarmActionStrategy pattern appears to be useless as far as I can tell.

My work around is to use the same Aspect API to add the AlarmActions.

//Workaround to add Alarm actions generated by .monitorScope(...)
public class AlarmActionAspect : DeputyBase, IAspect
{
    public void Visit(IConstruct node)
    {
        if (node is AlarmBase alarmBase)
        {
            alarmBase.AddAlarmAction(...); //Your alarm action
        }
    }
}

//Assuming a nested stack scope
var monitoringNestedStack = ...

//Configure Facade
monitoringNestedStack.monitoringFacade
  .monitorScope(this, ....);

//Use aspect to add alarm actions
Aspects.Of(monitoringNestedStack).Add(new AlarmActionAspect());