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

Integrate speedscope #1064

Merged
merged 13 commits into from
Nov 29, 2021
Merged

Conversation

YiniXu9506
Copy link
Contributor

@YiniXu9506 YiniXu9506 commented Nov 19, 2021

This PR achieved:

  1. integrating Speedscope to TiDB dashboard with the following improvements:
    • ignore samples with negative weights
    • make stack name more concise
    • set left_heavy as default view mode
  2. unifying fetch method for all instances by adding Content-Type:application/protobuf to TiKV/TiFlash request header.
  3. supporting to view CPU profiling of each instance as flamegraph online by importing protobuf file.
  4. hiding Graph temporary and will support in the near future.

Screen Shot 2021-11-19 at 11 57 01 AM

Many thanks to speedscope for the amazing work, here we use the Left Heavy view mode to understand where all the time is going in situations where there are hundreds or thousands of function calls interleaved between other call stacks. Other view modes and more details can be referred here.

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Nov 19, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • shhdgit

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.

@YiniXu9506 YiniXu9506 requested review from breezewish, baurine and shhdgit and removed request for breezewish and baurine November 19, 2021 05:54
@breezewish breezewish self-assigned this Nov 19, 2021
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

mostly lgtm

pkg/apiserver/apiserver.go Outdated Show resolved Hide resolved
pkg/apiserver/profiling/profile.go Show resolved Hide resolved
pkg/apiserver/profiling/model.go Outdated Show resolved Hide resolved
pkg/apiserver/profiling/fetcher.go Outdated Show resolved Hide resolved
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

Seems that using StaticFS just causes you more trouble than simply putting all assets in public/ and reuse current logics, without breaking current code guarantees :)

This is the downside of introducing a new mechanism.

pkg/apiserver/apiserver.go Outdated Show resolved Hide resolved
ui/lib/apps/InstanceProfiling/pages/Detail.tsx Outdated Show resolved Hide resolved
@YiniXu9506
Copy link
Contributor Author

YiniXu9506 commented Nov 24, 2021

Seems that using StaticFS just causes you more trouble than simply putting all assets in public/ and reuse current logics, without breaking current code guarantees :)

This is the downside of introducing a new mechanism.

I have been updated the integration method, which is copy all assets of speedscope from node_modules to `public/speedscope/' without changing current endpoint mechanism.

This copy operation is implemented in gulpfile.ems.js.

YiniXu9506 added 2 commits November 26, 2021 10:50
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

The rest LGTM. There are no code logic problems.

pkg/apiserver/profiling/model.go Outdated Show resolved Hide resolved
pkg/apiserver/profiling/fetcher.go Show resolved Hide resolved
pkg/apiserver/profiling/model.go Show resolved Hide resolved
pkg/apiserver/profiling/pprof.go Outdated Show resolved Hide resolved
pkg/apiserver/profiling/pprof.go Outdated Show resolved Hide resolved
pkg/apiserver/profiling/router.go Outdated Show resolved Hide resolved
pkg/apiserver/profiling/router.go Outdated Show resolved Hide resolved
pkg/httpc/client.go Outdated Show resolved Hide resolved
@YiniXu9506 YiniXu9506 requested a review from shhdgit November 26, 2021 11:39
Copy link
Member

@shhdgit shhdgit left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

The rest LGTM

pkg/apiserver/profiling/pprof.go Outdated Show resolved Hide resolved
@breezewish
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 39bc1b3

@ti-chi-bot ti-chi-bot merged commit 8029289 into pingcap:master Nov 29, 2021
@YiniXu9506 YiniXu9506 deleted the integrate_speedscope branch November 29, 2021 11:57
baurine pushed a commit to baurine/tidb-dashboard that referenced this pull request Dec 30, 2021
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.

4 participants