Skip to content

Commit

Permalink
Merge pull request #416 from hashicorp/b-gh-398
Browse files Browse the repository at this point in the history
agent: return req error if prometheus metrics are disabled.
  • Loading branch information
jrasell authored Mar 10, 2021
2 parents c1c67f6 + 32f00d5 commit fa75e04
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## UNRELEASED

BUG FIXES:
* agent: Only allow querying Prometheus formatted metrics if Prometheus is enabled within the config [[GH-416](https://github.com/hashicorp/nomad-autoscaler/pull/416)]
* policy: Ensure metric emitters use the correct context and are stopped when appropriate [[GH-408](https://github.com/hashicorp/nomad-autoscaler/pull/408)]

## 0.3.0 (February 25, 2021)
Expand Down
2 changes: 1 addition & 1 deletion agent/http/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestServer_agentReload(t *testing.T) {
},
}

srv, stopSrv := TestServer(t)
srv, stopSrv := TestServer(t, false)
defer stopSrv()

for _, tc := range testCases {
Expand Down
2 changes: 1 addition & 1 deletion agent/http/health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestServer_getHealth(t *testing.T) {
}

// Create our HTTP server.
srv, stopSrv := TestServer(t)
srv, stopSrv := TestServer(t, false)
defer stopSrv()

for _, tc := range testCases {
Expand Down
6 changes: 6 additions & 0 deletions agent/http/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ func (s *Server) getMetrics(w http.ResponseWriter, r *http.Request) (interface{}
}

if format := r.URL.Query().Get("format"); format == "prometheus" {

// Only return Prometheus formatted metrics if the user has enabled
// this functionality.
if !s.promEnabled {
return nil, newCodedError(http.StatusUnsupportedMediaType, "Prometheus is not enabled")
}
s.getPrometheusMetrics().ServeHTTP(w, r)
return nil, nil
}
Expand Down
19 changes: 14 additions & 5 deletions agent/http/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,41 +14,50 @@ func TestServer_getMetrics(t *testing.T) {
inputWriter *httptest.ResponseRecorder
expectedRespCode int
expectedRespContains string
enableProm bool
name string
}{
{
inputReq: httptest.NewRequest("PUT", "/v1/metrics", nil),
inputWriter: httptest.NewRecorder(),
expectedRespCode: 405,
expectedRespContains: "Invalid method",
enableProm: false,
name: "incorrect request method",
},
{
inputReq: httptest.NewRequest("GET", "/v1/metrics", nil),
inputWriter: httptest.NewRecorder(),
expectedRespCode: 200,
expectedRespContains: "Counters\":[],\"Gauges\":[],\"Points\":[],\"Samples\":[]",
enableProm: false,
name: "correct request for JSON metrics",
},

{
inputReq: httptest.NewRequest("GET", "/v1/metrics?format=prometheus", nil),
inputWriter: httptest.NewRecorder(),
expectedRespCode: 200,
expectedRespContains: "# TYPE go_goroutines gauge",
enableProm: true,
name: "correct request for Prometheus formatted metrics",
},
{
inputReq: httptest.NewRequest("GET", "/v1/metrics?format=prometheus", nil),
inputWriter: httptest.NewRecorder(),
expectedRespCode: 415,
expectedRespContains: "",
enableProm: false,
name: "Prometheus format metrics disabled",
},
}

// Create our HTTP server.
srv, stopSrv := TestServer(t)
defer stopSrv()

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
srv, stopSrv := TestServer(t, tc.enableProm)
srv.mux.ServeHTTP(tc.inputWriter, tc.inputReq)
assert.Equal(t, tc.expectedRespCode, tc.inputWriter.Code, tc.name)
assert.Contains(t, tc.inputWriter.Body.String(), tc.expectedRespContains, tc.name)
stopSrv()
})
}
}
13 changes: 9 additions & 4 deletions agent/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ type Server struct {
mux *http.ServeMux
srv *http.Server

// promEnabled tracks whether Prometheus formatted metrics should be
// enabled.
promEnabled bool

// aliveness is used to describe the health response and should be set
// atomically using healthAlivenessReady and healthAlivenessUnavailable
// const declarations.
Expand All @@ -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) {

srv := &Server{
log: log.Named("http_server"),
mux: http.NewServeMux(),
agent: agent,
log: log.Named("http_server"),
mux: http.NewServeMux(),
agent: agent,
promEnabled: prom,
}

// Setup our handlers.
Expand Down
4 changes: 2 additions & 2 deletions agent/http/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import (
"github.com/hashicorp/nomad-autoscaler/agent/config"
)

func TestServer(t *testing.T) (*Server, func()) {
func TestServer(t *testing.T, enableProm bool) (*Server, func()) {
cfg := &config.HTTP{
BindAddress: "127.0.0.1",
BindPort: 0, // Use next available port.
}

s, err := NewHTTPServer(false, cfg, hclog.NewNullLogger(), &agent.MockAgentHTTP{})
s, err := NewHTTPServer(false, enableProm, cfg, hclog.NewNullLogger(), &agent.MockAgentHTTP{})
if err != nil {
t.Fatalf("failed to start test server: %v", err)
}
Expand Down
3 changes: 2 additions & 1 deletion command/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ func (c *AgentCommand) Run(args []string) int {

// create and run agent and HTTP server
c.agent = agent.NewAgent(parsedConfig, configPaths, logger)
httpServer, err := agentHTTP.NewHTTPServer(parsedConfig.EnableDebug, parsedConfig.HTTP, logger, c.agent)
httpServer, err := agentHTTP.NewHTTPServer(
parsedConfig.Telemetry.PrometheusMetrics, parsedConfig.EnableDebug, parsedConfig.HTTP, logger, c.agent)
if err != nil {
logger.Error("failed to setup HTTP getHealth server", "error", err)
return 1
Expand Down

0 comments on commit fa75e04

Please sign in to comment.