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.

Sign up for GitHub

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

Jump to bottom

Add Beta Ceph metrics module to Kibana Home #20223

Merged
merged 4 commits into from
Jul 16, 2018
Merged

Add Beta Ceph metrics module to Kibana Home #20223

merged 4 commits into from
Jul 16, 2018

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jun 26, 2018 •
edited by nreese
Loading

Add beta Ceph module to Metrics modules.

screen shot 2018-06-29 at 12 43 22 pm

screen shot 2018-06-29 at 12 43 30 pm

@ruflin ruflin mentioned this pull request Jun 26, 2018
Closed
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck thomasneirynck requested a review from nreese June 29, 2018 18:36
@nreese nreese added the Feature:Add Data Add Data and sample data feature on Home label Jun 29, 2018
nreese
nreese approved these changes Jun 29, 2018
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm
ran changes in Chrome and viewed Ceph Add Data page. Did not attempt to follow instructions and setup data flow

@nreese
Copy link
Contributor

nreese commented Jun 29, 2018

@gchaps Do you want to review?

@nreese nreese mentioned this pull request Jun 29, 2018
Closed
@gchaps
Copy link
Contributor

gchaps commented Jul 2, 2018

Read through the text. LGTM

Minor: Until the icon is ready, you could shift the text in the Ceph metrics box to the left on the Add Data page.

@ruflin
Copy link
Contributor Author

ruflin commented Jul 3, 2018

@gchaps Could you share on how I can do that?

@cchaos
Copy link
Contributor

cchaos commented Jul 3, 2018

@ruflin I believe if you just comment out this line: https://github.com/ruflin/kibana/blob/3976b57ea5cd762dccc69edeabc99a023ecb9053/src/core_plugins/kibana/server/tutorials/ceph_metrics/index.js#L34

It should layout that card without an icon since that icon does not actually exist yet.

@ruflin ruflin force-pushed the ceph-metrics-tutorial branch from 3976b57 to dc16c0f Compare July 4, 2018 14:29
@ruflin
Copy link
Contributor Author

ruflin commented Jul 4, 2018

@cchaos Thanks for your help. Change applied.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ruflin ruflin force-pushed the ceph-metrics-tutorial branch from dc16c0f to 97b778c Compare July 13, 2018 12:30
ruflin
ruflin commented Jul 13, 2018
View reviewed changes
src/core_plugins/kibana/server/tutorials/ceph_metrics/index.js
artifacts: {
application: {
label: 'Discover',
path: '/app/kibana#/discover'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tsg @alexfrancoeur I'm jumping here now to the discovery page. I was thinking if we should jump to discovery and already filter for the data from the module?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option is to leave that out and then there is simply no button at the end. But I think Discover is not bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this at first but it feels awkward as the user kind of gets stuck. Everything is complete, what now? Will move forward with discover for now and can still improve it in the future.

@ruflin
Copy link
Contributor Author

ruflin commented Jul 13, 2018

I rebased this PR now on top of #20514 to get the new check data feature and added a link to discovery. Please have a look again.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

tsg
tsg approved these changes Jul 13, 2018
View reviewed changes
Copy link
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

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

LGTM.

@ruflin ruflin merged commit 5111863 into elastic:master Jul 16, 2018
@ruflin ruflin deleted the ceph-metrics-tutorial branch July 16, 2018 06:21
@ruflin
Copy link
Contributor Author

ruflin commented Jul 16, 2018

@tsg Thanks a lot for your changes in #20514. Makes adding new modules even easier.

yankouskia pushed a commit to yankouskia/kibana that referenced this pull request Jul 16, 2018
@ruflin
Add Beta Ceph metrics module to Kibana Home (elastic#20223)
6b1c670 
<img width="2043" alt="screen shot 2018-06-29 at 12 43 22 pm" src="https://user-images.githubusercontent.com/373691/42109208-1aedc92e-7b9a-11e8-8c96-1c9be12a10d1.png">

<img width="2042" alt="screen shot 2018-06-29 at 12 43 30 pm" src="https://user-images.githubusercontent.com/373691/42109217-1f81ce40-7b9a-11e8-8eca-f3e8695b9e7f.png">
ruflin added a commit to ruflin/kibana that referenced this pull request Jul 19, 2018
@ruflin
Add Beta Ceph metrics module to Kibana Home (elastic#20223)
509996e 
<img width="2043" alt="screen shot 2018-06-29 at 12 43 22 pm" src="https://user-images.githubusercontent.com/373691/42109208-1aedc92e-7b9a-11e8-8c96-1c9be12a10d1.png">

<img width="2042" alt="screen shot 2018-06-29 at 12 43 30 pm" src="https://user-images.githubusercontent.com/373691/42109217-1f81ce40-7b9a-11e8-8eca-f3e8695b9e7f.png">
@ruflin ruflin mentioned this pull request Jul 19, 2018
Merged
ruflin added a commit that referenced this pull request Jul 19, 2018
@ruflin
Add Beta Ceph metrics module to Kibana Home (#20223) (#20973)
1e88f01 
<img width="2043" alt="screen shot 2018-06-29 at 12 43 22 pm" src="https://user-images.githubusercontent.com/373691/42109208-1aedc92e-7b9a-11e8-8c96-1c9be12a10d1.png">

<img width="2042" alt="screen shot 2018-06-29 at 12 43 30 pm" src="https://user-images.githubusercontent.com/373691/42109217-1f81ce40-7b9a-11e8-8eca-f3e8695b9e7f.png">
@kertal kertal mentioned this pull request Jan 15, 2020
Merged
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsg tsg tsg approved these changes

@nreese nreese nreese approved these changes

Assignees
No one assigned
Labels
Feature:Add Data Add Data and sample data feature on Home
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ruflin @elasticmachine @nreese @gchaps @cchaos @tsg @thomasneirynck