Add Timestamps to Health Status

[mkieweg]

Summary

Currently there is no way to notify on a certain health status being active longer than a set period of time. I propose to add a lastUpdateTime timestamp to app.status.health indicating how long the current status is active for.

Motivation

Since an Application with an erroneous configuration might be Progressing forever, I want to be able to leverage ArgoCD notifications to solve this issue. Currently the only workaround I'm aware (Slack discussion) of is to cross reference app.status.operationState.startedAt and app.status.health.status like this:

trigger.on-progressing-stuck: |
  - description: Application stuck progressing
    send:
    - app-stuck-progressing
    when: app.status.health.status == 'Progressing'
      and time.Now().Sub(time.Parse(app.status.operationState.startedAt)).Minutes() > 60 and time.Now().Sub(time.Parse(app.status.operationState.startedAt)).Minutes() < 62

which is prone to over-alerting as it triggers any instance of Progressing within this time span. So it's impossible to distinguish bad timing of a pod eviction from an app actually stuck on Progressing

Proposal

The lastUpdateTime field should be added to the respective struct and be set to the current time when the status changes.

I'm happy to take on the implementation.

[svghadi]

This would also help to stablize on-deployed notification trigger. I am looking for something similar to address false positive for on-deployed notifications. Having a field like since or lastUpdateTime under health status could help minimize false positives.
Ref: #9070 (comment)

[mkieweg]

Having a field like since or lastUpdateTime under health status could help minimize false positives.

That sentence made me realise that since might be confusing because of time.Since() having a different semantic. Changed the proposal accordingly.

[svghadi]

Hi @mkieweg, I discussed this enhancement in today's contributors meeting, and there were no objections. It's a green light for implementation. Please let me know if you have the bandwidth to take this up, otherwise, I am happy to proceed with it.

[mkieweg]

Hi @svghadi, happy to hear that. I'll take on the implementation, I've allocated time for it next week :)

[svghadi]

Cool. Here is something I was trying out last week for lastUpdateTime. I have not tested it out extensively. Just sharing for reference.
svghadi@d8a5321

[hiddeco]

While I am super supportive of this being added, I have an edge case that I think falls into this category but is not covered by just this addition.

Within Argo CD, the sync operations are completely separated from the application health. However, in certain scenarios, you do want to know if the current state of the health check is the outcome of a previous sync operation.

With the proposal, this would become possible by comparing the .finishedAt from the .status.operationState against the .lastUpdateTime. This, however, only works when you sync workload resources, which means that it will not help in scenarios where you only have non-workload resources, which will always end up Healthy without causing a transition.

To also cover this, and while taking into account the comments[1][2], I wonder if it would be an idea to introduce a "heartbeat" kind of timestamp that updates at most every 30 seconds (to not cause any API pressure) to indicate the "freshness" of the observation even without it transitioning.