Skip to content

Conversation

@TheFox0x7
Copy link
Contributor

Enables scrapping pprof endpoint for continuous profiling

Closes: #32854


There's probably room for improvement - for example dropping graceful_lifecycle completely and making with-hammer into with: hammer key value (or something similar). But I'd wait with that for a proper spec in OTel so there there won't be a need to align to spec n times.

Name is compliant with both prometheus regex and OTel recommendation (but not to their semantic conventions):

For each multi-word dot-delimited component of the attribute name separate the words by underscores (i.e. use snake_case).

Enables scrapping pprof endpoint for continuous profiling
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 16, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Dec 16, 2024
@lunny
Copy link
Member

lunny commented Dec 16, 2024

Is this a breaking change for those upgraded Gitea instance?

@TheFox0x7
Copy link
Contributor Author

It was marked as such in the fork. However the for it to be breaking someone would actively have to scrape the endpoint for (semi-)automated analysis, which I very much doubt anyone is doing (evidence being that no one complained about it).

If you consider that a real possibility then yes, it is breaking.

@lunny lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Dec 16, 2024
@lunny lunny added this to the 1.24.0 milestone Dec 16, 2024
@lunny
Copy link
Member

lunny commented Dec 16, 2024

The pprof labels was introduced in #19202 and I found there is a grafana mixin introduced in #17758. Maybe this can also be added to grafana mixin in the same PR?

@wxiaoguang
Copy link
Contributor

Maybe this can also be added to grafana mixin in the same PR?

No, there are different, do not mix them at the moment.

@wxiaoguang wxiaoguang removed the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Dec 17, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 17, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 17, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 17, 2024
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

Let's measure if it is breaking by whether someone complains 😄

@wxiaoguang
Copy link
Contributor

Let's measure if it is breaking by whether someone complains 😄

Actually it breaks nothing .....

@wxiaoguang wxiaoguang merged commit b945742 into go-gitea:main Dec 18, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Dec 18, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Mar 18, 2025
@TheFox0x7 TheFox0x7 deleted the change-pprof-label branch October 31, 2025 23:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change pprof labels to be conformat with prometheus spec

5 participants