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

Update pims health checks to support configuration based enable/disab… #4099

Merged
merged 6 commits into from
Jun 12, 2024

Conversation

devinleighsmith
Copy link
Collaborator

…le and period.

@devinleighsmith devinleighsmith added enhancement New feature or request tech-debt Removing technical debt labels Jun 11, 2024
@devinleighsmith devinleighsmith self-assigned this Jun 11, 2024
Copy link
Contributor

✅ No secrets were detected in the code.

1 similar comment
Copy link
Contributor

✅ No secrets were detected in the code.

Copy link
Contributor

✅ No secrets were detected in the code.

Copy link
Contributor

✅ No secrets were detected in the code.

if (allHealthCheckOptions.ApiMetrics.Enabled)
{
services.AddHealthChecks().Add(new HealthCheckRegistration(
"ApiMetrics",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we use the string "api-metrics" in our Prometheus/Sysdig dashboards. We will need to update the dashboards to read from the new "ApiMetrics" information or they will show no data...

Copy link
Collaborator

Choose a reason for hiding this comment

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

NVM - I checked Sysdig and we don't use that name at all - disregard previous comment.

Comment on lines 32 to 33
"Enabled": true,
"Duration": 1
Copy link
Collaborator

@asanchezr asanchezr Jun 11, 2024

Choose a reason for hiding this comment

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

I see a discrepancy here in regards to the duration/period. In other configuration files I see the override values as "Period" vs here they are set originally as "Duration". I think you need to pick one...
image

Perhaps rename these ones as Period?

Copy link
Contributor

✅ No secrets were detected in the code.

1 similar comment
Copy link
Contributor

✅ No secrets were detected in the code.

@devinleighsmith devinleighsmith merged commit 6429e64 into bcgov:dev Jun 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tech-debt Removing technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants