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

Refine conprof #1102

Merged
merged 17 commits into from
Dec 29, 2021
Merged

Refine conprof #1102

merged 17 commits into from
Dec 29, 2021

Conversation

baurine
Copy link
Collaborator

@baurine baurine commented Dec 16, 2021

What did:

  • update wording
  • rename i18n key "continue_profiling" to "conprof"
  • update check ngm ready logic
  • replace the time range component
  • align the detail page UI and UX with manual profiling detail page

image

image

image

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 16, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • breeswish

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@baurine baurine marked this pull request as draft December 16, 2021 14:33
@baurine baurine marked this pull request as ready for review December 17, 2021 10:45
This reverts commit 3b03fdb.
@breezewish
Copy link
Member

breezewish commented Dec 21, 2021

Cool! Could you also prepare a preview (merging with changes from #1095) so that @lilyjazz can evaluate and provide some UI design comments? Thanks a lot!

@breezewish breezewish self-assigned this Dec 21, 2021
@baurine
Copy link
Collaborator Author

baurine commented Dec 21, 2021

Cool! Could you also prepare a preview (merging with changes from #1095) so that @lilyjazz can evaluate and provide some UI design comments? Thanks a lot!

OK

@baurine
Copy link
Collaborator Author

baurine commented Dec 24, 2021

@breeswish @shhdgit @YiniXu9506 PTAL, thanks!

Comment on lines 46 to 51
// rename profile to cpu for profile_type for easier sort
profiles.forEach((p) => {
if (p.profile_type === 'profile') {
p.profile_type = 'cpu'
}
})
Copy link
Member

Choose a reason for hiding this comment

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

Can we sort profile type by specific enum order.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How can we do it, do you have an example?

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to specify the ordering here? Maybe I misunderstood it before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, the design spec requires.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe keeping a specific order is fine. There is no need to make CPU the first one? Please confirm with PM.

Even if you want to sort CPU as the first, what you should do is defining a sort key array and sort by the sort key.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed with PM, it is required, let me refine the sort method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

shhdgit added a commit to shhdgit/tidb-dashboard that referenced this pull request Dec 28, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@df625e0). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1102   +/-   ##
=========================================
  Coverage          ?   23.11%           
=========================================
  Files             ?      102           
  Lines             ?     9981           
  Branches          ?        0           
=========================================
  Hits              ?     2307           
  Misses            ?     7524           
  Partials          ?      150           
Flag Coverage Δ
be_integration_test_latest 7.05% <0.00%> (?)
be_integration_test_v4.0.1 7.05% <0.00%> (?)
be_unit_test 24.12% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df625e0...6333063. Read the comment docs.

@baurine
Copy link
Collaborator Author

baurine commented Dec 29, 2021

all done, PTAL, thanks!

@baurine baurine merged commit 37aa6b2 into pingcap:master Dec 29, 2021
@baurine baurine deleted the refine-conprof branch December 29, 2021 10:12
baurine added a commit to baurine/tidb-dashboard that referenced this pull request Dec 30, 2021
* update wording

* not check prom any more

* replace time range component

* i18n

* support view profile by diffrent ways

* extract ActionsButton

* change download data format

* refine

* comments

* Revert "comments"

This reverts commit 3b03fdb.

* fix view cpu profile fail

* update state

* hide action button if disable

* address feedback
baurine added a commit that referenced this pull request Dec 30, 2021
* Revert "Release v2021.12.06.1 (#1084)"

This reverts commit bcc43a0.

* Compitable with different TiDB versions for conprof and non-root-login features (#1047)

* make conprof independent

* check feature enable

* add check feature enable middleware

* hide menu if feature is not enabled

* refactor non root login switch by new design

* i18n

* yarn fmt

* renaming

* adjust fe code

* refine

* remove unused log

* build(deps): bump ws from 5.2.2 to 5.2.3 in /ui (#1055)

* CICD: Update the release pipeline for recent PD format policies (#1054)

* fix i18n wording (#1056)

* Refactor: Change util module to util package (#1052)

* Refactor: Fix godot incorrectly add dot suffix to annotations (#1059)

* lint: Add goheader for copyright lints (#1062)

* Refactor: Migrate to use the `rest` package in util/ (#1060)

* fix(*): globally delete/update data by GORM (#1065)

* ui: bump dependencies (#1066)

* refactor: Switch to use ziputil, netutil, reflectutil and fileswap (#1067)

* Fix request header being pinned after pd profiling (#1069)

* Integrate speedscope (#1064)

* fix potential panic when GetPDInstances (#1075)

Signed-off-by: crazycs <chen.two.cs@gmail.com>

* Refactor: a new httpclient (#1073)

* Refactor: Switch to use util/distro in all places (#1078)

* chore: support import relative file URL (#1082)

* Refactor: Move tools into a standalone module (#1079)

* Fix script to embed the ui (#1088)

* Fix script to embed the ui

* Hack write_strings

* Refactor feature flag to support more modules (#1057)

* Drop sysutil dependency (#1093)

* chore: add graph generation (#1085)

* Refactor: Add TopologyProvider (#1098)

* esbuild: i18n + dep (#1101)

* script: Add a script to generate version matrix (#1104)

* distro: support dynamic config (#1094)

* chore: support multiple profiling types (#1095)

* fix(distro): check distro_strings.json fmt by prettier (#1106)

* script: fix generate assets (#1107)

* Add integration test (#1083)

* debug_api: Switch to use the new util (#1103)

* refactor(ui): auto refresh button (#1105)

* refactor(ui): auto refresh button

* chore: update translation

* fix: remain seconds

* refine: refresh button

* fix: onRefresh

* fix: auto refresh

* fix: continue tick

* chore: add some comments

* tweak: remaining refresh seconds

* chore: clean code

Co-authored-by: Wenxuan <breezewish@pingcap.com>

* ui: refine conprof (#1102)

* update wording

* not check prom any more

* replace time range component

* i18n

* support view profile by diffrent ways

* extract ActionsButton

* change download data format

* refine

* comments

* Revert "comments"

This reverts commit 3b03fdb.

* fix view cpu profile fail

* update state

* hide action button if disable

* address feedback

* update release-version

* sync with master

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Wenxuan <breezewish@pingcap.com>
Co-authored-by: Suhaha <jklopsdfw@gmail.com>
Co-authored-by: Yini Xu <34967660+YiniXu9506@users.noreply.github.com>
Co-authored-by: crazycs <crazycs520@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants