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

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

Open
PatrykMilewski opened this issue Mar 2, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@PatrykMilewski
Copy link

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

@PatrykMilewski PatrykMilewski added the bug Something isn't working label Mar 2, 2023
@bestickley
Copy link

@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
Copy link
Author

From my experience they are created but not returned

@echeung-amzn
Copy link
Member

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
Copy link

JCKortlang commented Feb 1, 2024

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());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants