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

connect: emit a metric for the seconds until root CA expiry #9924

Merged
merged 2 commits into from
Jun 18, 2021
Merged

connect: emit a metric for the seconds until root CA expiry #9924

merged 2 commits into from
Jun 18, 2021

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Mar 24, 2021 •
edited
Loading

Implements the metrics portion of #9891

This PR adds a new metric, consul.mesh.root-ca.expiry which is a gauge. The metric is update every hour by a goroutine run from the leader. The value of the gauge is the number of seconds until the root CA expires. Someone running Consul should be able to setup an alert on this value, so that they get notified when the value drops below some threshold.

This is difficult to unit test so I might try and test it out manually.

TODO:

  • manual test (using socat -d - udp6-listen:8125 as a statsd server)
  • update static metric definitions so this is populated properly when used with prometheus.
  • changelog

@dnephin dnephin added type/enhancement Proposed improvement or new feature theme/telemetry Anything related to telemetry or observability labels Mar 24, 2021
@dnephin dnephin requested a review from a team March 24, 2021 21:41
@github-actions github-actions bot added type/docs Documentation needs to be created/updated/clarified and removed theme/telemetry Anything related to telemetry or observability labels Mar 24, 2021
@hashicorp-ci
Copy link
Contributor

🤔 This PR has changes in the website/ directory but does not have a type/docs-cherrypick label. If the changes are for the next version, this can be ignored. If they are updates to current docs, attach the label to auto cherrypick to the stable-website branch after merging.

@dnephin dnephin mentioned this pull request Mar 24, 2021
Closed
dnephin
dnephin commented Mar 24, 2021
View reviewed changes
agent/consul/leader_connect.go Outdated
@@ -146,6 +148,47 @@ func (s *Server) pruneCARoots() error {
return nil
}

func emitCAExpirationMetrics(s *Server) func(ctx context.Context) error {
key := []string{"mesh", "root-ca", "expiry"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on this metric name? I couldn't find any other CA specific metrics. We could use a different key.

@dnephin dnephin marked this pull request as draft April 7, 2021 18:38
@hashicorp-cla
Copy link

hashicorp-cla commented May 25, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – consul May 25, 2021 18:17 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging May 25, 2021 18:17 Inactive
@dhiaayachi dhiaayachi changed the base branch from master to dhia/debug-generate-single-file May 25, 2021 18:28
@dhiaayachi dhiaayachi changed the base branch from dhia/debug-generate-single-file to master May 25, 2021 18:28
@dnephin dnephin force-pushed the dnephin/cert-expiration-metric branch from e501512 to ab176b0 Compare May 31, 2021 20:01
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging May 31, 2021 20:01 Inactive
@dnephin dnephin force-pushed the dnephin/cert-expiration-metric branch from ab176b0 to e2ebb2c Compare June 10, 2021 19:22
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging June 10, 2021 19:22 Inactive
@dnephin dnephin force-pushed the dnephin/cert-expiration-metric branch from e2ebb2c to 83ed904 Compare June 10, 2021 21:09
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging June 10, 2021 21:10 Inactive
@dnephin dnephin requested review from a team and removed request for a team June 10, 2021 21:10
@dnephin dnephin force-pushed the dnephin/cert-expiration-metric branch from 83ed904 to e9a0f01 Compare June 10, 2021 21:12
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging June 10, 2021 21:12 Inactive
@dnephin dnephin marked this pull request as ready for review June 10, 2021 21:13
kyhavlov
kyhavlov approved these changes Jun 11, 2021
View reviewed changes
Copy link
Contributor

@kyhavlov kyhavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

I can't think of a better name for the metric - mesh.root-ca.expiry is concise and accurate.

@dnephin dnephin force-pushed the dnephin/cert-expiration-metric branch from e9a0f01 to 42ad984 Compare June 14, 2021 21:00
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging June 14, 2021 21:00 Inactive
@dnephin
Update metric name
aec7e79 
and handle the case where there is no active root CA.
@dnephin dnephin force-pushed the dnephin/cert-expiration-metric branch from 42ad984 to aec7e79 Compare June 14, 2021 21:01
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging June 14, 2021 21:01 Inactive
@dnephin
Copy link
Contributor Author

dnephin commented Jun 14, 2021 •
edited
Loading

Updated the metric name to active-root-ca and handled the case where the CaRootActive call returns nil.

@dnephin dnephin merged commit d81f527 into master Jun 18, 2021
@dnephin dnephin deleted the dnephin/cert-expiration-metric branch June 18, 2021 18:18
@hc-github-team-consul-core
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/389762.

@dnephin dnephin mentioned this pull request Aug 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@freddygv freddygv freddygv approved these changes

@kyhavlov kyhavlov kyhavlov approved these changes

Assignees
No one assigned
Labels
type/docs Documentation needs to be created/updated/clarified type/enhancement Proposed improvement or new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dnephin @hashicorp-ci @hashicorp-cla @hc-github-team-consul-core @freddygv @kyhavlov