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

feat: allow specifying regions/accounts at a more granular level #533

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

echeung-amzn
Copy link
Member

@echeung-amzn echeung-amzn commented Jun 25, 2024

This allows users to target different regions/account for specific things within a facade rather than relying on 1 facade per region/account with the facade's global settings, so something like:

this.facade
  .monitorDynamoTable({
    ...,
  })
  .monitorDynamoTable({
    ...,
    region: <some_other_region>,
    account: <some_other_account>,
  })

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

echeung-amzn added a commit that referenced this pull request Jun 25, 2024
So we can later use it for #533.

---

_By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license_
@echeung-amzn echeung-amzn force-pushed the more-regions branch 6 times, most recently from f8539b4 to e845d15 Compare June 25, 2024 19:26
@echeung-amzn echeung-amzn added the do-not-merge Label to signify Mergify to not auto-merge. Useful for grouping PR merges into a single release. label Jul 2, 2024
@echeung-amzn echeung-amzn force-pushed the more-regions branch 2 times, most recently from a482960 to 256985d Compare July 2, 2024 14:54
echeung-amzn added a commit that referenced this pull request Jul 3, 2024
This is a continuation of #534, but it includes breaking changes to some
existing metric factories' constructor API contracts.

Also includes some minor private method renaming for consistency and a
basic facade test, pulled out from #533.

---

_By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license_
@echeung-amzn echeung-amzn marked this pull request as ready for review July 3, 2024 13:50
@echeung-amzn echeung-amzn force-pushed the more-regions branch 2 times, most recently from 52c95b0 to ebe6deb Compare July 3, 2024 14:03
---

_By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license_
@echeung-amzn echeung-amzn merged commit 4801636 into cdklabs:main Jul 3, 2024
9 checks passed
@echeung-amzn echeung-amzn deleted the more-regions branch July 3, 2024 15:45
echeung-amzn added a commit that referenced this pull request Aug 1, 2024
Addressing a few misses from #533.

Also updates the test in `MonitoringFacade.test.ts` since XAXR alarms
aren't possible.

Fixes #434, but only in cases where we can infer that the account/region
is actually different from the construct scope to prevent unnecessary
XAXR alarm errors.

Fixes #428, in cases where the account/region is actually different from
the construct scope.

---

_By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license_
echeung-amzn added a commit that referenced this pull request Aug 1, 2024
…r dashboards (#551)

Building on top of #533, but `monitorCustom` has its own API for
simplifying search expression queries so we need to explicitly handle
it.

Regular `new Metric(...)` calls just pass in the `account`/`metric` via
that API.

---

_By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Label to signify Mergify to not auto-merge. Useful for grouping PR merges into a single release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants