Add support for labels/filters from go-metrics #3369
Conversation
agent/http.go
Outdated
| @@ -97,6 +98,7 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler { | |||
| handleFuncMetrics("/v1/agent/maintenance", s.wrap(s.AgentNodeMaintenance)) | |||
| handleFuncMetrics("/v1/agent/reload", s.wrap(s.AgentReload)) | |||
| handleFuncMetrics("/v1/agent/monitor", s.wrap(s.AgentMonitor)) | |||
| handleFuncMetrics("/v1/agent/metrics", s.wrap(s.agent.MemSink.DisplayMetrics)) | |||
There was a problem hiding this comment.
We will need to make an intermediate wrapper that applies the ACL policy.
There was a problem hiding this comment.
what sort of acl policy applies to be able to view metrics?
There was a problem hiding this comment.
it'll require agent:read
api/agent.go
Outdated
| // Metrics info is used to store different types of metric values from the agent. | ||
| type MetricsInfo struct { | ||
| Timestamp string | ||
| Gauges []map[string]interface{} |
There was a problem hiding this comment.
These would be more useful with concrete types. The structs can be defined in the API package.
agent/http.go
Outdated
| @@ -97,6 +98,7 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler { | |||
| handleFuncMetrics("/v1/agent/maintenance", s.wrap(s.AgentNodeMaintenance)) | |||
| handleFuncMetrics("/v1/agent/reload", s.wrap(s.AgentReload)) | |||
| handleFuncMetrics("/v1/agent/monitor", s.wrap(s.AgentMonitor)) | |||
| handleFuncMetrics("/v1/agent/metrics", s.wrap(s.agent.MemSink.DisplayMetrics)) | |||
There was a problem hiding this comment.
what sort of acl policy applies to be able to view metrics?
|
|
||
| ```json | ||
| { | ||
| "Timestamp": "2017-08-08 02:55:10 +0000 UTC", |
There was a problem hiding this comment.
Is this the timestamp of when the endpoint was queried, or the timestamp of the last bucket corresponding to the metrics being returned? Some clarification in the docs would help.
There was a problem hiding this comment.
It's for the interval the metrics were collected in - if a different timestamp comes back, it's a different set of metrics. I'll add the info about the fields below this example
| * <a name="telemetry-filter_default"></a><a href="#telemetry-filter_default">`filter_default`</a> | ||
| This controls whether to allow metrics that have not been specified by the filter. Defaults to `true`, which will | ||
| allow all metrics when no filters are provided. When set to `false` with no filters, no metrics will be sent. | ||
|
|
There was a problem hiding this comment.
Should these be made reloadable, so people can turn on extra metrics for troubleshooting without restarting?
There was a problem hiding this comment.
yeah that's a good catch, I forgot about making it reloadable
There was a problem hiding this comment.
I don't think the go-metrics interface is set up for it to be reloadable unless we make changes, I'm ok if this cut requires an agent restart for now.
| @@ -1130,6 +1130,24 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass | |||
| * <a name="telemetry-disable_hostname"></a><a href="#telemetry-disable_hostname">`disable_hostname`</a> | |||
| This controls whether or not to prepend runtime telemetry with the machine's hostname, defaults to false. | |||
|
|
|||
| * <a name="telemetry-prefix_filter"></a><a href="#telemetry-prefix_filter">`prefix_filter`</a> | |||
There was a problem hiding this comment.
Can you add this to the reloadable config table at the bottom of this page?
| | `GET` | `/agent/metrics` | `application/json` | | ||
|
|
||
| This endpoint will dump the metrics for the most recent finished interval. | ||
| For more information about metrics, see the [telemetry](/docs/agent/telemetry.html) |
There was a problem hiding this comment.
Let's link the telemetry page back over here, too.
agent/http.go
Outdated
| @@ -262,6 +264,26 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque | |||
| } | |||
| } | |||
|
|
|||
| type handlerFunc func(resp http.ResponseWriter, req *http.Request) (interface{}, error) | |||
|
|
|||
| // requireAgentRead wraps the given function, requiring a token with agent read permissions | |||
There was a problem hiding this comment.
Since we don't have other places in mind that will use this, I'd probably just make an agent_endpoint.go:AgentMetrics() method that has this logic, so it looks like the others and that can just call in to s.agent.MemSink.DisplayMetrics rather than wrapping. Can you add a unit test for ACLs as well?
Also adds the
/v1/agent/metricsendpoint.Resolves #817.