Connect underlying ES client so ES version is set

[ycombinator]

Currently, if X-Pack Monitoring is setup for a Beat, the Elasticsearch monitoring code in libbeat tries to make a decision based on the Elasticsearch version over here:

beats/libbeat/monitoring/report/elasticsearch/client.go

Line 120 in 821baec

if c.es.GetVersion().Major < 7 {

However, it turns out that the Elasticsearch client (c.es) never initializes its version properly. As a result, c.es.GetVersion().Major will always return 0 (the zero-value for an int).

---

The reason for this is that the Monitoring Elasticsearch client's Connect() method does not attempt to determine the Elasticsearch cluster's version:

beats/libbeat/monitoring/report/elasticsearch/client.go

Lines 48 to 81 in 821baec

	func (c *publishClient) Connect() error {
	debugf("Monitoring client: connect.")
	params := map[string]string{
	"filter_path": "features.monitoring.enabled",
	}
	status, body, err := c.es.Request("GET", "/_xpack", "", params, nil)
	if err != nil {
	return fmt.Errorf("X-Pack capabilities query failed with: %v", err)
	}
	if status != 200 {
	return fmt.Errorf("X-Pack capabilities query failed with status code: %v", status)
	}
	resp := struct {
	Features struct {
	Monitoring struct {
	Enabled bool
	}
	}
	}{}
	if err := json.Unmarshal(body, &resp); err != nil {
	return err
	}
	if !resp.Features.Monitoring.Enabled {
	debugf("XPack monitoring is disabled.")
	return errNoMonitoring
	}
	debugf("XPack monitoring is enabled")
	return nil
	}

However, if you look at the Output Elasticsearch client's Connect() method, it does attempt to determine the Elasticsearch cluster's version:

beats/libbeat/outputs/elasticsearch/client.go

Lines 675 to 696 in 821baec

	// Connect connects the client. It runs a GET request against the root URL of
	// the configured host, updates the known Elasticsearch version and calls
	// globally configured handlers.
	func (conn *Connection) Connect() error {
	versionString, err := conn.Ping()
	if err != nil {
	return err
	}
	if version, err := common.NewVersion(versionString); err != nil {
	logp.Err("Invalid version from Elasticsearch: %v", versionString)
	conn.version = common.Version{}
	} else {
	conn.version = *version
	}
	err = conn.onConnectCallback()
	if err != nil {
	return fmt.Errorf("Connection marked as failed because the onConnect callback failed: %v", err)
	}
	return nil
	}

Additionally, it also calls any configured connection callbacks.

---

Since the Monitoring Elasticsearch client is based on the Output Elasticsearch client, this PR makes it so that the first thing the Monitoring Elasticsearch client's Connect() method does is call the underlying Output Elasticsearch client's Connect() method. That way the base Output Elasticsearch client can perform any generic connection-related duties before the Monitoring Elasticsearch client performs any Monitoring-specific ones.

Resolves #11204

[elasticmachine]

Pinging @elastic/stack-monitoring

[ycombinator]

An alternative solution here would be to extract the version-setting code from the Output Elasticsearch client's Connect() method into its own method. Then the Output Elasticsearch client's Connect() method as well as the Monitoring Elasticsearch client's Connect() method could both call this new version-setting method.

The pro of this alternative solution is that each client can pick and choose what additional duties it wants to perform as part of connecting to Elasticsearch.

The con is that if the Output Elasticsearch client's Connect() method decides to perform an additional duty, we would have to remember that the Monitoring Elasticsearch client is based on the Output Elasticsearch client and therefore might want to perform the same duty (or not).

[cachedout]

I'm in favor of the approach as it is presently implemented.

[ycombinator]

jenkins, test this