-
Notifications
You must be signed in to change notification settings - Fork 4
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 /metrics endpoint #51
Conversation
93be500
to
13e47af
Compare
This comment has been minimized.
This comment has been minimized.
13e47af
to
93310de
Compare
This comment has been minimized.
This comment has been minimized.
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.
comment nit
This comment has been minimized.
This comment has been minimized.
8470ec6
to
da8a887
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
this is shaping up nicely, just some questions and small bits
_, _ = w.Write([]byte("ok")) | ||
})) | ||
|
||
httpServer := &http.Server{ |
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.
does this mean the binary has two http servers (the api and the internalserver)? That's a little weird but I can see why we might do it that way. Though that means we need to make sure the /metrics call is going to the correct port vs the write calls. (Not sure where promscrape is configured)
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.
Yep! I figured out how to do it yesterday, so I'll have a follow-up PR to get that fixed once this gets merged.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
f444d7f
to
21e2e88
Compare
Go coverage report: Click to expand.
Go lint report: No issues found. 😎 |
flags.Int64Var(&cfg.HTTPMaxRequestSizeLimit, prefix+"server.http-max-req-size-limit", defaultHTTPRequestSizeLimit, "HTTP max request body size limit. Defaults to 10 mb.") | ||
flags.StringVar(&cfg.HTTPListenAddress, prefix+"server.http-listen-address", "0.0.0.0", "Sets listen address for the http server") | ||
flags.IntVar(&cfg.HTTPListenPort, prefix+"server.http-listen-port", defaultListenPort, "Sets listen address port for the http server") | ||
flags.IntVar(&cfg.HTTPConnLimit, prefix+"server.http-listen-conn-limit", 0, "Sets a limit to the amount of http connections, 0 means no limit") |
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.
nice, thanks for these updates
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.
thanks!
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.
thanks!
This PR adds an "internal" server that defaults to listening on port 8081 and provides two endpoints:
/metrics
: Prometheus metrics endpoint/healthz
: A health check endpoint