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

Most or all metrics exported by Retina should be counters, not gauges #237

Closed
andreev-io opened this issue Apr 8, 2024 · 3 comments · Fixed by #693
Closed

Most or all metrics exported by Retina should be counters, not gauges #237

andreev-io opened this issue Apr 8, 2024 · 3 comments · Fixed by #693
Assignees
Labels
area/metrics good first issue Good for newcomers lang/go The Go Programming Language priority/1 P1 type/bug Something isn't working
Milestone

Comments

@andreev-io
Copy link

andreev-io commented Apr 8, 2024

Describe the bug
Retina is currently exporting gauge-type metrics in its modules (e.g. forward.go, drops.go. Since these metrics only go up (or are reset to 0), they should be counters. This is important since a metric type different from the metric's semantical meaning can break the implicit contract and assumptions that people make when querying the data.

From Prometheus documentation:

A gauge is a metric that represents a single numerical value that can arbitrarily go up and down.

A counter is a cumulative metric that represents a single monotonically increasing counter whose value can only increase or be reset to zero on restart. For example, you can use a counter to represent the number of requests served, tasks completed, or errors.

To Reproduce
N/A

Expected behavior
N/A

Screenshots
N/A

Platform (please complete the following information):
N/A

Additional context
N/A

@andreev-io andreev-io changed the title Most or all metrics exported by Retina should be counters, not gauged Most or all metrics exported by Retina should be counters, not gauges Apr 8, 2024
@andreev-io
Copy link
Author

I'm going to open a PR to address this in the forward plugin as an example.

andreev-io added a commit to andreev-io/retina that referenced this issue Apr 8, 2024
@rbtr rbtr added type/bug Something isn't working good first issue Good for newcomers lang/go The Go Programming Language area/metrics priority/1 P1 labels Apr 8, 2024
@andreev-io
Copy link
Author

@rbtr Could you assign this to me? I opened #238, but I need some help with figuring out how to build Retina locally: #238 (comment).

@rbtr rbtr moved this to Ready in Retina Triage Board Apr 8, 2024
@rbtr rbtr added this to the 1.0 milestone Apr 12, 2024
@rbtr rbtr mentioned this issue May 17, 2024
7 tasks
@nddq
Copy link
Contributor

nddq commented Aug 30, 2024

just look at these metrics recently and thought of this issue. The reason why we are doing gauges and not counters, even though these metrics supposed to be only ever increasing, is that because we are doing snapshots of the bpf map periodically and Set the metric to whatever the values we've read. The actual counting is being done in eBPF. We should rename <MetricName>Counter to <MetricName>Gauge to prevent confusion

github-merge-queue bot pushed a commit that referenced this issue Sep 6, 2024
# Description

As title. Closes #237. Also
did a bunch of refactoring + removing unused metrics
## Related Issue

If this pull request is related to any issue, please mention it here.
Additionally, make sure that the issue is assigned to you before
submitting this pull request.

## Checklist

- [ ] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [ ] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [ ] I have correctly attributed the author(s) of the code.
- [ ] I have tested the changes locally.
- [ ] I have followed the project's style guidelines.
- [ ] I have updated the documentation, if necessary.
- [ ] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes
made.

## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.
@nddq nddq closed this as completed in #693 Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics good first issue Good for newcomers lang/go The Go Programming Language priority/1 P1 type/bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants