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

Convert citadel cert expiry timestamp metrics to citadel cert expiry time left #51037

Closed
wadhwakabir opened this issue May 14, 2024 · 4 comments · Fixed by #51076
Closed

Convert citadel cert expiry timestamp metrics to citadel cert expiry time left #51037

wadhwakabir opened this issue May 14, 2024 · 4 comments · Fixed by #51076
Assignees

Comments

@wadhwakabir
Copy link

(This is used to request new product features, please visit https://github.com/istio/istio/discussions for questions on using Istio)

Describe the feature request

Currently istio metrics citadel_server_cert_chain_expiry_timestamp reports The unix timestamp, in seconds, when Citadel cert chain will expire. A negative time indicates the cert is expired..
It is not possible in datadog to create alert for cert expiration is due in x days, as to get time left we need to subtract current time from metrics value. ( not supported functionality in datadog)

https://istio.io/latest/docs/reference/commands/pilot-discovery/#metrics

same case is for citadel_server_root_cert_expiry_timestamp metrics

Describe alternatives you've considered

we can add additional metrics citadel_server_cert_chain_expiry_time_left that report time in seconds left for expiration of certificate along with current metrics .

certChainExpiryTimestamp.Record(certChainExpiry)

same case is for citadel_server_root_cert_expiry_time_left metrics.

Affected product area (please put an X in all that apply)

[ ] Ambient
[ ] Docs
[ ] Dual Stack
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[*] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Affected features (please put an X in all that apply)

[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane

Additional context

@keithmattix
Copy link
Contributor

This feels reasonable to me; wdyt @jaellio @whitneygriffith?

@whitneygriffith
Copy link
Contributor

This feels reasonable to me; wdyt @jaellio @whitneygriffith?

I can pick it up

@whitneygriffith whitneygriffith self-assigned this May 14, 2024
@howardjohn
Copy link
Member

We already have one in agent so we should make sure we have a consistent name/semantics there

@whitneygriffith
Copy link
Contributor

We already have one in agent so we should make sure we have a consistent name/semantics there

That is, the citadel cert time left should be consistent with the cert_expiry_seconds metric in agent

Name: citadel_server_cert_chain_expiry_seconds and citadel_server_root_cert_expiry_seconds
Type: LastValue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants