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

Add _total suffix to counter metrics that didn't have it #1208

Merged
merged 5 commits into from
Jul 17, 2023

Conversation

lambdanis
Copy link
Contributor

@lambdanis lambdanis commented Jul 10, 2023

For compatibility with the OpenMetrics standard. This is a breaking change, but it shouldn't be painful - the semantics of the
metrics doesn't change, only the names.

While here, I renamed a few metrics to follow common naming conventions:

  • event_cache_count -> tetragon_event_cache_accesses_total
  • tetragon_policy_stats -> tetragon_policy_events_total
  • tetragon_syscall_stats -> tetragon_syscalls_total

Admittedly, it's also not super necessary - using plain Prometheus you can scrape and query existing metrics just fine. Maybe there are/will be other systems that are pedantic about the standard, then this change will bring some benefits, but otherwise it's kinda compatibility for its own sake. Please shout if you think it's not worth it, but IMO this change is innocent enough to do it :)

I also removed unused label value constants from the errormetrics package. Let me know if I shouldn't do that - these vars are exported, but not used anywhere.

All counter metrics that didn't have the _total suffix now have it.

@netlify
Copy link

netlify bot commented Jul 10, 2023

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 709952b
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/64b130acd7663d0008ba81e7
😎 Deploy Preview https://deploy-preview-1208--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@lambdanis lambdanis added the area/metrics Related to prometheus metrics label Jul 10, 2023
@lambdanis lambdanis force-pushed the pr/lambdanis/counters-total branch 2 times, most recently from d4cf298 to 0d8baee Compare July 12, 2023 19:17
@lambdanis lambdanis marked this pull request as ready for review July 12, 2023 19:28
@lambdanis lambdanis requested a review from a team as a code owner July 12, 2023 19:28
@lambdanis lambdanis requested a review from olsajiri July 12, 2023 19:28
Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
@lambdanis lambdanis force-pushed the pr/lambdanis/counters-total branch 2 times, most recently from 709952b to a5a3357 Compare July 14, 2023 11:39
To tetragon_event_cache_accesses_total.
The new name follows the OpenMetrics standard and common conventions.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
To tetragon_policy_events_total.
The new name follows the OpenMetrics standard and common conventions.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
To tetragon_syscalls_total.
The new name follows the OpenMetrics standard and common conventions.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
For compatibility with the OpenMetrics standard:
https://github.com/OpenObservability/OpenMetrics/blob/v1.0.0/specification/OpenMetrics.md#suffixes

This is a breaking change, but it shouldn't be painful - the semantics of the
metrics doesn't change, only the names.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
@lambdanis lambdanis force-pushed the pr/lambdanis/counters-total branch from a5a3357 to d6b4e3e Compare July 14, 2023 11:45
@kkourt kkourt merged commit be355a8 into cilium:main Jul 17, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Related to prometheus metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants