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

Added cloud.id and cloud.auth config settings #4964

Merged
merged 9 commits into from
Aug 23, 2017
Merged

Conversation

tsg
Copy link
Contributor

@tsg tsg commented Aug 22, 2017

This adds two new configuration settings: cloud.id and cloud.auth.
They can be used in the configuration file, or from the CLI like this:

./metricbeat -e -E cloud.id=<cloud-id> -E cloud.auth=<cloud-auth>

The ES/KB settings are changed via overwritting, but we display an INFO
level message that the settings are changed by the cloud-id.

Closes #4959.

Left TODOs:

  • more tests
  • docs for the new settings
  • changelog

This adds two new configuration settings: `cloud.id` and `cloud.auth`.
They can be used in the configuration file, or from the CLI like this:

```
./metricbeat -e -E cloud.id=<cloud-id> -E cloud.auth=<cloud-auth>
```

The ES/KB settings are changed via overwritting, but we display an INFO
level message that the settings are changed by the cloud-id.

Closes elastic#4959.
@tsg tsg added in progress Pull request is currently in progress. needs_backport PR is waiting to be backported to other branches. review v6.0.0-beta2 labels Aug 22, 2017
@tsg tsg requested review from urso and removed request for urso August 22, 2017 08:38
@tsg
Copy link
Contributor Author

tsg commented Aug 22, 2017

@dedemorton I added short docs descriptions for the new settings in the Output section, together with an example, but it might make sense (for another PR) to have a dedicated "Getting started with Elastic Cloud" guide.

@tsg tsg removed the in progress Pull request is currently in progress. label Aug 22, 2017
@tsg tsg requested a review from urso August 22, 2017 10:03
@@ -595,6 +595,19 @@ filebeat.prospectors:
#processors:
#- add_docker_metadata: ~

#============================= Elastic Cloud ==================================

# These settings simplify using Beatname with the Elastic Cloud (https://cloud.elastic.co/).
Copy link
Contributor

Choose a reason for hiding this comment

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

It says Beatname in all files

errMsg string
}{
{
name: "cloud.auth specified by cloud.id not",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a typo here: s/by/but/

return err
}

err = cfg.SetChild("output.elasticsearch.hosts", -1, esURLConfig)
Copy link

Choose a reason for hiding this comment

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

before setting anything in output.elasticsearch.X, check the config is actually enabled/available via:

esConfig, _ := cfg.GetChild("output.elasticsearch")
esEnabled = esConfig.Enabled()

kibConfig, _ := cfg.GetChild("setup.kibana")
kibEnabled = kibConfig.Enabled()

By setting values on a non-present section, you will automatically enable the section (enabled always defaults to true).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed, I added a check that by auto-enabling the ES output we don't enable two outputs.

return errors.Errorf("The cloud.id setting enables the Elasticsearch output, but you already have the %s output enabled in the config", namespace.Name())
}
}

Copy link

Choose a reason for hiding this comment

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

this can be simplified to:

tmp := struct {
  Output common.ConfigNamespace `config:"output"`
}{}
if err := cfg.Unpack(&tmp); err != nil {
  return err
}
if out := tmp.Output; if out.IsSet() && out.Name() != "elasticsearch" {
  return errors.Errorf("...")
}

You can also capture the Kibana settings, in case we want to do some validations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I took your version.

@urso urso merged commit bd6981d into elastic:master Aug 23, 2017
ruflin added a commit to ruflin/apm-server that referenced this pull request Aug 23, 2017
Changes:
* Add support for cloud id. See elastic/beats#4964 for more details. This could simplify the onboard for users that use apm-server with the Cloud in the future.
* Be able to overwrite index pattern in the index template. See elastic/beats#4769 for more details. This allows to clean up our system tests.
ruflin added a commit to ruflin/apm-server that referenced this pull request Aug 23, 2017
Important changes / additions:

* Add support for cloud id. See elastic/beats#4964 for more details. This could simplify the onboard for users that use apm-server with the Cloud in the future.
* Be able to overwrite index pattern in the index template. See elastic/beats#4769 for more details. This allows to clean up our system tests.
@tsg tsg removed the needs_backport PR is waiting to be backported to other branches. label Aug 23, 2017
ruflin added a commit to ruflin/apm-server that referenced this pull request Aug 23, 2017
Important changes / additions:

* Add support for cloud id. See elastic/beats#4964 for more details. This could simplify the onboard for users that use apm-server with the Cloud in the future.
* Be able to overwrite index pattern in the index template. See elastic/beats#4769 for more details. This allows to clean up our system tests.
tsg added a commit to tsg/beats that referenced this pull request Aug 23, 2017
* Added cloud.id and cloud.auth config settings

This adds two new configuration settings: `cloud.id` and `cloud.auth`.
They can be used in the configuration file, or from the CLI like this:

```
./metricbeat -e -E cloud.id=<cloud-id> -E cloud.auth=<cloud-auth>
```

The ES/KB settings are changed via overwritting, but we display an INFO
level message that the settings are changed by the cloud-id.

Closes elastic#4959.

* goimports fix

* aded more tests

* added basic docs

* changelog

* addressed comments

* Check that we're not enabling a second output

* remove unused var

* Addressed comments

(cherry picked from commit bd6981d)
simitt pushed a commit to elastic/apm-server that referenced this pull request Aug 24, 2017
Important changes / additions:

* Add support for cloud id. See elastic/beats#4964 for more details. This could simplify the onboard for users that use apm-server with the Cloud in the future.
* Be able to overwrite index pattern in the index template. See elastic/beats#4769 for more details. This allows to clean up our system tests.
ruflin pushed a commit that referenced this pull request Aug 24, 2017
This adds two new configuration settings: `cloud.id` and `cloud.auth`.
They can be used in the configuration file, or from the CLI like this:

```
./metricbeat -e -E cloud.id=<cloud-id> -E cloud.auth=<cloud-auth>
```

The ES/KB settings are changed via overwritting, but we display an INFO
level message that the settings are changed by the cloud-id.

Closes #4959.

* goimports fix

* aded more tests

* added basic docs

* changelog

* addressed comments

* Check that we're not enabling a second output

* remove unused var

* Addressed comments

(cherry picked from commit bd6981d)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…#4976)

This adds two new configuration settings: `cloud.id` and `cloud.auth`.
They can be used in the configuration file, or from the CLI like this:

```
./metricbeat -e -E cloud.id=<cloud-id> -E cloud.auth=<cloud-auth>
```

The ES/KB settings are changed via overwritting, but we display an INFO
level message that the settings are changed by the cloud-id.

Closes elastic#4959.

* goimports fix

* aded more tests

* added basic docs

* changelog

* addressed comments

* Check that we're not enabling a second output

* remove unused var

* Addressed comments

(cherry picked from commit 3d32342)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants