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

Move HTTP calls to Logstash from New() to Fetch() #15306

Merged
merged 5 commits into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix CPU count in docker/cpu in cases where no `online_cpus` are reported {pull}15070[15070]
- Fix mixed modules loading standard and light metricsets {pull}15011[15011]
- Make `kibana` module more resilient to Kibana unavailability. {issue}15258[15258] {pull}15270[15270]
- Make `logstash` module more resilient to Logstash unavailability. {issue}15276[15276] {pull}15306[15306]

*Packetbeat*

Expand Down
28 changes: 20 additions & 8 deletions metricbeat/module/logstash/logstash.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package logstash

import (
"encoding/json"
"fmt"
"net/url"

"github.com/pkg/errors"
Expand Down Expand Up @@ -158,8 +159,25 @@ func GetPipelines(m *MetricSet) ([]PipelineState, error) {
return pipelines, nil
}

// GetVersion returns the version of the Logstash node
func GetVersion(m *MetricSet) (*common.Version, error) {
// CheckPipelineGraphAPIsAvailable returns an error if pipeline graph APIs are not
// available in the version of the Logstash node.
func (m *MetricSet) CheckPipelineGraphAPIsAvailable() error {
logstashVersion, err := m.getVersion()
if err != nil {
return err
}

arePipelineGraphAPIsAvailable := elastic.IsFeatureAvailable(logstashVersion, PipelineGraphAPIsAvailableVersion)

if !arePipelineGraphAPIsAvailable {
const errorMsg = "the %v metricset with X-Pack enabled is only supported with Logstash >= %v. You are currently running Logstash %v"
return fmt.Errorf(errorMsg, m.FullyQualifiedName(), PipelineGraphAPIsAvailableVersion, logstashVersion)
}

return nil
}

func (m *MetricSet) getVersion() (*common.Version, error) {
const rootPath = "/"
content, err := fetchPath(m.HTTP, rootPath, "")
if err != nil {
Expand All @@ -178,12 +196,6 @@ func GetVersion(m *MetricSet) (*common.Version, error) {
return response.Version, nil
}

// ArePipelineGraphAPIsAvailable returns whether Logstash APIs that returns pipeline graphs
// are available in the given version of Logstash
func ArePipelineGraphAPIsAvailable(currentLogstashVersion *common.Version) bool {
return elastic.IsFeatureAvailable(currentLogstashVersion, PipelineGraphAPIsAvailableVersion)
}

func fetchPath(httpHelper *helper.HTTP, path string, query string) ([]byte, error) {
currentURI := httpHelper.GetURI()
defer httpHelper.SetURI(currentURI)
Expand Down
27 changes: 8 additions & 19 deletions metricbeat/module/logstash/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
package node

import (
"fmt"

"github.com/elastic/beats/metricbeat/mb"
"github.com/elastic/beats/metricbeat/mb/parse"
"github.com/elastic/beats/metricbeat/module/logstash"
Expand Down Expand Up @@ -58,23 +56,6 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

if ms.XPack {
logstashVersion, err := logstash.GetVersion(ms)
if err != nil {
return nil, err
}

arePipelineGraphAPIsAvailable := logstash.ArePipelineGraphAPIsAvailable(logstashVersion)
if err != nil {
return nil, err
}

if !arePipelineGraphAPIsAvailable {
const errorMsg = "The %v metricset with X-Pack enabled is only supported with Logstash >= %v. You are currently running Logstash %v"
return nil, fmt.Errorf(errorMsg, ms.FullyQualifiedName(), logstash.PipelineGraphAPIsAvailableVersion, logstashVersion)
}
}

return &MetricSet{
ms,
}, nil
Expand Down Expand Up @@ -106,3 +87,11 @@ func (m *MetricSet) Fetch(r mb.ReporterV2) error {

return nil
}

func (m *MetricSet) init() error {
if m.XPack {
return m.CheckPipelineGraphAPIsAvailable()
}

return nil
}
43 changes: 22 additions & 21 deletions metricbeat/module/logstash/node_stats/node_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
package node_stats

import (
"fmt"

"github.com/elastic/beats/metricbeat/mb"
"github.com/elastic/beats/metricbeat/mb/parse"
"github.com/elastic/beats/metricbeat/module/logstash"
Expand Down Expand Up @@ -59,25 +57,6 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
return nil, err
}

if ms.XPack {
logstashVersion, err := logstash.GetVersion(ms)
if err != nil {
return nil, err
}

arePipelineGraphAPIsAvailable := logstash.ArePipelineGraphAPIsAvailable(logstashVersion)
if err != nil {
return nil, err
}

if !arePipelineGraphAPIsAvailable {
const errorMsg = "The %v metricset with X-Pack enabled is only supported with Logstash >= %v. You are currently running Logstash %v"
return nil, fmt.Errorf(errorMsg, ms.FullyQualifiedName(), logstash.PipelineGraphAPIsAvailableVersion, logstashVersion)
}

ms.HTTP.SetURI(ms.HTTP.GetURI() + "?vertices=true")
}

return &MetricSet{
ms,
}, nil
Expand All @@ -87,6 +66,15 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
// It returns the event which is then forward to the output. In case of an error, a
// descriptive error must be returned.
func (m *MetricSet) Fetch(r mb.ReporterV2) error {
err := m.init()
if err != nil {
if m.XPack {
m.Logger().Error(err)
return nil
}
return err
}

content, err := m.HTTP.FetchContent()
if err != nil {
if m.XPack {
Expand All @@ -107,3 +95,16 @@ func (m *MetricSet) Fetch(r mb.ReporterV2) error {

return nil
}

func (m *MetricSet) init() error {
if m.XPack {
err := m.CheckPipelineGraphAPIsAvailable()
if err != nil {
return err
}

m.HTTP.SetURI(m.HTTP.GetURI() + "?vertices=true")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@ycombinator The two init function looks really close to me could it be possible to move them to the logstash package and use the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ph I extracted much of the common code into the logstash package. Please take a look at let me know what you think. Thanks!

return nil
}