Skip to content

Conversation

@plamen-bardarov
Copy link
Contributor

@plamen-bardarov plamen-bardarov commented Aug 12, 2024

Summary

The proposed change is to improve the observability of failed Liveness Checks by emitting a metric.
cloudfoundry/diego-release#953

Backward Compatibility

Breaking Change? No

@plamen-bardarov plamen-bardarov requested a review from a team as a code owner August 12, 2024 09:59
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 12, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@plamen-bardarov plamen-bardarov changed the title added metrics for failed liveness checks Add metrics for failed liveness checks Aug 12, 2024
Copy link
Member

@ameowlia ameowlia left a comment

Choose a reason for hiding this comment

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

👋 Hi @plamen-bardarov and @vlast3k,

Apologizes for the delay in the review. We have been struggling to get the Diego pipeline green, it is sadly still not green.

✅ The code looks good.
✅ I ran this IRL and was able to see the expected output.
❓ Can you please add a bosh level property to turn off this value by default?
😢 The pipeline is not green.

Once you get the config property in I will approve. And then once the pipeline is green I will merge.

@plamen-bardarov
Copy link
Contributor Author

Hi @ameowlia, do you mean adding 1 property to enable sending liveness check failure metrics altogether or you mean introducing more granular control over which types of liveness/startup check failures to emit(HTTP/TCP/etc.)?

@plamen-bardarov
Copy link
Contributor Author

plamen-bardarov commented Sep 17, 2024

Added new property for enabling the metrics. Will also create PR for diego-release
Update: Heres the PR for diego-release cloudfoundry/diego-release#964

@ameowlia ameowlia merged commit 079af51 into cloudfoundry:main Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants