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

Re-design benchmarks webpage #3447

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rishabh6788
Copy link
Contributor

@rishabh6788 rishabh6788 commented Nov 13, 2024

Description

The current benchmarks webpage is single page that has details of what it shows and hyperlinks to different workload performance metrics dashboards that render the iframe when clicked upon.
The hyperlinks are horizontally stacked and clutter the space as more and more dashboards are getting added.

This PR moves all the horizontal hyperlinks to be vertically stacked in the left column of the page and also generates dedicated page for each workload's metric dashboards. The user can click on the desired workload's dashboard hyperlink which will then render the data from iframe.

benchmarks.mov

Issues Resolved

[List any issues this PR will resolve]

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the BSD-3-Clause License.

Signed-off-by: Rishabh Singh <[email protected]>
@rishabh6788
Copy link
Contributor Author

rishabh6788 commented Nov 13, 2024

@AMoo-Miki @ashwin-pc Please review the PR and let me know if anything can be further improved.

@rishabh6788
Copy link
Contributor Author

Commenting for a review.

<h2>Why publish performance data?</h2>
<p>
The OpenSearch Project benchmarks the performance of OpenSearch releases to measure performance stability and gather data to inform software development. As an open-source project, we make this information publicly available in order to share it with the OpenSearch community.
</p>

<h2>Join the discussion</h2>
<p>
Questions or contributions? Connect with the OpenSearch community in the <a href="https://app.slack.com/client/T01QQ0Q5GMA/C0516H8EJ7R">#performance</a> channel on our public Slack.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line should probably link to https://opensearch.org/slack.html. This link didn't work for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@@ -64,66 +49,3 @@ benchmark_height_mobile: 6000
<p>
To learn more about OpenSearch performance improvements through version 2.14, read <a href="https://opensearch.org/blog/opensearch-performance-2.14/">this blog post</a>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This link should dynamically use the current version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This points to the last published performance blog. We are in the process of publishing a new blog, will updated once it is up.

benchmark_domain: 'benchmarks.opensearch.org'
---

<iframe src="https://{{ page.benchmark_domain }}/app/dashboards#/view/1a369150-e232-11ee-addb-f72c2feaa994?embed=true&_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-1w,to:now))&_a=(description:'',filters:!(('$state':(store:appState),meta:(alias:!n,disabled:!f,index:b4c18ee0-f35f-11ed-aff5-859eb6ed880f,key:query,negate:!f,type:custom,value:'%7B%22bool%22:%7B%22minimum_should_match%22:1,%22should%22:%5B%7B%22match_phrase%22:%7B%22user-tags.cluster-config%22:%22arm64-r6g.2xlarge-3-data-3-shards-1-replica-nmslib-cohere-1m%22%7D%7D,%7B%22match_phrase%22:%7B%22meta.tag_cluster-config%22:%22arm64-r6g.2xlarge-3-data-3-shards-1-replica-nmslib-cohere-1m%22%7D%7D%5D%7D%7D'),query:(bool:(minimum_should_match:1,should:!((match_phrase:(user-tags.cluster-config:arm64-r6g.2xlarge-3-data-3-shards-1-replica-nmslib-cohere-1m)),(match_phrase:(meta.tag_cluster-config:arm64-r6g.2xlarge-3-data-3-shards-1-replica-nmslib-cohere-1m))))))),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),query:(language:kuery,query:''),timeRestore:!f,title:'%5BVector%20Search%5D%20%5BNmslib%5D%201M%20Cohere%20768D',viewMode:view)&show-time-filter=true&hide-filter-bar=true"></iframe>
Copy link
Collaborator

@AMoo-Miki AMoo-Miki Nov 18, 2024

Choose a reason for hiding this comment

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

wrt to this iframe and all of the other iframes in this PR:

For the same reasons that page.benchmark_domain is a dynamic value, the URLs for each of these iframes needs to be dynamic. A different instance of OSD would more that likely have a different DashboardId; the 1a369150-e232-11ee-addb-f72c2feaa994 here would be different. Similarly, the IndexPatternId would be different as well; the b4c18ee0-f35f-11ed-aff5-859eb6ed880f in the URL.

Additionally, OSD provides a URL shortening feature which would work nice for these. My suggestion would be to have the full short-url for benchmark_url. Doing so, we wont need so many identical pages.

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 got the short-url point but not sure about Doing so, we wont need so many identical pages..
Do you mean to say we can achieve individual pages for each dashboard without having a dedicated .md files for them?
This is my first attempt on front-end programming, please excuse if my questions seem stupid. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AMoo-Miki If this is not a critical blocker can we have this merged and I can take up the refactoring in subsequent PRs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants