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

[RFC] Add host metric fields to ECS #950

Merged
merged 4 commits into from
Oct 13, 2020
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions rfcs/text/0005-host-metric-fields.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# 0005: Host Metric Fields
<!-- Leave this ID at 0000. The ECS team will assign a unique, contiguous RFC number upon merging the initial stage of this RFC. -->

- Stage: **0 (strawperson)** <!-- Update to reflect target stage. See https://elastic.github.io/ecs/stages.html -->
- Stage: **1 (proposal)** <!-- Update to reflect target stage. See https://elastic.github.io/ecs/stages.html -->
- Date: **2020-08-21** <!-- The ECS team sets this date at merge time. This is the date of the latest stage advancement. -->

<!--
Expand All @@ -13,7 +13,7 @@ We are proposing to add 7 new host fields into ECS for monitoring CPU, disk and
With existing `host.id` and `host.name`, these total 9 fields will become the common field schema for host metrics.

Proposed 7 new fields are:
* host.cpu.pct
* host.cpu.usage
* host.network.in.bytes
* host.network.in.packets
* host.network.out.bytes
Expand All @@ -26,6 +26,17 @@ Proposed 7 new fields are:
<!--
Stage 1: Describe at a high level how this change affects fields. Which fieldsets will be impacted? How many fields overall? Are we primarily adding fields, removing fields, or changing existing fields? The goal here is to understand the fundamental technical implications and likely extent of these changes. ~2-5 sentences.
-->
This RFC calls for the addition of host fields to collect basic monitoring metrics from a host or VM such as CPU, network and disk.

| field | type | description |
| --- | --- | --- |
| `host.cpu.usage` | scaled_float | Percent CPU used with scaling_factor of 1000. This value is normalized by the number of CPU cores and it ranges from 0 to 1. For example: For a two core host, this value should be the average of the 2 cores, between 0 and 1. |
| `host.network.in.bytes` | long | The number of bytes received on all network interfaces by the host in a given period of time. |
Copy link
Member

Choose a reason for hiding this comment

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

In the observer namespaces, ECS uses "ingress" and "egress". For consistency we might want to consider uses those terms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we document the nature of this counter. Is it a monotonic counter that is sometimes reset (e.g. system restart)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cyrille-leclerc These values will actually be gauges. For example, host.network.in.bytes will be the total bytes received aggregated among all network interfaces in 10 seconds. Next collection period, this value will change, might go up or down depends on the network usage of the next 10 seconds.

Copy link
Member

Choose a reason for hiding this comment

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

++ on aligning on ingress and egress

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks @kaiyan-sheng I was used to ever increasing counter this kind of metric but I don't know what's the state of the art today.
One benefit of ever increasing counters being that the collector can miss some collects without losing data.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with monotonic counters being superior here. Capturing rates may make it easier to render the data, however rates are lossy.

As Cyrille pointed out, counters are more resilient to an agent missing a beat (pun intended 😄). But counters are also superior for data rollups. Rolling up initial /10s metrics to /5m or hourly percentiles with a counter is trivial. With rates it's not so easy, as the basic piece of data is already an average. Not sure if we can do better in the Elastic Stack than the tool I was using in the past, though...

I'm not opposed to capturing rates if they're dramatically easier to work with in general, but I would like them to be accompanied with the counter backing them.

WDYT?

Copy link

@sorantis sorantis Sep 15, 2020

Choose a reason for hiding this comment

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

I think we're confusing two different things here.
The main purpose for this set of host metrics is to align the different resource providers, such as public cloud infrastructures, physical hosts, VMware clusters, other hypervisors, i.e. bring hosts/VMs to a common denominator. The purpose is not to capture high fidelity system metrics because some providers (AWS, GCP, Azure, etc.) simply don't provide these.
Granted, the Metricbeat system module can do much more than just the proposed CPU (that includes raw counters) metrics and should be used wherever possible. 
By making these metrics part of ECS we want improve user experience, i.e. we can always show basic health information about any hosts anywhere, and recommend users to enable the system module for enhanced details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@webmat @cyrille-leclerc Agree on the benefit of ever-increasing counters. We will definitely keep all the counters as they are right now from the system module. We only added extra calculation using these counters to get gauges in system module to match whatever we get from other resource providers such as AWS, Azure, and GCP. Unfortunately, they don't provide ever-increasing counters in their monitoring metrics as @sorantis mentioned above.

Also in the UI side, it's definitely easier to have metrics as gauges than counters.

| `host.network.in.packets` | long | The number of packets received on all network interfaces by the host in a given period of time. |
| `host.network.out.bytes` | long | The number of bytes sent out on all network interfaces by the host in a given period of time. |
Copy link
Member

Choose a reason for hiding this comment

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

We have in ES now _meta which also support byte as unit: elastic/elasticsearch#61941 Even though it is unfortunate that in older metrics we still have the postfix of .bytes, I think all new values we add we should skip the unit postfix and use _meta instead.

@webmat ECS should add support for _meta.

Copy link

Choose a reason for hiding this comment

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

This is a good point, and I think we should avoid postfixes just for the sake of making the unit explicit. In the case of system.network.out object (this metric comes from it) this is what we have today:

    "system": {
        "network": {
            "in": {
                "bytes": 37904869172,
                "dropped": 32,
                "errors": 0,
                "packets": 32143403
            }
        } 
     }

I wonder how we should differentiate all the different metrics there, specially between bytes and packets

Copy link
Member

Choose a reason for hiding this comment

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

Ha, interesting case. I'm tempted to say in this context the unit name might make sense. In any case, we should still add the info to _meta as this is what should be used by Kibana.

Alternative bytes could be called data but not sure if that is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Thank you @exekias and @ruflin. In this case, I still feel like bytes makes more sense in the metric name instead of data(too broad). bytes in this case, comparing with packets, is identifying the type of this value instead of only being the unit postfix.

| `host.network.out.packets` | long | The number of packets sent out on all network interfaces by the host in a given period of time. |
| `host.disk.read.bytes` | long | The total number of bytes read successfully in a given period of time. |
kaiyan-sheng marked this conversation as resolved.
Show resolved Hide resolved
| `host.disk.write.bytes` | long | The total number of bytes write successfully in a given period of time. |

<!--
Stage 2: Include new or updated yml field definitions for all of the essential fields in this draft. While not exhaustive, the fields documented here should be comprehensive enough to deeply evaluate the technical considerations of this change. The goal here is to validate the technical details for all essential fields and to provide a basis for adding experimental field definitions to the schema. Use GitHub code blocks with yml syntax formatting.
Expand All @@ -41,11 +52,23 @@ Stage 3: Add or update all remaining field definitions. The list should now be e
Stage 1: Describe at a high-level how these field changes will be used in practice. Real world examples are encouraged. The goal here is to understand how people would leverage these fields to gain insights or solve problems. ~1-3 paragraphs.
-->

These host metrics will be collected from different kinds of hosts such as bare
metal, virtual machines or virtual machines on public clouds like AWS, Azure and
GCP. These host metrics will be the standard minimal used in resource centric UI
views. For example, when user has VMs on bare metal, AWS and Azure, these host
fields will be collected from all VMs across all platforms and displayed in a
centralized location for better monitoring experience.

## Source data

<!--
Stage 1: Provide a high-level description of example sources of data. This does not yet need to be a concrete example of a source document, but instead can simply describe a potential source (e.g. nginx access log). This will ultimately be fleshed out to include literal source examples in a future stage. The goal here is to identify practical sources for these fields in the real world. ~1-3 sentences or unordered list.
-->
* Bare metal
* VMs
* AWS EC2 instances
* GCP compute engines
* Azure compute VMs
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if the following is applicable. But would we eventually want to capture the same metrics for containers as well?

If that's the case, perhaps we should consider defining this set of metrics independently, and make them nestable both under "host" and "container".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! We haven't got to container in inventory schema work yet. @exekias Can we potentially treat containers as it's own VMs and report the same set of host metrics?


<!--
Stage 2: Included a real world example source document. Ideally this example comes from the source(s) identified in stage 1. If not, it should replace them. The goal here is to validate the utility of these field changes in the context of a real world example. Format with the source name as a ### header and the example document in a GitHub code block with json formatting.
Expand All @@ -65,12 +88,28 @@ Stage 2: Identifies scope of impact of changes. Are breaking changes required? S
The goal here is to research and understand the impact of these changes on users in the community and development teams across Elastic. 2-5 sentences each.
-->

No breaking changes required.
These are new fields already added into Metricbeat:
* aws ec2 metricset
* googlecloud compute metricset
* azure compute_vm metricset

Only change would be once these fields are in ECS, we can remove these fields
from `metricbeat/_meta/fields.common.yml` file.

## Concerns

<!--
Stage 1: Identify potential concerns, implementation challenges, or complexity. Spend some time on this. Play devil's advocate. Try to identify the sort of non-obvious challenges that tend to surface later. The goal here is to surface risks early, allow everyone the time to work through them, and ultimately document resolution for posterity's sake.
-->

We need to carefully define each field because when these metrics are collected
from different platforms/services, the scope of these metrics change. We need to
make sure when users are using these metrics, they are all collected to represent
the same thing. For example, `host.network.in.bytes` needs to be an aggregated
value for all network interfaces. `host.cpu.pct` needs to be a normalized value
between 0 and 1.

<!--
Stage 2: Document new concerns or resolutions to previously listed concerns. It's not critical that all concerns have resolutions at this point, but it would be helpful if resolutions were taking shape for the most significant concerns.
-->
Expand All @@ -94,6 +133,8 @@ Stage 4: Identify at least one real-world, production-ready implementation that
The following are the people that consulted on the contents of this RFC.

* @kaiyan-sheng | author
* @cyrille-leclerc | sponsor
* @exekias | subject matter expert

<!--
Who will be or has consulted on the contents of this RFC? Identify authorship and sponsorship, and optionally identify the nature of involvement of others. Link to GitHub aliases where possible. This list will likely change or grow stage after stage.
Expand All @@ -117,6 +158,7 @@ e.g.:
<!-- An RFC should link to the PRs for each of it stage advancements. -->

* Stage 0: https://github.com/elastic/ecs/pull/947
* Stage 1: https://github.com/elastic/ecs/pull/950

<!--
* Stage 1: https://github.com/elastic/ecs/pull/NNN
Expand Down