Skip to content

Add pprof support#22005

Merged
reasonerjt merged 1 commit into
goharbor:mainfrom
strigazi:pprof
Jan 22, 2026
Merged

Add pprof support#22005
reasonerjt merged 1 commit into
goharbor:mainfrom
strigazi:pprof

Conversation

@strigazi

@strigazi strigazi commented May 17, 2025

Copy link
Copy Markdown
Contributor

Env vars:

  • PPROF_ENABLED enable/disable pprof handler, disabled by default
  • PPROF_ADDR address to bind to, defaults to :6060

@codecov

codecov Bot commented May 26, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.88%. Comparing base (c8c11b4) to head (86323ff).
⚠️ Report is 628 commits behind head on main.

Files with missing lines Patch % Lines
src/lib/pprof.go 0.00% 15 Missing ⚠️
src/cmd/exporter/main.go 0.00% 3 Missing ⚠️
src/core/main.go 0.00% 3 Missing ⚠️
src/jobservice/main.go 0.00% 3 Missing ⚠️
src/registryctl/main.go 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main   #22005       +/-   ##
===========================================
+ Coverage   45.36%   65.88%   +20.51%     
===========================================
  Files         244     1073      +829     
  Lines       13333   115889   +102556     
  Branches     2719     2925      +206     
===========================================
+ Hits         6049    76354    +70305     
- Misses       6983    35310    +28327     
- Partials      301     4225     +3924     
Flag Coverage Δ
unittests 65.88% <0.00%> (+20.51%) ⬆️

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

Files with missing lines Coverage Δ
src/cmd/exporter/main.go 0.00% <0.00%> (ø)
src/core/main.go 0.00% <0.00%> (ø)
src/jobservice/main.go 0.00% <0.00%> (ø)
src/registryctl/main.go 0.00% <0.00%> (ø)
src/lib/pprof.go 0.00% <0.00%> (ø)

... and 982 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/cmd/standalone-db-migrator/main.go Outdated
Comment thread src/lib/pprof.go
Comment thread src/lib/pprof.go Outdated
@wy65701436

Copy link
Copy Markdown
Contributor

@strigazi thanks for your contribution, it's generally LGTM.

Comment thread src/lib/pprof.go Outdated

@chlins chlins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@wy65701436

Copy link
Copy Markdown
Contributor

@strigazi can you fix the ci failure?

Error: 07:21:50 lib/pprof.go:19:2: blank-imports: a blank import should be only in a main or test package, or have a comment justifying it (revive)
07:21:50 _ "net/http/pprof"
07:21:50 ^
07:21:50 1 issues:

@strigazi

Copy link
Copy Markdown
Contributor Author

@strigazi can you fix the ci failure?

Error: 07:21:50 lib/pprof.go:19:2: blank-imports: a blank import should be only in a main or test package, or have a comment justifying it (revive)
07:21:50 _ "net/http/pprof"
07:21:50 ^
07:21:50 1 issues:

DONE

I added a comment similar to:
https://github.com/goharbor/harbor/blob/main/src/controller/icon/controller.go#L23-L26

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@wy65701436 wy65701436 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@wy65701436
wy65701436 enabled auto-merge (squash) July 29, 2025 06:49
@wy65701436 wy65701436 closed this Jul 29, 2025
auto-merge was automatically disabled July 29, 2025 06:49

Pull request was closed

@wy65701436

Copy link
Copy Markdown
Contributor

@strigazi can you help to merge the latest changes from main? Then I can merge this PR, thanks.

@wy65701436
wy65701436 enabled auto-merge (squash) July 29, 2025 06:50
Env vars:
* PPROF_ENABLED enable/disable pprof handler, disabled by default
* PPROF_ADDR address to bind to, defaults to :6060

Signed-off-by: Spyros Trigazis <[email protected]>
auto-merge was automatically disabled July 31, 2025 06:50

Head branch was pushed to by a user without write access

@strigazi

Copy link
Copy Markdown
Contributor Author

@strigazi can you help to merge the latest changes from main? Then I can merge this PR, thanks.

I just rebased on top of main

@wy65701436
wy65701436 enabled auto-merge (squash) August 11, 2025 09:15
@wy65701436

Copy link
Copy Markdown
Contributor

@strigazi I'm sorry that you have to rebase it again, and I've enable the auto-marge once the CI is passed.

@wy65701436

wy65701436 commented Oct 24, 2025

Copy link
Copy Markdown
Contributor

@strigazi any updates? do you still willing to work on the PR? I think, we're OK to merge it, but re-base with the latest code changes.

@github-actions

Copy link
Copy Markdown

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions Bot added the Stale label Dec 23, 2025
@bupd

bupd commented Dec 28, 2025

Copy link
Copy Markdown
Member

not-stale, got three approvals, should be merged,

@github-actions github-actions Bot removed the Stale label Dec 29, 2025
@reasonerjt
reasonerjt disabled auto-merge January 22, 2026 07:27
@reasonerjt

Copy link
Copy Markdown
Contributor

For some reason, the "update branch" button does not appear, but I checked the code seems the risk is small.

I'm force merging this one. I'll fix the issues if there are any discovered after merging

@reasonerjt
reasonerjt merged commit 9ce30f8 into goharbor:main Jan 22, 2026
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/update Update or Fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants