Skip to content

Add prometheus metrics to osctrl-tls #522

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

Merged
merged 10 commits into from
Oct 10, 2024
Merged

Conversation

zhuoyuan-liu
Copy link
Contributor

@zhuoyuan-liu zhuoyuan-liu commented Sep 27, 2024

This PR added Prometheus metrics support for osctrl-tls. Issue: #504

Currently, it has all default metrics go metrics such as memory info and gc duration etc. Also, we added one custom histogram metric to measure the total request/latency to each osquery endpoint.

The metric server would run as a separate service in a different port. We can query e.g. P90 latency for each endpoint with different environments with the histogram metric.

image

Also the request rate for different path and status code.
image

@zhuoyuan-liu zhuoyuan-liu marked this pull request as ready for review September 27, 2024 10:37
@javuto javuto self-assigned this Oct 1, 2024
@javuto javuto added osctrl-tls osctrl-tls related changes 📊 metrics Metrics related issues labels Oct 1, 2024
@javuto javuto self-requested a review October 2, 2024 17:24
@javuto
Copy link
Collaborator

javuto commented Oct 2, 2024

Thank you for the PR! I will add some comments in a code review 🙂

Copy link
Collaborator

@javuto javuto left a comment

Choose a reason for hiding this comment

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

Thanks again for the PR! 🙏

I have added few comments, mostly related with having metrics not enabled by default, and the ability to enable them via configuration or flags.

@zhuoyuan-liu
Copy link
Contributor Author

I have updated the flag. Should I start to clean up the old metrics components for osctrl-tls?

@zhuoyuan-liu zhuoyuan-liu requested a review from javuto October 4, 2024 14:36
@javuto javuto mentioned this pull request Oct 8, 2024
@javuto
Copy link
Collaborator

javuto commented Oct 8, 2024

I have updated the flag. Should I start to clean up the old metrics components for osctrl-tls?

You mean the emitted metrics? Instead of removing them, it would be better if we can find a way for those to be utilized by prometheus too.

@zhuoyuan-liu
Copy link
Contributor Author

@javuto I have added the flag, could you please take another look?

@zhuoyuan-liu
Copy link
Contributor Author

@javuto I Just fixed the conflicts.

@javuto javuto merged commit 5ad3ce7 into jmpsec:main Oct 10, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📊 metrics Metrics related issues osctrl-tls osctrl-tls related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants