Skip to content

Conversation

@DerDackel
Copy link
Contributor

@DerDackel DerDackel commented Dec 20, 2023

  • Please check if the PR fulfills these requirements
  • The commit message describes your change
  • Tests for the changes have been added if possible (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Changes are mentioned in the changelog (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Re-add the monitoring option from cdk-serverless v1. This implementation builds on the cdk-monitoring-constructs library in hopes of keeping implementation effort for anything monitorting- and alerting-related low.

  • What is the current behavior? (You can also link to an open issue here)

There's a monitoring flag on the cdk-serverless' API constructs, but it currently does nothing.

  • What is the new behavior (if this is a feature change)?

A Cloudwatch dashboard with monitoring widgets for each API construct, including its endpoint Lambdas will be created if the monitoring flag is activated.

  • Does this PR introduce a breaking change? (What changes might users need to make in their setup due to this PR?)

As cdk-monitoring-constructs currently requires at least aws-cdk v2.112.0, this will raise the minimum required CDK version on cdk-serverless.

  • Other information:

Currently a rough draft, just to get something started as a discussion point on where to go. Not 100% sure about the usefulness of cdk-monitoring-constructs, I'm planning on evaluating it within the scope of this PR or return to a custom implementation if it proves not to be a good fit.
At the very least, I'd like to see the old Cloudwatch dashboard reconstructed. If possible, I want this to provide a base for an easy alerting abstraction over the API constructs, using Cloudwatch Alarms.

@DerDackel
Copy link
Contributor Author

WIP screenshot of the first draft dashboard: First Draft Rest API Dashboard

@hoegertn
Copy link
Contributor

Hi, any timeline for this PR?

@DerDackel
Copy link
Contributor Author

Currently got a bunch of work stuff to do, so will probably return to this one by the end of next week. Do you have any input on how the dashboard should look?

@DerDackel DerDackel force-pushed the reintroduce-monitoring branch from e72aa62 to 373de6c Compare February 26, 2024 06:57
@DerDackel
Copy link
Contributor Author

...that was a long week, eh? I ended up despairing over the feeling that I don't really grok cdk-monitoring-constructs. I should get back to this.

@DerDackel DerDackel force-pushed the reintroduce-monitoring branch from a0060e3 to ccc494f Compare July 3, 2025 11:03
@DerDackel DerDackel force-pushed the reintroduce-monitoring branch from ccc494f to dc1e41f Compare July 3, 2025 11:11
@DerDackel
Copy link
Contributor Author

DerDackel commented Jul 3, 2025

Rechecked the old PR and rebased it onto a current state.

Open points:

  1. I built my own monitoring instance to aggregate metrics from all functions in the same widget, as the old monitoring from v1 did - I wonder if that was overkill, but I found no way of doing this inside the cdk-monitoring-constructs.
  2. With the current setup, functions added after construction of an API construct instance will not be included. I think I can make this work by keeping my custom Monitoring construct as a protected instance field.
  3. I do not have enough experience with GraphQL to properly test that side of the monitoring.

There is some stateful wonkery inside, because cdk-monitoring-constructs lacks functionality for influencing the layout of a dashboard
@DerDackel DerDackel force-pushed the reintroduce-monitoring branch from c9cde58 to 2d84aa3 Compare July 3, 2025 14:49
@DerDackel
Copy link
Contributor Author

I forgot: cdk-monitoring-constructs still does not support any way to influence widget ordering or sizing on the generated dashboards. For now I've separated initializing the function monitoring and adding it to the overall MonitoringFacade, so that leaves a chance for the API and DynamoDB monitoring in the API subclasses to be initialized.

Still not sure about the GraphQL part, but this should at least do for monitoring RestAPIs.

@DerDackel DerDackel marked this pull request as ready for review July 3, 2025 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants