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

Add host inventory metrics to googlecloud compute metricset #20391

Merged
merged 6 commits into from
Aug 20, 2020
Merged

Add host inventory metrics to googlecloud compute metricset #20391

merged 6 commits into from
Aug 20, 2020

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Jul 31, 2020

What does this PR do?

Why is it important?

This PR is to add proposed host common fields into compute metricset:

  • host.id
  • host.name
  • host.cpu.pct
  • host.network.in.bytes
  • host.network.in.packets
  • host.network.out.bytes
  • host.network.out.packets
  • host.disk.read.bytes
  • host.disk.write.bytes

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

  1. Start Metricbeat googlecloud module: ./metricbeat modules enable googlecloud
  2. Edit modules.d/googlecloud.yml to only include compute metricset:
- module: googlecloud
  metricsets:
    - compute
  region: "us-east1"
  project_id: elastic-observability
  credentials_file_path: "/Users/kaiyansheng/Downloads/elastic-observability-d17781618202.json"
  exclude_labels: false
  period: 300s

3.Change add_host_metadata processor config in metricbeat.yml file:

processors:
  - add_host_metadata:
      replace_fields: false
  1. Start metricbeat
  2. You should see metrics from compute metricset and includes fields listed above.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 31, 2020
@kaiyan-sheng kaiyan-sheng self-assigned this Jul 31, 2020
@kaiyan-sheng kaiyan-sheng added needs_backport PR is waiting to be backported to other branches. Team:Platforms Label for the Integrations - Platforms team labels Jul 31, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 31, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 31, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20391 updated]

  • Start Time: 2020-08-19T22:33:23.418+0000

  • Duration: 49 min 18 sec

Test stats 🧪

Test Results
Failed 0
Passed 843
Skipped 72
Total 915

},
"provider": "googlecloud"
},
"cloud.availability_zone": "us-central1-a",
"cloud.availability_zone": "asia-northeast1-b",
"cloud.region": "asia-northeast1",
Copy link
Member

@jsoriano jsoriano Aug 20, 2020

Choose a reason for hiding this comment

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

Nothing to change here, just thinking loud. It might be interesting to have cloud-agnostic common values for regions. So a user can more easily group metrics coming from asia-northeast1 in GCP, and ap-northeast-1 in AWS.

Imagine for example being able to ingest with something like this metadata:

"cloud.region": {
    "name": "asia-northeast1",
    "area": "asia-pacific",
    "country": "japan",
    "city": "tokyo",
}

And if it were ingested from AWS:

"cloud.region": {
    "name": "ap-northeast-1",
    "area": "asia-pacific",
    "country": "japan",
    "city": "tokyo",
}

Not sure if cloud providers have APIs for this information.

Though maybe for that we can add geodata through an ingest pipeline in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jsoriano Yeah good point! Different cloud has different names for regions. I don't see a specific API for GCP though but I think the geoip processor should be sufficient if we see good use cases in the future!

@kaiyan-sheng kaiyan-sheng merged commit cbb2f37 into elastic:master Aug 20, 2020
@kaiyan-sheng kaiyan-sheng added the test-plan Add this PR to be manual test plan label Aug 20, 2020
@kaiyan-sheng kaiyan-sheng added v7.10.0 and removed needs_backport PR is waiting to be backported to other branches. labels Aug 20, 2020
kaiyan-sheng added a commit that referenced this pull request Aug 24, 2020
…20724)

* Add host inventory metrics to googlecloud compute metricset

(cherry picked from commit cbb2f37)
@kaiyan-sheng kaiyan-sheng deleted the add_host_data_gcp branch September 16, 2020 15:04
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Oct 3, 2020
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
…20391)

* Add host inventory metrics to googlecloud compute metricset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Platforms Label for the Integrations - Platforms team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants