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

Cherry-pick #17913 to 7.x: [Metricbeat] allow partial region and zone in googlecloud module config #18015

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Apr 27, 2020

Cherry-pick of PR #17913 to 7.x branch. Original message:

What does this PR do?

This PR is to enable partial region and partial zone in googlecloud module config. User can choose to use wildcard * or not. For example:

- module: googlecloud
  metricsets:
    - compute
  region: "us-west"
  project_id: "elastic-observability"
  credentials_file_path: "your JSON credentials file path"
  exclude_labels: false
  period: 60s

This config will enable data collection from all regions start with us-west, which includes us-west1, us-west2, us-west3 and us-west4.

- module: googlecloud
  metricsets:
    - compute
  region: "us-*"
  project_id: "elastic-observability"
  credentials_file_path: "your JSON credentials file path"
  exclude_labels: false
  period: 60s

Similarly this config will enable data collection from all regions start with us-, which includes all regions starts with us-west, us-east and us-central.

Why is it important?

This will allow users to not copy paste region/zone names when they want to monitor several regions/zones. If user wants to monitor all regions in both US and Europe, they still need to copy paste but that's very minimum. For example:

- module: googlecloud
  metricsets:
    - compute
  region: "us-*"
  project_id: "elastic-observability"
  credentials_file_path: "your JSON credentials file path"
  exclude_labels: false
  period: 60s
- module: googlecloud
  metricsets:
    - compute
  region: "europe-*"
  project_id: "elastic-observability"
  credentials_file_path: "your JSON credentials file path"
  exclude_labels: false
  period: 60s

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

There are two parts need to be tested. One for regions and another for zones.

  • Test for regions:
    Enable googlecloud with this config and you should see metrics collection from all regions start with us-.
- module: googlecloud
  metricsets:
    - compute
  region: "us-"
  project_id: "elastic-observability"
  credentials_file_path: "your JSON credentials file path"
  exclude_labels: false
  period: 60s
  • Test for zones:
    Enable googlecloud with this config and you should see metrics collection from all zones start with europe-west3-, which includes europe-west3-a, europe-west3-b and europe-west3-c.
- module: googlecloud
  metricsets:
    - compute
  zone: "europe-west3-"
  project_id: "elastic-observability"
  credentials_file_path: "your JSON credentials file path"
  exclude_labels: false
  period: 60s

Related issues

…ig (#17913)

* allow partial region and zone in googlecloud module config
* add wildcard support in region and zone

(cherry picked from commit 1eb3032)
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 27, 2020
@kaiyan-sheng kaiyan-sheng self-assigned this Apr 27, 2020
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

Looks like there are some additional changes in metricbeat/docs/modules/googlecloud.asciidoc but they look like cleanup. Just wondering if you intend to make these same changes in master too — in that case, it might be better to remove them from this PR and backport them on their own?

@kaiyan-sheng
Copy link
Contributor Author

@ycombinator I'm wondering the same thing hmmm I tried to run mage fmt update in master branch but nothing got updated. But there is definitely a difference between metricbeat/docs/modules/googlecloud.asciidoc and x-pack/metricbeat/module/googlecloud/_meta/docs.asciidoc. Am I running the wrong command under elastic/beats/metricbeat?

@andresrc andresrc added [zube]: Inbox and removed needs_team Indicates that the issue/PR needs a Team:* label labels Apr 28, 2020
@ycombinator
Copy link
Contributor

I think you're running the right command. I'm guessing there is some difference in the doc toolchains between master and 7.x. Anyway the changes are cosmetic so I'd just go ahead and merge this.

@kaiyan-sheng kaiyan-sheng merged commit ec0d29e into elastic:7.x Apr 28, 2020
@kaiyan-sheng kaiyan-sheng deleted the backport_17913_7.x branch April 28, 2020 15:54
@zube zube bot added [zube]: Done and removed [zube]: Inbox labels Apr 28, 2020
@zube zube bot removed the [zube]: Done label Oct 13, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 13, 2020
@botelastic
Copy link

botelastic bot commented Oct 13, 2020

This pull request doesn't have a Team:<team> label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport needs_team Indicates that the issue/PR needs a Team:* label review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants