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

feat: new cron workflow trigger counter metric #13274

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Conversation

Joibel
Copy link
Member

@Joibel Joibel commented Jun 28, 2024

From #12589.

A new metric which counts how many times each cron workflow has triggered. A simple enough counter which can be checked against expectations for the cron.

Note to reviewers: this is now a standalone commit

@Joibel Joibel force-pushed the opentelemetry-podpending branch from 4daa3a0 to fe4d153 Compare July 1, 2024 07:42
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from c5d2f77 to dc7479a Compare July 1, 2024 07:42
@Joibel Joibel added area/controller Controller issues, panics area/metrics labels Jul 1, 2024
@agilgur5 agilgur5 changed the title feat: new cron workflow trigger counter metric 🪧 [DNM] feat: new cron workflow trigger counter metric Jul 1, 2024
@Joibel Joibel force-pushed the opentelemetry-podpending branch from fe4d153 to 0401646 Compare July 5, 2024 10:28
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from dc7479a to 38143f4 Compare July 5, 2024 10:28
@Joibel Joibel force-pushed the opentelemetry-podpending branch from 0401646 to 34739c8 Compare July 8, 2024 10:06
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from 38143f4 to 481c236 Compare July 8, 2024 10:07
@Joibel Joibel force-pushed the opentelemetry-podpending branch from 34739c8 to c2aa5eb Compare July 9, 2024 14:27
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from 481c236 to 18bb3a9 Compare July 9, 2024 14:27
@Joibel Joibel force-pushed the opentelemetry-podpending branch from c2aa5eb to a269114 Compare July 12, 2024 08:49
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from 18bb3a9 to 9def3b4 Compare July 12, 2024 08:49
@Joibel Joibel force-pushed the opentelemetry-podpending branch from a269114 to 7980eec Compare August 12, 2024 11:13
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from 9def3b4 to aaf38bd Compare August 12, 2024 11:13
@Joibel Joibel force-pushed the opentelemetry-podpending branch from 7980eec to 406bb3a Compare August 12, 2024 11:20
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from aaf38bd to 98775d8 Compare August 12, 2024 11:20
@Joibel Joibel force-pushed the opentelemetry-podpending branch from 406bb3a to 214ee64 Compare August 14, 2024 10:41
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from 98775d8 to a43524a Compare August 14, 2024 10:42
@Joibel Joibel force-pushed the opentelemetry-podpending branch from 214ee64 to e5ce65c Compare August 15, 2024 08:04
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from a43524a to 45303a9 Compare August 15, 2024 08:04
@Joibel Joibel force-pushed the opentelemetry-podpending branch from e5ce65c to 69dfcca Compare August 15, 2024 14:06
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from 45303a9 to e2eecd2 Compare August 15, 2024 14:07
@Joibel Joibel force-pushed the opentelemetry-podpending branch from 69dfcca to d1a14c8 Compare August 16, 2024 09:09
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from e2eecd2 to 8f68609 Compare August 16, 2024 09:10
@Joibel Joibel force-pushed the opentelemetry-podpending branch from d1a14c8 to f6fa6d5 Compare August 16, 2024 09:24
@Joibel Joibel changed the title 🪧 [DNM] feat: new cron workflow trigger counter metric feat: new cron workflow trigger counter metric Aug 16, 2024
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from 8f68609 to 075816f Compare August 16, 2024 09:28
@Joibel Joibel changed the base branch from opentelemetry-podpending to main August 16, 2024 09:30
Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

LGTM

@Joibel Joibel enabled auto-merge (squash) August 19, 2024 07:12
From #12589.

A new metric which counts how many times each cron workflow has
triggered. A simple enough counter which can be checked against
expectations for the cron.

Note to reviewers: this is part of a stack of reviews for metrics
changes. Please don't merge until the rest of the stack is also ready.

Signed-off-by: Alan Clucas <[email protected]>
@Joibel Joibel force-pushed the opentelemetry-crontrigger branch from cf87ddc to 780f611 Compare August 19, 2024 09:20
@Joibel Joibel merged commit 2192344 into main Aug 19, 2024
28 checks passed
@Joibel Joibel deleted the opentelemetry-crontrigger branch August 19, 2024 09:47
Copy link

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Small docs misses below

docs/metrics.md Show resolved Hide resolved
@@ -201,6 +201,15 @@ Metrics for the [Four Golden Signals](https://sre.google/sre-book/monitoring-dis
Some metric attributes may have high cardinality and are marked with ⚠️ to warn you. You may need to disable this metric or disable the attribute.
<!-- titles should be the exact metric name for deep-linking, alphabetical ordered -->
<!-- titles are without argo_workflows prefix -->
#### `cronworkflows_triggered_total`

Choose a reason for hiding this comment

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

Suggested change
> v3.6 and after

Version reference

@@ -201,6 +201,15 @@ Metrics for the [Four Golden Signals](https://sre.google/sre-book/monitoring-dis
Some metric attributes may have high cardinality and are marked with ⚠️ to warn you. You may need to disable this metric or disable the attribute.
<!-- titles should be the exact metric name for deep-linking, alphabetical ordered -->
<!-- titles are without argo_workflows prefix -->

Choose a reason for hiding this comment

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

I'd probably add a new line above and below this comment. A bit surprised markdownlint didn't require that

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

Successfully merging this pull request may close these issues.

3 participants