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.

Sign up for GitHub

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

Jump to bottom

[secrets] Add support for secrets count metric #137

Closed
echeung-amzn opened this issue May 11, 2022 · 8 comments · Fixed by #333
Closed

[secrets] Add support for secrets count metric #137

echeung-amzn opened this issue May 11, 2022 · 8 comments · Fixed by #333
Assignees
@zqumei0
Labels
effort: small Small development effort feature-request New feature good first issue Good for newcomers

Comments

@echeung-amzn
Copy link
Member

Feature scope

AWS Secrets Manager

Describe your suggested feature

Announced in https://aws.amazon.com/about-aws/whats-new/2022/05/aws-secrets-manager-publishes-usage-metrics-to-amazon-cloudwatch/

Docs: https://docs.aws.amazon.com/secretsmanager/latest/userguide/monitoring-cloudwatch.html
@voho
Copy link
Contributor

voho commented Jul 5, 2022

Interesting. Do you have any example of a use case? When I would like to get an alarm?

@echeung-amzn
Copy link
Member Author

I personally haven't had a need for it, so I'm not too sure. The announcement does note:

You can also set alarms for an unexpected increase or decrease in number of secrets.

...which may be interesting. If anyone has a need for this, do let us know in this issue!

@voho voho added good first issue Good for newcomers effort: small Small development effort labels Jul 22, 2022
@zqumei0
Copy link
Contributor

zqumei0 commented Feb 15, 2023

I was thinking of working on this issue as a first issue on this repository, would the direction for this type of a feature be creating its own module like aws-secretmanager-usage or adding another monitoring ts file under aws-secretsmanager (like how aws-sqs has two monitoring files)?

@echeung-amzn
Copy link
Member Author

SecretsManagerSecretMetricFactory and SecretsManagerSecretMonitoring would be appropriate places to do it.

SQS is a bit unique since we'd want slightly different metrics/dashboards for a DLQ vs. a regular queue.

@zqumei0
Copy link
Contributor

zqumei0 commented Feb 16, 2023

Just for a little more clarity, wouldn't SecretsManagerSecretMonitoring be specifically setting up metrics on a specific secret? A metric like secrets count would be some monitor that does not take in a secret only a scope since its a metric encompassing all secrets. I could make the secret parameter optional for SecretsManagerSecretMonitoring and if no secret is passed to the monitor then it will just do a secrets count, but not sure if that would be the right way to go.

@echeung-amzn
Copy link
Member Author

Ah, got it. Yes you're right, it'd seem appropriate to have a separate set of classes to handle these global-level metrics.

@zqumei0
Copy link
Contributor

zqumei0 commented Feb 16, 2023

Great, thanks for the response! Mind if you assign this to me? I'll get work on it this week.

zqumei0 pushed a commit to zqumei0/cdk-monitoring-constructs that referenced this issue Feb 27, 2023
@0xDream
feat(SecretsManager): Add support for secrets count metric (cdklabs#137)
6188049 
Added new Secrets Manager Monitor to support Account-Level Secrets
Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new
SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets
count and change in secrets count.
zqumei0 pushed a commit to zqumei0/cdk-monitoring-constructs that referenced this issue Feb 27, 2023
@0xDream
feat(SecretsManager): Add support for secrets count metric (cdklabs#137)
567e20f 
Added new Secrets Manager Monitor to support Account-Level Secrets
Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new
SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets
count and change in secrets count.
zqumei0 added a commit to zqumei0/cdk-monitoring-constructs that referenced this issue Feb 27, 2023
@zqumei0
feat(SecretsManager): Add support for secrets count metric (cdklabs#137)
a6bc465 
Added new Secrets Manager Monitor to support Account-Level Secrets
Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new
SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets
count and change in secrets count.
zqumei0 added a commit to zqumei0/cdk-monitoring-constructs that referenced this issue Feb 27, 2023
@zqumei0
feat(SecretsManager): Add support for secrets count metric (cdklabs#137)
d365b75 
Added new Secrets Manager Monitor to support Account-Level Secrets
Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new
SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets
count and change in secrets count.
@zqumei0
Copy link
Contributor

zqumei0 commented Feb 27, 2023

Comment on the change:
I created an alarm to detect any change on secret counts (addChangeInSecretCountAlarm). It works when set to just checking for an increase or a decrease in count, but when both are enabled it suffers from the same issue as the AnomalyAlarm bug in issue 332.

Let me know what you think about the alarms created and if maybe creating a separate issue than this for the anomaly check when the bug is resolved.

@echeung-amzn echeung-amzn mentioned this issue Feb 27, 2023
Merged
zqumei0 added a commit to zqumei0/cdk-monitoring-constructs that referenced this issue Feb 27, 2023
@zqumei0
feat(SecretsManager): Add support for secrets count metric (cdklabs#137)
399fb0d 
Added new Secrets Manager Monitor to support Account-Level Secrets
Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new
SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets
count and change in secrets count.
zqumei0 added a commit to zqumei0/cdk-monitoring-constructs that referenced this issue Jul 6, 2023
@zqumei0
feat(SecretsManager): Add support for secrets count metric (cdklabs#137)
b123aec 
Added new Secrets Manager Monitor to support Account-Level Secrets
Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new
SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets
count and change in secrets count.
zqumei0 added a commit to zqumei0/cdk-monitoring-constructs that referenced this issue Jul 6, 2023
@zqumei0
feat(SecretsManager): Add support for secrets count metric (cdklabs#137)
b744404 
Added new Secrets Manager Monitor to support Account-Level Secrets
Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new
SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets
count and change in secrets count.
@mergify mergify bot closed this as completed in #333 Jul 6, 2023
mergify bot pushed a commit that referenced this issue Jul 6, 2023
@zqumei0
feat(SecretsManager): Add support for secrets count metric (#333)
cdf8bd0 
Added new Secrets Manager Monitor to support Account-Level Secrets Count. This includes a new SecretsManagerMonitor, a new SecretsManagerMetricsFactory, and a new SecretsManagerAlarmFactory. There is alarm support for Min/Max Secrets count and change in secrets count.

Closes #137

---

_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
Assignees

@zqumei0 zqumei0

Labels
effort: small Small development effort feature-request New feature good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(SecretsManager): Add support for secrets count metric zqumei0/cdk-monitoring-constructs
3 participants
@voho @echeung-amzn @zqumei0