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

agent: return req error if prometheus metrics are disabled. #416

Merged
merged 2 commits into from
Mar 10, 2021
Merged

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Mar 9, 2021

If the user has disabled Prometheus metrics and a request is
sent to the metrics endpoint requesting Prometheus formatted
metrics, then the request should fail.

The returned response code copies that of Consul and Nomad,
to provide a consistent user experience.

closes #398

Copy link
Contributor

@cgbaker cgbaker left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

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

Minor comment on config sprawl on the HTTP server, but not a blocker 👍

@@ -62,12 +66,13 @@ type Server struct {
}

// NewHTTPServer creates a new agent HTTP server.
func NewHTTPServer(debug bool, cfg *config.HTTP, log hclog.Logger, agent AgentHTTP) (*Server, error) {
func NewHTTPServer(debug, prom bool, cfg *config.HTTP, log hclog.Logger, agent AgentHTTP) (*Server, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're getting to the point where the input to NewHTTPServer would be better as a struct 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed and this did cross my mind that we just pass the whole config object. I'll follow this up in a new PR to refactor the func signature.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think passing the whole config would be too much, just the current params in a HTTPServerConfig struct should be enough.

@jrasell jrasell merged commit fa75e04 into main Mar 10, 2021
@jrasell jrasell deleted the b-gh-398 branch March 10, 2021 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

telemetry: Prometheus config flag prometheus_metrics doesn't alter endpoint behaviour
3 participants