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] Use sum for aggregating AWS metrics. #43293

Merged
merged 5 commits into from
Aug 16, 2019

Conversation

skh
Copy link
Contributor

@skh skh commented Aug 14, 2019

Summary

Implements changes to the processing of AWS metrics as discussed in #42687 (comment)

This can be tested against the Observability 8.x.x cluster, which has metrics for one AWS instance.

@skh skh added Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes v7.4.0 labels Aug 14, 2019
@skh skh requested a review from a team as a code owner August 14, 2019 16:46
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-logs-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@skh
Copy link
Contributor Author

skh commented Aug 15, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@simianhacker simianhacker self-requested a review August 15, 2019 14:40
Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

Per our discussion here are the changes to the models: simianhacker@9108e44

I tried to push it to your branch but for some reason I don't have permissions to do so.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@skh
Copy link
Contributor Author

skh commented Aug 16, 2019

@simianhacker ready for another look.

@simianhacker simianhacker self-requested a review August 16, 2019 15:01
Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

Looks like there is a typo in the network ID as noted below. I also forgot to put a positive_only after the derivative metric. Here is an example of what that looks like:

{
  id: 'posonly-deriv-csum-sum-net-out',
  field: 'deriv-csum-sum-net-out',
  type: InfraMetricNodeMetricType.positive_only,
}

We need to put one of those after each derivative. I'm sorry I missed that but it occurred to me while I was debugging the missing data for the network in.

Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

Looks good!

          __         ______     ______   __    __    
         /\ \       /\  ___\   /\__  _\ /\ "-./  \   
         \ \ \____  \ \ \__ \  \/_/\ \/ \ \ \-./\ \  
          \ \_____\  \ \_____\    \ \_\  \ \_\ \ \_\ 
           \/_____/   \/_____/     \/_/   \/_/  \/_/ 
                                                     

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@skh skh merged commit 2110b9c into elastic:master Aug 16, 2019
skh added a commit to skh/kibana that referenced this pull request Aug 16, 2019
* Use sum for aggregations.

* Use cumulative sum/derivative instead of hard coded rate

* Fix typo.

* Add positive_only after derivatives.
skh added a commit that referenced this pull request Aug 16, 2019
* Use sum for aggregations.

* Use cumulative sum/derivative instead of hard coded rate

* Fix typo.

* Add positive_only after derivatives.
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 19, 2019
…_update_json_spec

* 'master' of github.com:elastic/kibana: (35 commits)
  fix: 🐛 pass whole action context to isCompatible() method (elastic#43457)
  Deleted old kbn-top-nav directive (elastic#43168)
  [ML] Fixing cloning of single metric distinct count job (elastic#43435)
  Update @elastic/charts version 8.1.6 > 9.1.1 (elastic#43516)
  [Inspector Views] [Request View] - Migrate inspector_views to new platform (elastic#43191)
  [ML] Adding loading indicators to all wizard charts (elastic#43382)
  disable flaky test (elastic#43492)
  feature(code/frontend): cancel file blob and directory commits request if outdated (elastic#43348)
  fix(code/frontend): button group url should have previous query string (elastic#43428)
  [SIEM] Fixes index substring incorrectly matching configured indices and failing to install ML job (elastic#43409)
  [SIEM] Adds performance enhancements such by removing wasted renderers and adding incremental DOM rendering (elastic#43157)
  disable flaky test (elastic#37859)
  Added sass lint to Canvas (elastic#43410)
  [Maps] add indicator when layer is filtered by search bar (elastic#43283)
  Properly validate current user password during password change. (elastic#43447)
  Spaces - allow for hex color codes that include uppercase characters (elastic#43470)
  [Reporting] Add a bit more logging and a few more logging level promotions (elastic#43415)
  Partially convert index pattern server to typescript (elastic#43291)
  [Infra UI] Use sum for aggregating AWS metrics. (elastic#43293)
  [SIEM] Format bytes columns in timeline (elastic#43147)
  ...
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:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants