[Monitoring] Update missed logstash pages to EUI by chrisronline · Pull Request #27258 · elastic/kibana

chrisronline · 2018-12-14T22:15:28Z

This PR updates the logstash monitoring UI to update some missed work from #27064 in regards to the Logstash monitoring UI, specifically the pipeline and node UIs.

I'm not that familiar with these UIs so I'll probably need @justinkambic and/or @ycombinator to double check all of this.

elasticmachine · 2018-12-14T22:15:30Z

Pinging @elastic/stack-monitoring

elasticmachine · 2018-12-14T22:57:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

ycombinator · 2018-12-17T17:50:33Z

When I tried to load the Logstash Node Overview page, I got a blank page and an error in the browser's console log:

…h_eui_missed_parts

pickypg · 2018-12-17T18:09:13Z

How did this pass CI if that error is happening?

But I suspect this PR depends on #27253 noting that many of the changes appear to be in there.

chrisronline · 2018-12-17T18:09:56Z

@pickypg This PR was picked from that one but obviously missed a few things. Fixing up now

ycombinator · 2018-12-17T18:15:51Z

Not a result of this PR, but I noticed that the table on the Logstash Node Listing page is not sortable. Since you're fixing up things, mind fixing this too?

[UPDATE] If it helps, here's the original sorting logic:

kibana/x-pack/plugins/monitoring/public/directives/logstash/listing/index.js

Lines 26 to 34 in 3a8166c

    
           const columns = [ 
        
             { title: 'Name', sortKey: 'logstash.name', sortOrder: SORT_ASCENDING }, 
        
             { title: 'CPU Usage', sortKey: 'process.cpu.percent' }, 
        
             { title: 'Load Average', sortKey: 'os.cpu.load_average.1m', }, 
        
             { title: 'JVM Heap Used', sortKey: 'jvm.mem.heap_used_percent' }, 
        
             { title: 'Events Ingested', sortKey: 'events.out' }, 
        
             { title: 'Config Reloads' }, 
        
             { title: 'Version', sortKey: 'logstash.version' } 
        
           ];

…t base controller methods

elasticmachine · 2018-12-17T18:50:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

chrisronline · 2018-12-17T18:59:59Z

@ycombinator Done!

elasticmachine · 2018-12-17T19:11:52Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

ycombinator · 2018-12-18T18:22:49Z

Not sure if it's this PR or not, but the filtering on the Logstash Node Listing table does not seem to be working:

FWIW, filtering works on the Logstash Pipeline Listing table.

chrisronline · 2018-12-18T19:36:30Z

@ycombinator Hmm. Interesting. I'm investigating now

chrisronline · 2018-12-19T14:49:48Z

@ycombinator Definitely an issue. If it's okay with you, I'd like to address this in a later PR (which I'm working on now) as this affects all of the monitoring tables.

ycombinator · 2018-12-19T14:51:38Z

Definitely an issue. If it's okay with you, I'd like to address this in a later PR (which I'm working on now)...

I'm good with this being addressed in a follow up PR.

... as this affects all of the monitoring tables.

FWIW, I did not notice this issue on the Logstash Pipeline Listing table, which is also EUI?

chrisronline · 2018-12-19T14:54:48Z

@ycombinator I think it's hit or miss how each table might perform as the filtering/searching logic is somewhat complex and I'm not sure which situations will work and which won't. Therefore, for the follow up PR, I'm going to simplify it and write custom code to handle it in a predicable manner

ycombinator · 2018-12-19T14:58:00Z

Sounds good @chrisronline. Once you've rebased this PR to resolve the conflict, I'll review it once more. Ping me here when it's ready for review.

chrisronline · 2018-12-19T15:07:00Z

@ycombinator Done and ready for review

elasticmachine · 2018-12-19T15:41:35Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

chrisronline · 2018-12-19T16:04:44Z

x-pack/plugins/monitoring/public/views/logstash/pipeline/index.js

  const pipelineId = $route.current.params.id;
  const pipelineHash = $route.current.params.hash || '';
-  const url = `../api/monitoring/v1/clusters/${clusterUuid}/logstash/pipeline/${pipelineId}/${pipelineHash}`;
+  const url = pipelineHash


@ycombinator Bug fix here

chrisronline · 2018-12-19T16:24:03Z

@ycombinator FYI, the other PR #27504

ycombinator

I tested this PR functionally and it LGTM.

There are other issues that were found while testing this PR but they exist in master as well, therefore are not a result of this PR.

* Update logstash to EUI pages * Adding missed functionality * Ensure columns are sortable and update the nodes view to use the right base controller methods

chrisronline · 2018-12-21T16:36:22Z

Backport:
6.x: 73e66e2
6.6: 2358d9f

Update logstash to EUI pages

d571695

chrisronline added review v7.0.0 Team:Monitoring Stack Monitoring team v6.6.0 labels Dec 14, 2018

chrisronline self-assigned this Dec 14, 2018

Merge remote-tracking branch 'elastic/master' into monitoring/logstas…

1220577

…h_eui_missed_parts

Adding missed functionality

2f7bcb9

Ensure columns are sortable and update the nodes view to use the righ…

a1ce228

…t base controller methods

Merge in master

ddf6d87

chrisronline commented Dec 19, 2018

View reviewed changes

chrisronline mentioned this pull request Dec 19, 2018

[Monitoring] Fixing issue with EUI table filtering in monitoring UI #27504

Merged

ycombinator approved these changes Dec 19, 2018

View reviewed changes

chrisronline merged commit 8283ea5 into elastic:master Dec 19, 2018

chrisronline deleted the monitoring/logstash_eui_missed_parts branch December 19, 2018 18:14

chrisronline mentioned this pull request Dec 19, 2018

[6.x] [Monitoring] Update missed logstash pages to EUI (#27258) #27523

Merged

chrisronline added a commit to chrisronline/kibana that referenced this pull request Dec 21, 2018

Backport elastic#27258 and fix merge conflicts

133757e

chrisronline mentioned this pull request Dec 21, 2018

[6.6] [Monitoring] Update missed logstash pages to EUI (#27258) #27705

Merged

chrisronline added a commit that referenced this pull request Dec 21, 2018

Backport #27258 and fix merge conflicts (#27705)

2358d9f

ycombinator mentioned this pull request Jan 3, 2019

Monitoring Nodes 'Sort By' Not Working #27950

Closed

Conversation

chrisronline commented Dec 14, 2018

Uh oh!

elasticmachine commented Dec 14, 2018

Uh oh!

elasticmachine commented Dec 14, 2018

💚 Build Succeeded

Uh oh!

ycombinator commented Dec 17, 2018

Uh oh!

pickypg commented Dec 17, 2018

Uh oh!

chrisronline commented Dec 17, 2018

Uh oh!

ycombinator commented Dec 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Dec 17, 2018

💚 Build Succeeded

Uh oh!

chrisronline commented Dec 17, 2018

Uh oh!

elasticmachine commented Dec 17, 2018

💚 Build Succeeded

Uh oh!

ycombinator commented Dec 18, 2018

Uh oh!

chrisronline commented Dec 18, 2018

Uh oh!

chrisronline commented Dec 19, 2018

Uh oh!

ycombinator commented Dec 19, 2018

Uh oh!

chrisronline commented Dec 19, 2018

Uh oh!

ycombinator commented Dec 19, 2018

Uh oh!

chrisronline commented Dec 19, 2018

Uh oh!

elasticmachine commented Dec 19, 2018

💚 Build Succeeded

Uh oh!

chrisronline Dec 19, 2018

Choose a reason for hiding this comment

Uh oh!

chrisronline commented Dec 19, 2018

Uh oh!

ycombinator left a comment

Choose a reason for hiding this comment

Uh oh!

chrisronline commented Dec 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ycombinator commented Dec 17, 2018 •

edited

Loading

chrisronline commented Dec 21, 2018 •

edited

Loading