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

ci: support for generating flame graph #6136

Merged
merged 74 commits into from
Feb 16, 2022
Merged

ci: support for generating flame graph #6136

merged 74 commits into from
Feb 16, 2022

Conversation

tzssangglass
Copy link
Member

What this PR does / why we need it:

example: https://github.com/tzssangglass/apisix/actions/runs/1709028726

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@tzssangglass
ci: add perf test and generate flame graph
bdf4823 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
add trigger
2d842f8 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
add perf.sh
3a62b87 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
update install deps order
4bdae40 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
update
ffba68b 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
add debug
1244a38 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
add debug
b9b32bc 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
rm debug
325f55a 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
Merge branch 'master' into flamegraph
f40bbfb
@tzssangglass
fix typo
422370a 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
add test
a29ece5 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
add install_stap_tools
aec24c4 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
fix typo
6070279 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
remove bionic-security repo
d3cd626 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
test
24d9d1c 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
rm debug
58d9eb5 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
chore order of cmd
ef17fc7 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
add debug
aba8099 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
build apisix-base
5769b3b 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
build apisix-base
4970bad 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
build apisix-base
4445693 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
build apisix-base
45699e5 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
chore
d03b9be 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
chore
d6c5000 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
chore
a434fc6 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
basic completed
b821024 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
rm debug
80f6b15 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
chore
a604c01 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
chore-2
45222c0 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
chore-3
25d1a44 
Signed-off-by: tzssangglass <[email protected]>
@TobKed TobKed mentioned this pull request Jan 22, 2022
Merged
13 tasks
@tzssangglass tzssangglass added performance generate flamegraph for the current PR and removed performance generate flamegraph for the current PR labels Feb 8, 2022
@tzssangglass
add comments
8db8508 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
test for create comments
8c03fd8 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
debug
cd9d162 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
fix CI error
b5c69de 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
rm echo
85947b3 
Signed-off-by: tzssangglass <[email protected]>
@tzssangglass
rm unused code
703a6e9 
Signed-off-by: tzssangglass <[email protected]>
shuaijinchao
shuaijinchao reviewed Feb 10, 2022
View reviewed changes
@tzssangglass
rm action
1ae6b6c 
Signed-off-by: tzssangglass <[email protected]>
shuaijinchao
shuaijinchao previously approved these changes Feb 11, 2022
View reviewed changes
spacewander
spacewander reviewed Feb 13, 2022
View reviewed changes
Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

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

Better to provide some hints about where to find the output

.github/workflows/performance.yml

jobs:
performance:
if: github.event_name == 'pull_request' || github.event.label.name == 'performance'
Copy link
Member

Choose a reason for hiding this comment

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

Should we only rely on the label? The pull request can be triggered by any user.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. However, the current test case only contains jwt-auth, and I feel that it seems too early to combine the user's PR with the test case. It would be nice to have targeted tests for the user's PR. Do you have any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

We can apply a label if needed. Note that people can run anything based on their PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can apply a label if needed

Now this PR is satisfied, add performance and it will run. To run it again you need to remove performance and add it again.

Note that people can run anything based on their PR.

Yes, the idealization is so. But this requires continued enhancements to the performance testing framework.
Maybe we can change it the next time we need to run performance tests for other PRs.

@tzssangglass
add comments
f876d1f 
Signed-off-by: tzssangglass <[email protected]>
@spacewander spacewander merged commit d8b9b70 into apache:master Feb 16, 2022
@tzssangglass tzssangglass deleted the flamegraph branch February 21, 2022 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spacewander spacewander spacewander approved these changes

@shuaijinchao shuaijinchao shuaijinchao approved these changes

@tokers tokers tokers approved these changes

@bisakhmondal bisakhmondal Awaiting requested review from bisakhmondal

Assignees
No one assigned
Labels
performance generate flamegraph for the current PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tzssangglass @spacewander @shuaijinchao @tokers