Skip to content

Commit

Permalink
Fix issue that wrong config panice http metricsets
Browse files Browse the repository at this point in the history
When setting an invalide http config in a metricbeat module / metricset it could happen that the metricset paniced. The reason is that the error when unpacking the config was not properly returned and handled which lead to an empty http instance.

This PR changes the behaviour to return the errors which can then be handled by the metricsets. The issue also affects all metricsets which were based on the prometheus helper.

Closes #6192
  • Loading branch information
ruflin committed Jan 31, 2018
1 parent a02fc6d commit 69deda7
Show file tree
Hide file tree
Showing 43 changed files with 203 additions and 48 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
- Fix kafka OffsetFetch request missing topic and partition parameters. {pull}5880[5880]
- Change kubernetes.node.cpu.allocatable.cores to float. {pull}6130[6130]
- Fix system process metricset for kernel processes. {issue}5700[5700]
- Fix panic in http dependent modules when invalid config was used.

*Packetbeat*

Expand Down
10 changes: 5 additions & 5 deletions metricbeat/helper/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ type HTTP struct {
}

// NewHTTP creates new http helper
func NewHTTP(base mb.BaseMetricSet) *HTTP {
func NewHTTP(base mb.BaseMetricSet) (*HTTP, error) {
config := struct {
TLS *outputs.TLSConfig `config:"ssl"`
Timeout time.Duration `config:"timeout"`
Headers map[string]string `config:"headers"`
}{}
if err := base.Module().UnpackConfig(&config); err != nil {
return nil
return nil, err
}

if config.Headers == nil {
Expand All @@ -41,15 +41,15 @@ func NewHTTP(base mb.BaseMetricSet) *HTTP {

tlsConfig, err := outputs.LoadTLSConfig(config.TLS)
if err != nil {
return nil
return nil, err
}

var dialer, tlsDialer transport.Dialer

dialer = transport.NetDialer(config.Timeout)
tlsDialer, err = transport.TLSDialer(dialer, tlsConfig, config.Timeout)
if err != nil {
return nil
return nil, err
}

return &HTTP{
Expand All @@ -65,7 +65,7 @@ func NewHTTP(base mb.BaseMetricSet) *HTTP {
method: "GET",
uri: base.HostData().SanitizedURI,
body: nil,
}
}, nil
}

// FetchResponse fetches a response for the http metricset.
Expand Down
9 changes: 6 additions & 3 deletions metricbeat/helper/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ type Prometheus struct {
}

// NewPrometheusClient creates new prometheus helper
func NewPrometheusClient(base mb.BaseMetricSet) *Prometheus {
http := NewHTTP(base)
return &Prometheus{*http}
func NewPrometheusClient(base mb.BaseMetricSet) (*Prometheus, error) {
http, err := NewHTTP(base)
if err != nil {
return nil, err
}
return &Prometheus{*http}, nil
}

// GetFamilies requests metric families from prometheus endpoint and returns them
Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/apache/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,13 @@ type MetricSet struct {

// New creates new instance of MetricSet.
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
base,
helper.NewHTTP(base),
http,
}, nil
}

Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/ceph/cluster_disk/cluster_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The ceph cluster_disk metricset is beta")

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetHeader("Accept", "application/json")

return &MetricSet{
Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/ceph/cluster_health/cluster_health.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The ceph cluster_health metricset is beta")

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetHeader("Accept", "application/json")

return &MetricSet{
Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/ceph/cluster_status/cluster_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The ceph cluster_status metricset is beta")

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetHeader("Accept", "application/json")

return &MetricSet{
Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/ceph/monitor_health/monitor_health.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The ceph monitor_health metricset is beta")

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetHeader("Accept", "application/json")

return &MetricSet{
Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/ceph/osd_df/osd_df.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The ceph osd_df metricset is experimental")

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetHeader("Accept", "application/json")

return &MetricSet{
Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/ceph/osd_tree/osd_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The ceph osd_tree metricset is beta")

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetHeader("Accept", "application/json")

return &MetricSet{
Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/ceph/pool_disk/pool_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Experimental("The ceph pool_disk metricset is experimental")

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetHeader("Accept", "application/json")

return &MetricSet{
Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/couchbase/bucket/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The couchbase bucket metricset is beta")

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
BaseMetricSet: base,
http: helper.NewHTTP(base),
http: http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/couchbase/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The couchbase cluster metricset is beta")

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
BaseMetricSet: base,
http: helper.NewHTTP(base),
http: http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/couchbase/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The couchbase node metricset is beta")

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
BaseMetricSet: base,
http: helper.NewHTTP(base),
http: http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/dropwizard/collector/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
BaseMetricSet: base,
http: helper.NewHTTP(base),
http: http,
namespace: config.Namespace,
}, nil
}
Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/elasticsearch/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,13 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The elasticsearch node metricset is beta")

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
base,
helper.NewHTTP(base),
http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/elasticsearch/node_stats/node_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,13 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The elasticsearch node_stats metricset is beta")

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
base,
helper.NewHTTP(base),
http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/etcd/leader/leader.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
base,
helper.NewHTTP(base),
http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/etcd/self/self.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
base,
helper.NewHTTP(base),
http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/etcd/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
base,
helper.NewHTTP(base),
http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/golang/expvar/expvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
BaseMetricSet: base,
http: helper.NewHTTP(base),
http: http,
namespace: config.Namespace,
}, nil
}
Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/golang/heap/heap.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ type MetricSet struct {
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Experimental("The golang heap metricset is experimental")

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
BaseMetricSet: base,
http: helper.NewHTTP(base),
http: http,
}, nil
}

Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/http/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetMethod(config.Method)
http.SetBody([]byte(config.Body))

Expand Down
5 changes: 4 additions & 1 deletion metricbeat/module/jolokia/jmx/jmx.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

http := helper.NewHTTP(base)
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
http.SetMethod("POST")
http.SetBody(body)

Expand Down
7 changes: 6 additions & 1 deletion metricbeat/module/kibana/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ type MetricSet struct {
// New create a new instance of the MetricSet
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
cfgwarn.Beta("The kafka partition metricset is beta")

http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
base,
helper.NewHTTP(base),
http,
}, nil
}

Expand Down
6 changes: 5 additions & 1 deletion metricbeat/module/kubernetes/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,13 @@ type MetricSet struct {
// Part of new is also setting up the configuration by processing additional
// configuration entries if needed.
func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
http, err := helper.NewHTTP(base)
if err != nil {
return nil, err
}
return &MetricSet{
BaseMetricSet: base,
http: helper.NewHTTP(base),
http: http,
}, nil
}

Expand Down
Loading

0 comments on commit 69deda7

Please sign in to comment.