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

[Infra UI] Add cloud metrics and cloud/host info to metadata endpoint #41836

Merged
merged 21 commits into from
Aug 2, 2019

Conversation

simianhacker
Copy link
Member

@simianhacker simianhacker commented Jul 23, 2019

Summary

This PR closes #39280 by adding a nodes cloud/host details to the metadata. Then using cloud.instance.id to query for cloud metrics event.dataset. For Kubernetes pods, we first have to find the kubernetes.node.name then use that to get the host's cloud/host details because the host/cloud data attached to the Kubernetes metrics are the host that reported the metric which is not always the host where the pod lives.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@simianhacker simianhacker added the Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services label Jul 26, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-logs-ui

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@simianhacker simianhacker marked this pull request as ready for review July 26, 2019 18:29
@simianhacker simianhacker requested a review from a team as a code owner July 26, 2019 18:29
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@simianhacker
Copy link
Member Author

@skh Refactor is done, wanna take another 👀

@skh
Copy link
Contributor

skh commented Aug 1, 2019

This is a way larger refactor than I was thinking about, I meant only getting rid of the separate adapter and domain parts, but 👍 👍 for cleaning away a piece of GraphQL!

Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

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

First, great effort to get away from GraphQL for one more endpoint! 👍

I have commented inline with a few questions.

In general, I see that we are following two different patterns in the HTTP API endpoints that are currently being added. One is everything related to logs, the other is the metric explorer endpoint and now this one. The differences are:

  • route and types below common or below server
  • using io-ts vs. not
  • using ui/kfetch vs our own fetch helper

This worries me a bit, because we're introducing more variety even as we are still busy cleaning up older tech debt. What do think?


I've tested this by opening the node detail page for the AWS instance in the Observability-8.x.x cluster. The response looks good:

{
    "features": [
        {
            "name": "system.process",
            "source": "metrics"
        },
        {
            "name": "system.network",
            "source": "metrics"
        },
        {
            "name": "system.cpu",
            "source": "metrics"
        },
        {
            "name": "system.load",
            "source": "metrics"
        },
        {
            "name": "system.memory",
            "source": "metrics"
        },
        {
            "name": "system.process.summary",
            "source": "metrics"
        },
        {
            "name": "system.socket.summary",
            "source": "metrics"
        },
        {
            "name": "system.filesystem",
            "source": "metrics"
        },
        {
            "name": "system.fsstat",
            "source": "metrics"
        },
        {
            "name": "system.uptime",
            "source": "metrics"
        },
        {
            "name": "system.process",
            "source": "metrics"
        },
        {
            "name": "system.network",
            "source": "metrics"
        },
        {
            "name": "system.cpu",
            "source": "metrics"
        },
        {
            "name": "system.load",
            "source": "metrics"
        },
        {
            "name": "system.memory",
            "source": "metrics"
        },
        {
            "name": "system.process.summary",
            "source": "metrics"
        },
        {
            "name": "system.socket.summary",
            "source": "metrics"
        },
        {
            "name": "system.filesystem",
            "source": "metrics"
        },
        {
            "name": "system.fsstat",
            "source": "metrics"
        },
        {
            "name": "aws.ec2",
            "source": "metrics"
        },
        {
            "name": "system.uptime",
            "source": "metrics"
        }
    ],
    "id": "ip-172-31-47-9.us-east-2.compute.internal",
    "info": {
        "cloud": {
            "account": {
                "id": "015351775590"
            },
            "availability_zone": "us-east-2c",
            "image": {
                "id": "ami-0d8f6eb4f641ef691"
            },
            "instance": {
                "id": "i-011454f72559c510b"
            },
            "machine": {
                "type": "t2.micro"
            },
            "provider": "aws",
            "region": "us-east-2"
        },
        "host": {
            "architecture": "x86_64",
            "containerized": false,
            "hostname": "ip-172-31-47-9.us-east-2.compute.internal",
            "id": "ded64cbff86f478990a3dfbb63a8d238",
            "name": "ip-172-31-47-9.us-east-2.compute.internal",
            "os": {
                "codename": "Karoo",
                "family": "redhat",
                "kernel": "4.14.123-111.109.amzn2.x86_64",
                "name": "Amazon Linux",
                "platform": "amzn",
                "version": "2"
            }
        }
    },
    "name": "ip-172-31-47-9.us-east-2.compute.internal"
}

The response for the monitoring host, however, also lists aws.ec2 as available metricset, though I believe it shouldn't:

{
    "features": [
        {
            "name": "system.process",
            "source": "metrics"
        },
        {
            "name": "system.network",
            "source": "metrics"
        },
        {
            "name": "system.cpu",
            "source": "metrics"
        },
        {
            "name": "system.load",
            "source": "metrics"
        },
        {
            "name": "system.memory",
            "source": "metrics"
        },
        {
            "name": "system.process.summary",
            "source": "metrics"
        },
        {
            "name": "system.socket.summary",
            "source": "metrics"
        },
        {
            "name": "system.filesystem",
            "source": "metrics"
        },
        {
            "name": "system.fsstat",
            "source": "metrics"
        },
        {
            "name": "aws.ec2",
            "source": "metrics"
        },
        {
            "name": "system.uptime",
            "source": "metrics"
        },
        {
            "name": "system.process",
            "source": "metrics"
        },
        {
            "name": "system.network",
            "source": "metrics"
        },
        {
            "name": "system.cpu",
            "source": "metrics"
        },
        {
            "name": "system.load",
            "source": "metrics"
        },
        {
            "name": "system.memory",
            "source": "metrics"
        },
        {
            "name": "system.process.summary",
            "source": "metrics"
        },
        {
            "name": "system.socket.summary",
            "source": "metrics"
        },
        {
            "name": "system.filesystem",
            "source": "metrics"
        },
        {
            "name": "system.fsstat",
            "source": "metrics"
        },
        {
            "name": "system.uptime",
            "source": "metrics"
        }
    ],
    "id": "skh-aws-monitoring-host",
    "info": {
        "cloud": {
            "availability_zone": "europe-west3-c",
            "instance": {
                "id": "1872939573891739009",
                "name": "skh-aws-monitoring-host"
            },
            "machine": {
                "type": "f1-micro"
            },
            "project": {
                "id": "elastic-observability"
            },
            "provider": "gcp"
        },
        "host": {
            "architecture": "x86_64",
            "containerized": false,
            "hostname": "skh-aws-monitoring-host",
            "id": "6d984a37b87a34f73f0c46063a439980",
            "name": "skh-aws-monitoring-host",
            "os": {
                "codename": "stretch",
                "family": "debian",
                "kernel": "4.9.0-9-amd64",
                "name": "Debian GNU/Linux",
                "platform": "debian",
                "version": "9 (stretch)"
            }
        }
    },
    "name": "skh-aws-monitoring-host"
}

The monitoring host is correctly reporting cloud metadata for itself, because it is also running in the cloud (on a GCP instance), but for this host there should not be any aws.ec2 metadata being reported as available, as it only sends aws.ec2 data for other hosts.

* you may not use this file except in compliance with the Elastic License.
*/

import { boomify } from 'boom';
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not part of this change, but I notice that we require a different version of boom than the main Kibana package.json. Is there a reason for that (both that we need a newer one and that the version couldn't be bumped for all of Kibana)?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was updated by @spalger via #37555, maybe he can explain?

@simianhacker
Copy link
Member Author

@skh Looks like we need to filter out the event.dataset for aws.ec2 from getMetricMetadata and only add it in getCloudMetricsMetadata.

With regards to how we are doing the HTTP endpoints I just followed what I did for Metrics Explorer. I can move this one over to follow the conventions above. Then file a separate PR to convert Metrics Explorer so they are all consistent.

@simianhacker
Copy link
Member Author

@skh It looks like we are using Joi to validate the incoming HTTP request in all the Logging API's as well. I prefer Joi for that part since it integrates nicely with Hapi. You can just give it a schema and if it finds a mismatch it returns a very nicely formatted error.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@simianhacker simianhacker requested a review from skh August 1, 2019 23:51
@skh
Copy link
Contributor

skh commented Aug 2, 2019

@skh Looks like we need to filter out the event.dataset for aws.ec2 from getMetricMetadata and only add it in getCloudMetricsMetadata.

This works as expected now 👍

Copy link
Contributor

@skh skh left a comment

Choose a reason for hiding this comment

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

Thanks for the changes towards better consistency throughout Logs & Infrastructure. Some of my questions were based on Felix' work in a draft PR (#42356 ) -- apologies if those sounded more like requests than questions, and thanks for considering them anyway.

@simianhacker simianhacker merged commit cf21757 into elastic:master Aug 2, 2019
simianhacker added a commit to simianhacker/kibana that referenced this pull request Aug 2, 2019
…elastic#41836)

* [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint

* Adding cloud metrics

* Correcting the pod host/cloud info

* Adding tests to metadata for new cloud/host info

* Fixing test to include machine.type

* Adding aws test data

* Refactor metadata container into hook

* Functionally complete

* updating tests

* Removing Metadata GraphQL endpoint and supporting files

* Moving types under common/http_api and prefixing with Infra

* adding filter for aws.ec2 dataset

* move away from fetch to useHTTPRequest

* Add decode function to useHTTPRequest; rename data to response;

* Changing from Typescript types to IO-TS types and adding checks at client and server
simianhacker added a commit to simianhacker/kibana that referenced this pull request Aug 2, 2019
- Adds APM feature to metadata
- Closes elastic#42167
- Based on elastic#41836
simianhacker added a commit that referenced this pull request Aug 2, 2019
…dpoint (#41836) (#42536)

* [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (#41836)

* [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint

* Adding cloud metrics

* Correcting the pod host/cloud info

* Adding tests to metadata for new cloud/host info

* Fixing test to include machine.type

* Adding aws test data

* Refactor metadata container into hook

* Functionally complete

* updating tests

* Removing Metadata GraphQL endpoint and supporting files

* Moving types under common/http_api and prefixing with Infra

* adding filter for aws.ec2 dataset

* move away from fetch to useHTTPRequest

* Add decode function to useHTTPRequest; rename data to response;

* Changing from Typescript types to IO-TS types and adding checks at client and server

* Fixing field type
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 5, 2019
…-or-edit-existing-rollup-job

* 'master' of github.com:elastic/kibana: (67 commits)
  [TSVB] Shim new platform (elastic#39169)
  [Metric Vis] Shim new platform (elastic#42240)
  [Tag Cloud] Shim new platform (elastic#42348)
  Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS.
  Add disk space percentage to node listing (elastic#42145)
  [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440)
  Document HTTP service (elastic#42331)
  [Reporting] Sanitize 409 error log message (elastic#42495)
  [docs][skip ci] Maps read only access (elastic#35561)
  [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407)
  [DOCS] Updates images and content in Dashboard docs (elastic#42500)
  Allow sorting on multiple columns in Discover (elastic#41918)
  [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287)
  [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534)
  Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298)
  Increase max-old-space-size for builds (elastic#42218)
  [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836)
  [Logs UI][a11y] Announce name of column on remove column button (elastic#41695)
  Inspector 👉 New Platform (elastic#42164)
  Make alerting properly space aware (elastic#42081)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 5, 2019
…s_autocomplete

* 'master' of github.com:elastic/kibana: (189 commits)
  [TSVB] Shim new platform (elastic#39169)
  [Metric Vis] Shim new platform (elastic#42240)
  [Tag Cloud] Shim new platform (elastic#42348)
  Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS.
  Add disk space percentage to node listing (elastic#42145)
  [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440)
  Document HTTP service (elastic#42331)
  [Reporting] Sanitize 409 error log message (elastic#42495)
  [docs][skip ci] Maps read only access (elastic#35561)
  [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407)
  [DOCS] Updates images and content in Dashboard docs (elastic#42500)
  Allow sorting on multiple columns in Discover (elastic#41918)
  [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287)
  [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534)
  Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298)
  Increase max-old-space-size for builds (elastic#42218)
  [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836)
  [Logs UI][a11y] Announce name of column on remove column button (elastic#41695)
  Inspector 👉 New Platform (elastic#42164)
  Make alerting properly space aware (elastic#42081)
  ...
simianhacker added a commit to simianhacker/kibana that referenced this pull request Aug 14, 2019
* [Infra UI] Add APM to Metadata Endpoint

- Adds APM feature to metadata
- Closes elastic#42167
- Based on elastic#41836

* migrating to new http api
simianhacker added a commit that referenced this pull request Aug 14, 2019
* [Infra UI] Add APM to Metadata Endpoint

- Adds APM feature to metadata
- Closes #42167
- Based on #41836

* migrating to new http api
simianhacker added a commit that referenced this pull request Aug 14, 2019
* [Infra UI] Add APM to Metadata Endpoint

- Adds APM feature to metadata
- Closes #42167
- Based on #41836

* migrating to new http api
@simianhacker simianhacker deleted the fixes-39280 branch April 17, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Infra UI] Change metadata endpoint to check for cloud metrics
4 participants