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 ec2 metricset #20171

Merged
merged 14 commits into from
Aug 18, 2020
Merged

Add host inventory metrics to ec2 metricset #20171

merged 14 commits into from
Aug 18, 2020

Conversation

kaiyan-sheng
Copy link
Contributor

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

What does this PR do?

This PR is to add proposed host common fields into ec2 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 AWS module: ./metricbeat modules enable aws
  2. Edit modules.d/aws.yml to only include ec2 metricset:
- module: aws
  period: 5m
  credential_profile_name: elastic-beats
  metricsets:
    - ec2
  1. 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 ec2 metricset and includes fields listed above.
    (host.name only equals to EC2 instance name when EC2 instance name is available.)

Event Example

{
    "@timestamp": "2017-10-12T08:05:34.853Z",
    "aws": {
        "ec2": {
            "cpu": {
                "credit_balance": 144,
                "credit_usage": 0.05477,
                "surplus_credit_balance": 0,
                "surplus_credits_charged": 0,
                "total": {
                    "pct": 1.166484417714406
                }
            },
            "diskio": {
                "read": {
                    "bytes": 0,
                    "bytes_per_sec": 0,
                    "count": 0,
                    "count_per_sec": 0
                },
                "write": {
                    "bytes": 0,
                    "bytes_per_sec": 0,
                    "count": 0,
                    "count_per_sec": 0
                }
            },
            "instance": {
                "core": {
                    "count": 1
                },
                "image": {
                    "id": "ami-04bc3da8f14823e88"
                },
                "monitoring": {
                    "state": "disabled"
                },
                "private": {
                    "dns_name": "ip-172-31-9-119.us-west-1.compute.internal",
                    "ip": "172.31.9.119"
                },
                "public": {
                    "dns_name": "ec2-13-52-163-56.us-west-1.compute.amazonaws.com",
                    "ip": "13.52.163.56"
                },
                "state": {
                    "code": 16,
                    "name": "running"
                },
                "threads_per_core": 1
            },
            "network": {
                "in": {
                    "bytes": 6644.4,
                    "bytes_per_sec": 22.148,
                    "packets": 44.6,
                    "packets_per_sec": 0.14866666666666667
                },
                "out": {
                    "bytes": 6016.6,
                    "bytes_per_sec": 20.055333333333333,
                    "packets": 35.4,
                    "packets_per_sec": 0.118
                }
            },
            "status": {
                "check_failed": 0,
                "check_failed_instance": 0,
                "check_failed_system": 0
            }
        },
        "tags": {
            "Name": "mysql-test",
            "created-by": "ks"
        }
    },
    "cloud": {
        "account": {
            "id": "428152502467",
            "name": "elastic-beats"
        },
        "availability_zone": "us-west-1b",
        "instance": {
            "id": "i-0516ddaca5c1d231f",
            "name": "mysql-test"
        },
        "machine": {
            "type": "t2.micro"
        },
        "provider": "aws",
        "region": "us-west-1"
    },
    "event": {
        "dataset": "aws.ec2",
        "duration": 115000,
        "module": "aws"
    },
    "host": {
        "cpu": {
            "pct": 1.166484417714406
        },
        "disk": {
            "read": {
                "bytes": 0
            }
        },
        "diskio": {
            "write": {
                "bytes": 0
            }
        },
        "id": "i-0516ddaca5c1d231f",
        "name": "mysql-test",
        "network": {
            "in": {
                "bytes": 6644.4,
                "packets": 44.6
            },
            "out": {
                "bytes": 6016.6,
                "packets": 35.4
            }
        }
    },
    "metricset": {
        "name": "ec2",
        "period": 10000
    },
    "service": {
        "type": "aws"
    }
}

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 22, 2020
@kaiyan-sheng kaiyan-sheng self-assigned this Jul 22, 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 22, 2020
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 22, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 22, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20171 updated]

  • Start Time: 2020-08-18T16:35:20.865+0000

  • Duration: 69 min 42 sec

Test stats 🧪

Test Results
Failed 0
Passed 3541
Skipped 794
Total 4335

@kaiyan-sheng kaiyan-sheng marked this pull request as ready for review July 23, 2020 22:09
@elasticmachine
Copy link
Collaborator

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

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

This is looking good.

x-pack/metricbeat/module/aws/ec2/ec2.go Outdated Show resolved Hide resolved
Comment on lines 116 to 117
"bytes": 6644.4,
"packets": 44.6
Copy link
Member

Choose a reason for hiding this comment

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

Is this a counter? Why is it a decimal number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a gauge actually and it's an average value among 5 minutes:

Screen Shot 2020-07-31 at 8 36 46 AM

CHANGELOG.next.asciidoc Show resolved Hide resolved
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

There are some issues with documented fields, for the rest it is looking good!

metricbeat/docs/fields.asciidoc Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/_meta/fields.yml Outdated Show resolved Hide resolved
@jsoriano jsoriano dismissed their stale review August 18, 2020 16:23

Issues fixed

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

LGTM if CI gets green.

@kaiyan-sheng kaiyan-sheng merged commit 54d2268 into elastic:master Aug 18, 2020
@kaiyan-sheng kaiyan-sheng deleted the add_host_data branch August 18, 2020 18:20
@kaiyan-sheng kaiyan-sheng added v7.10.0 and removed needs_backport PR is waiting to be backported to other branches. labels Aug 18, 2020
@sorantis
Copy link
Contributor

@kaiyan-sheng, @jsoriano, in the following configuration

processors:
  - add_host_metadata:
      replace_fields: false

what does the replace_fields flag do?

@jsoriano
Copy link
Member

@kaiyan-sheng, @jsoriano, in the following configuration

processors:
  - add_host_metadata:
      replace_fields: false

what does the replace_fields flag do?

It can be used to control if add_host_metadata should replace existing host fields. It defaults to true, so this processor overwrites existing host fields, and can be set to false to avoid this overwrite, that is what we are going to want with inventory fields added by modules. We added this option to support inventory fields, while avoiding the introduction of a breaking change by now.

Full discussion and change to add this flag can be found here: #20490

@sorantis
Copy link
Contributor

sorantis commented Aug 20, 2020

ok, my expectation was that this field would either map or remove the respective ec2.* fields in favor of host.* fields.
Because the resulting document contains duplicate metrics (e.g.: ec2.cpu.total.pct = host.cpu.pct), which increases the doc size by 10-11%

kaiyan-sheng added a commit that referenced this pull request Aug 20, 2020
* Add host inventory metrics to ec2 metricset
* move host fields into metricbeat/_meta/fields.common.yml

(cherry picked from commit 54d2268)
@kaiyan-sheng
Copy link
Contributor Author

@sorantis Currently we are duplicating the fields so yes we have both ec2.* fields and host.* fields. But I'm planning to create a PR to remove the duplicated ec2.* fields and use alias to keep backward compatibility. For example:

- name: aws.ec2
  type: group
  fields:
    - name: cpu.total.pct
      type: alias
      path: host.cpu.pct
      migration: true
    - name: network.in.bytes
      type: alias
      path: host.network.in.bytes
      migration: true

@sorantis
Copy link
Contributor

Great! Thanks @kaiyan-sheng .

melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
* Add host inventory metrics to ec2 metricset
* move host fields into metricbeat/_meta/fields.common.yml
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 v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants