-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Feature/prometheus #415
Feature/prometheus #415
Conversation
…et the metrics data as this feels more inline with the Prometheus style
Fix json encoding archive escaped <, >, and &
stats/prometheus/prometheus.go
Outdated
) | ||
|
||
func HandlePrometheusMetrics() http.Handler { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this newline!
stats/prometheus/prometheus.go
Outdated
exporter := NewExporter(scrapeURI) | ||
prometheus.MustRegister(exporter) | ||
prometheus.MustRegister(version.NewCollector("k6_exporter")) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could honestly drop this one too.
api/server.go
Outdated
) | ||
|
||
func NewHandler() http.Handler { | ||
mux := http.NewServeMux() | ||
mux.Handle("/v1/", v1.NewHandler()) | ||
mux.Handle("/ping", HandlePing()) | ||
mux.Handle("/", HandlePing()) | ||
mux.Handle("/metrics", prometheus.HandlePrometheusMetrics()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really minor nitpick: this would look better above "/".
Here's a thought: instead of having Prometheus be a regular collector, since we're always listening on a port for the API anyhow, we could have it be a part of the API server. The API server already has a connection to the Engine, which has a built-in view of the current state of all metrics; if you can lazily request all metrics from there (no overhead if it's never called), we could have it always available that way. (I believe we can even export stuff like go runtime stats via prometheus? Because that would come in real handy for profiling. Or we could just make them metrics and hide them behind a flag, I suppose.) |
Closes grafana#211
Fixes panicking tests
hey @liclac, thanks for taking a look. Yea we could do. I did debate this, ultimately I ended up opting for using the API server, as I thought any transformations that had been done to the individual metrics would already be handled (such as units ms or μs), and would mean the Prom code would not need to be changed/refactored for this reason going forwards if these transformations were to change. I could be wrong with this but that was the logic behind it. In regards to the go runtime stats, this code already exposes this, see metrics below:
These metrics get exposed by default via the prom client, hence you won't see them explicitly created in prometheus.go. On another note, do you know why CI is failing this? 😕 |
`k6 cloud` command
`k6 login cloud` command
Add test coverage
When t.Count is even, t.Med should be half the sum of the two middle values in t.Values.
[Fix] Correct calculation for t.Med
[Fix] Corrects calculation of t.Med in stats output
…to feature/prometheus
…et the metrics data as this feels more inline with the Prometheus style
…to feature/prometheus
Closing this in favour of #478 |
Hey guys,
Could I get an eye over this and thoughts/comments. Please test this out via:
go build
k6 run script.js -v
http://<host>:6565/metrics
K6 metric names exposed to Prometheus:
#343