Make cloud.id work for monitoring Elasticsearch reporter

[ycombinator]

Resolves #14399.

This PR allows users to specify two new optional settings:

• monitoring.cloud.id. If specified, it's value will be decoded and used to override monitoring.elasticsearch.hosts.
• monitoring.cloud.auth. If specified, it's value will be decoded and used to override monitoring.elasticsearch.username and monitoring.elasticsearch.password.

Before this PR

The monitoring.elasticsearch.* settings were determined using the following precedence order:

• Is monitoring.elasticsearch.* set?
  • Yes: monitoring.elasticsearch.* will be used.
  • No:
    • Is cloud.* set?
      • Yes: cloud.* will be decoded and used to set elasticsearch.* and monitoring.elasticsearch.*.
      • No:
        • Is elasticsearch.* set?
          • Yes: It will be used to set monitoring.elasticsearch.*.
          • No: No monitoring.elasticsearch.* will be set.

After this PR

The monitoring.elasticsearch.* settings will be determined using the following precedence order (new settings and related precedence rules are bolded):

• Is monitoring.cloud.* set?
  • Yes: monitoring.cloud.* will be decoded and used to override monitoring.elasticsearch.*.
  • No:
    • Is monitoring.elasticsearch.* set?
      • Yes: monitoring.elasticsearch.* will be used.
      • No:
        • Is cloud.* set?
          • Yes: cloud.* will be decoded and used to set elasticsearch.* and monitoring.elasticsearch.*.
          • No:
            • Is elasticsearch.* set?
              • Yes: It will be used to set monitoring.elasticsearch.*.
              • No: No monitoring.elasticsearch.* will be set.

Important Note

The enhancement introduced by this PR has no impact on the deprecated xpack.monitoring.* settings. That is, even with this PR, users won't be able to set xpack.monitoring.cloud.* nor will setting monitoring.cloud.* cause xpack.monitoring.elasticsearch.* to be set or overridden.

Testing this PR

1. Create a deployment on cloud.elastic.co.
2. Grab the values for cloud.id and cloud.auth from your deployment.
3. Build a Beat, say Filebeat, with this PR.

   cd $GOPATH/src/github.com/elastic/beats/filebeat
   mage build
   

4. Configure your beat with the following monitoring settings:

   monitoring:
     enabled: true
     cloud:
       id: YOUR_CLOUD_ID
       auth: YOUR_CLOUD_AUTH

5. Make any other necessary configuration for the Beat to ingest some data.
6. Start the Beat.
7. Verify in your Cloud deployment's ES cluster that there's .monitoring-beats-* indices and documents in those indices.

[elasticmachine]

Pinging @elastic/stack-monitoring (Stack monitoring)

[urso]

If we start to expose this function, we should also try to make it more clear what is returned. Either by introducing a new type or naming the return values. I'd prefer a struct like:

type CloudIDDetails struct {
  ElasticsearchURL string
  KibanaURL string
}

[urso]

Hm... I guess these details are used for Elasticsearch and Kibana? We might want to add those to the struct as well?

alternatively one can hide the parsing behind a structure (lazy or eager parsing is possible)

type CloudID struct {
  id string
  username, password, esURL, kibURL string
}

func NewCloudID(id string) (*CloudID, error) {
  // parse id and fill structure
}

func (cid *CloudID) ID() string {
  return cid
}

func (cid *CloudID) Username() string {
  return cid.username
}

...

[urso]

If possible I'd prefer to not overwrite settings in a *common.Config. It's an anti-pattern we unfortunately use throughout Beats.

e.g. based on the CloudID structure one could introduce an interface type like:

type StackDetails interface {
  Username() string
  Password() string
  ElasticsearchURL() string
}

Then it's a matter of constructing the right object, instead of overwriting a shared configuration object that might already be applied partially.

I think for this PR it is ok as done, as monitoring settings are quite 'local'. But in general when dealing with configs we should prefer construction over modifications (treat *common.Config as readonly). It is a common issue that we sometimes did unpack a configuration before it has been modified.

[ycombinator]

++ to not overwriting settings, I was only doing it to be consistent with how we are handling the global cloud.id and cloud.auth settings.

[ycombinator]

@urso I like your suggestions to a) hide the cloud ID functionality behind a struct and methods and b) not mutate the shared*common.Config object directly. Much of the "bad" implementation comes from mirroring the existing implementation for parsing the global cloud.id and cloud.auth settings.

I'm going to cleanup the existing implementation in a separate refactoring-only PR first, and then use the cleaned up implementation in this PR.

[urso]

I'm going to cleanup the existing implementation in a separate refactoring-only PR first, and then use the cleaned up implementation in this PR.

I think we can progress here and cleanup both locations later at the same time. Amount of work shouldn't be much different, but no need to hold back this enhancement.

[ycombinator]

@urso This PR is ready for your review again. I think any further cleanup (to prevent overwriting settings) will not be trivial so we should defer it to a future PR. WDYT?

[ycombinator]

@urso I've addressed your latest round of feedback. This PR is ready for your review again. Thanks!

[ycombinator]

jenkins, test this

[kvch]

Opened issue: #15638