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

Lighthouse performance metrics setup #9304

Open
wants to merge 48 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
8913f15
lighthouse ci added
Jan 28, 2025
bb50042
lighthouse setup
Jan 29, 2025
49713ef
lighthouse setup
Jan 29, 2025
fb42ef5
lighthouse setup
Jan 29, 2025
3c0799e
lught house steup
Jan 29, 2025
3d5dddc
ci metrics setup
Jan 29, 2025
063d9a1
Merge branch 'opensearch-project:main' into ruchi/lighthouse-backup
ruchidh Jan 30, 2025
20dbacf
fix missing lhci install from flow
ruchidh Jan 30, 2025
6636bf9
add yarn bootstrap to run dependencies
ruchidh Jan 30, 2025
b937c42
download opensearch
ruchidh Jan 30, 2025
3f15a5f
add wait for for be server
ruchidh Jan 31, 2025
4aa842b
add node options
ruchidh Jan 31, 2025
33b9660
remove yarn build and add boottsrap
ruchidh Jan 31, 2025
7f59b70
update version
ruchidh Jan 31, 2025
4b63789
change lhci from global to dev
ruchidh Jan 31, 2025
20957e7
update version
ruchidh Jan 31, 2025
1bcb100
updare version
ruchidh Jan 31, 2025
664d4de
add mock data
ruchidh Jan 31, 2025
f6c03a7
add wait
ruchidh Jan 31, 2025
846bccf
add wait on
ruchidh Feb 2, 2025
5f785ae
reevrt waiton opensearch
ruchidh Feb 2, 2025
c566adf
reevrt waiton opensearch
ruchidh Feb 2, 2025
1f70a7b
commetn wait on
ruchidh Feb 3, 2025
127f17d
add version mismatch
ruchidh Feb 3, 2025
db60ecd
use curl sleep
ruchidh Feb 3, 2025
edc648f
yarn lchi command fix
ruchidh Feb 3, 2025
3142bd4
Delete .lighthouseci/links.json
ruchidh Feb 3, 2025
5f3b91a
add faile dmetrics
ruchidh Feb 4, 2025
38a6caa
run lighthouse view
ruchidh Feb 4, 2025
0045291
add github aoo token
ruchidh Feb 4, 2025
e90be27
update performance testing
ruchidh Feb 7, 2025
7feae97
use github lightouse ci action
ruchidh Feb 7, 2025
75591b7
update config
ruchidh Feb 7, 2025
c947e1b
revert security check
ruchidh Feb 7, 2025
7550500
udpate post comment format
ruchidh Feb 7, 2025
ee1320d
udpate name for comment step
ruchidh Feb 7, 2025
f3919b7
set env token
ruchidh Feb 7, 2025
ce194a5
update token
ruchidh Feb 7, 2025
fe08858
add gthub token test
ruchidh Feb 7, 2025
ea7c386
replace lightouse to github token
ruchidh Feb 7, 2025
43b7df8
udate extentions to txt
ruchidh Feb 7, 2025
09c2fa8
remove preset
ruchidh Feb 7, 2025
403f9c4
update metis
ruchidh Feb 8, 2025
4e0a4b6
remove security check
ruchidh Feb 8, 2025
2d46b30
update comment format
ruchidh Feb 8, 2025
22512cc
update xpected and actul value
ruchidh Feb 8, 2025
aa578dc
update web view and baseline
ruchidh Feb 9, 2025
15c7500
fix url
ruchidh Feb 9, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .github/workflows/performance_testing.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: Performance Testing
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need:

# Copyright OpenSearch Contributors
# SPDX-License-Identifier: Apache-2.0


on:
pull_request:
branches:
- main

jobs:
lighthouse:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Setup Node
uses: actions/setup-node@v2
with:
node-version-file: '.nvmrc'
registry-url: 'https://registry.npmjs.org'

- name: Setup Yarn
run: |
npm uninstall -g yarn
npm i -g [email protected]

- name: Run bootstrap
run: yarn osd bootstrap

- name: Install Lighthouse CI
Copy link
Member

Choose a reason for hiding this comment

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

is there not a github action that we can pull from the marketplace and run this?

run: yarn global add @lhci/cli

- name: Build OpenSearch Dashboards
run: yarn build
Copy link
Member

Choose a reason for hiding this comment

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

build will build a platform distribution. are we sure we dont want to yarn osd bootstrap?


- name: Start OpenSearch Dashboards
run: yarn start --no-base-path & yarn wait-on http://localhost:5601

- name: Run Lighthouse CI
run: yarn lhci autorun || echo "Lighthouse assertion failed, check results"

- name: Post Lighthouse Report to PR
if: always()
run: |
echo "🚦 **Lighthouse CI Report**" > comment.txt
echo "" >> comment.txt
lhci upload --target=temporary-public-storage | tee lhci_output.txt
REPORT_URL=$(grep -o 'https://storage.googleapis.com/lighthouse-infrastructure-.*' lhci_output.txt | head -n 1)
echo "🔗 [View Full Lighthouse Report]($REPORT_URL)" >> comment.txt
gh pr comment ${{ github.event.pull_request.number }} --body "$(cat comment.txt)"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
28 changes: 28 additions & 0 deletions .lighthouserc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
const baseUrl = 'http://localhost:5601';
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need a license here? (like .eslintrc.js)

Copy link
Member

Choose a reason for hiding this comment

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

baseUrl technically doesn't have to be 5601. It can be configurable to be something like 443


module.exports = {
ci: {
collect: {
url: [`${baseUrl}`, `${baseUrl}/app/data-explorer/discover`, `${baseUrl}/app/dashboards`], // Add more URLs as needed
Copy link
Contributor

@angle943 angle943 Jan 30, 2025

Choose a reason for hiding this comment

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

Something i'm wondering about...

Without some specific actions, I don't think any results would be loaded in discover and no dashboards will be set up on dashboards page, so i'm not sure if we are getting the full benefit of having this.

Is it possible to start with some examples that'll load? perhaps the sample data (but even with that i think we need to modify the URL so we are hitting the correct page. For example /app/dashboards might just go to the list of dashboards you need to select from)

Screenshot 2025-01-30 at 3 30 51 PM

Copy link
Member

Choose a reason for hiding this comment

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

+1, you can actually hit the api or bulk insert saved objects and data

Copy link
Author

Choose a reason for hiding this comment

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

yes, have plan to do so, updating mock data

startServerCommand: 'yarn start --no-base-path',
numberOfRuns: 3,
settings: {
chromePath: require('puppeteer').executablePath(),
chromeFlags: '--no-sandbox --disable-gpu --headless',
},
},
assert: {
preset: 'lighthouse:recommended',
assertions: {
performance: ['error', { minScore: 0.9 }],
Copy link
Contributor

Choose a reason for hiding this comment

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

oof i think we need make the minscore much lower. and it might differ between the different URLs.

For example on dashboards page with the example data loaded, i am getting a score of 34 (yikes)

Copy link
Author

Choose a reason for hiding this comment

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

This is only for testing purpose, would change in final review.

'first-contentful-paint': ['warn', { maxNumericValue: 1800 }],
'largest-contentful-paint': ['warn', { maxNumericValue: 2500 }],
interactive: ['warn', { maxNumericValue: 5000 }],
'total-blocking-time': ['warn', { maxNumericValue: 300 }],
},
},
upload: {
target: 'temporary-public-storage', // Required for GitHub integration
},
},
};
Copy link
Contributor

Choose a reason for hiding this comment

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

which OS instance does this connect to?

Copy link
Author

@ruchidh ruchidh Jan 31, 2025

Choose a reason for hiding this comment

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

for our BE, 9200. This temporary-public-storage , is a public instance of google report .

4 changes: 2 additions & 2 deletions config/opensearch_dashboards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,10 @@

# Set the backend roles in groups or users, whoever has the backend roles or exactly match the user ids defined in this config will be regard as dashboard admin.
# Dashboard admin will have the access to all the workspaces(workspace.enabled: true) and objects inside OpenSearch Dashboards.
# The default config is [], and no one will be dashboard admin.
# The default config is [], and no one will be dashboard admin.
# If the user config is set to wildcard ["*"], anyone will be dashboard admin.
# opensearchDashboards.dashboardAdmin.groups: ["dashboard_admin"]
# opensearchDashboards.dashboardAdmin.users: ["dashboard_admin"]

# Set the value to true to enable the new UI for savedQueries in Discover
# data.savedQueriesNewUI.enabled: true
# data.savedQueriesNewUI.enabled: true
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@
"@elastic/filesaver": "1.1.2",
"@elastic/makelogs": "^6.1.0",
"@faker-js/faker": "^7.6.0",
"@lhci/cli": "^0.14.0",
Copy link
Member

Choose a reason for hiding this comment

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

if this is a dev dependency then bootstrapping should be suffice enough correct? do we still need to add it the step to install it globally in the github workflow?

"@microsoft/api-documenter": "^7.13.78",
"@microsoft/api-extractor": "^7.19.3",
"@osd/babel-preset": "1.0.0",
Expand Down Expand Up @@ -464,6 +465,7 @@
"postcss": "^8.4.5",
"prettier": "^2.1.1",
"prop-types": "^15.7.2",
"puppeteer": "^24.1.1",
Copy link
Member

Choose a reason for hiding this comment

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

i would be hesitant to add this. i know some opensearch plugin uses puppeteer and they have to bundle a chrome binary so that it can not fail on some integration testing.

Copy link
Member

Choose a reason for hiding this comment

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

i think other plugin teams have used puppeteer-core? not sure haven't looked too deep into it

Copy link
Author

Choose a reason for hiding this comment

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

okay, let's discuss this

"react": "^16.14.0",
"react-grid-layout": "^0.16.2",
"react-markdown": "^4.3.1",
Expand Down
Loading
Loading