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

added prometheus metrics to the live running host #396

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tehlers320
Copy link

Hello, i found your project and was looking to use it for some very long runs and would prefer the metric data to be sent live if possible. AFAIK you cant do this out of the box. I've tested a quick-n-dirty implementation of go-grpc-prometheus with config flags to turn it on/off.

Here is a sample dashboard all based on the examples here.
Screen Shot 2023-01-13 at 3 32 38 PM

Thanks, let me know what you think.

@@ -30,81 +32,81 @@ var (
// Proto
isProtoSet = false
proto = kingpin.Flag("proto", `The Protocol Buffer .proto file.`).
PlaceHolder(" ").IsSetByUser(&isProtoSet).String()
Copy link
Author

Choose a reason for hiding this comment

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

Sorry my IDE did this should i revert all of them?

@bojand
Copy link
Owner

bojand commented Apr 15, 2023

Sorry somehow missed this PR. Will take a look soon.

@bojand
Copy link
Owner

bojand commented Apr 15, 2023

I am interested to learn more about your use case. I get the general idea and I think something like this could be useful. The issue with this specific implementation is that we would possibly (probably?) miss some scrape data? Prometheus normally scrapes metrics endpoints at some interval (ex 60s). So if for example you do a ghz run that runs less than 60s, or any amount of time over 1m, you could potentially miss almost a full scrape worth of data at the end. This may or may not matter, but can be problematic. And this is only one example. Essentially the process needs to run until all data has been scraped for this to be useful. So we almost need a different signal to stop the process somehow.

Additionally I think that package is deprecated in favour of github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus.

I think something like this is useful, but it might be better to try and build something that can be used programatically first and maybe generalized... that may inform opinions and offer some insights into practical way to provide it as part of CLI if possible.

I will think about this some more.

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

Successfully merging this pull request may close these issues.

None yet

2 participants