Change pprof labels to be conformat with prometheus spec #13826

New Issue

GiteaMirror · 2025-11-02T10:54:15-06:00

GiteaMirror commented

2025-11-02 10:54:15 -06:00

Originally created by @TheFox0x7 on GitHub (Dec 15, 2024).

Feature Description

This would be another port of my changes.

There are systems, like pyroscope allowing for continuous profiling which do not allow for - in labels leading to upload failure.

To resolve this names like process-description can be changed either to processDescription or process_description.

Screenshots

No response

Originally created by @TheFox0x7 on GitHub (Dec 15, 2024). ### Feature Description This would be [another port of my changes](https://codeberg.org/forgejo/forgejo/pulls/2933). There are systems, like pyroscope allowing for continuous profiling which do not allow for `-` in labels leading to upload failure. To resolve this names like `process-description` can be changed either to `processDescription` or `process_description`. ### Screenshots _No response_

GiteaMirror added the type/proposal label 2025-11-02 10:54:15 -06:00

GiteaMirror closed this issue

2025-11-02 10:54:16 -06:00

GiteaMirror commented

2025-11-02 10:54:17 -06:00

@silverwind commented on GitHub (Dec 15, 2024):

Can you link to some docs from prometheus that confirms that regex?

@silverwind commented on GitHub (Dec 15, 2024): Can you link to some docs from prometheus that confirms that regex?

GiteaMirror commented

2025-11-02 10:54:17 -06:00

@wxiaoguang commented on GitHub (Dec 15, 2024):

If I understand correctly, these "labels" are designed for internal profiling/tracing only, there is no guarantee that these names will always be stable or compatible. Should we really expose these labels to external systems? (I even can't remember where&when we exposed them)

@wxiaoguang commented on GitHub (Dec 15, 2024): If I understand correctly, these "labels" are designed for internal profiling/tracing only, there is no guarantee that these names will always be stable or compatible. Should we really expose these labels to external systems? (I even can't remember where&when we exposed them)

GiteaMirror commented

2025-11-02 10:54:17 -06:00

@TheFox0x7 commented on GitHub (Dec 15, 2024):

Can you link to some docs from prometheus that confirms that regex?

That's metrics specific, but https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
Otel spec allows for . as well from what I remember.

The labels are included in pprof endpoints and having them compatible would enable someone interested to continuously gather profiles. Current labels prevent importing goroutine profiles, rest works fine. It might be only grafana stack issue - Otel has profiles on experimental phase so I can't rely on it for a hard spec.

@TheFox0x7 commented on GitHub (Dec 15, 2024): > Can you link to some docs from prometheus that confirms that regex? That's metrics specific, but https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels Otel spec allows for `.` as well from what I remember. The labels are included in pprof endpoints and having them compatible would enable someone interested to continuously gather profiles. Current labels prevent importing goroutine profiles, rest works fine. It might be only grafana stack issue - Otel has profiles on experimental phase so I can't rely on it for a hard spec.

GiteaMirror commented

2025-11-02 10:54:17 -06:00

@wxiaoguang commented on GitHub (Dec 15, 2024):

The labels are included in pprof endpoints and having them compatible would enable someone interested to continuously gather profiles. Current labels prevent importing goroutine profiles, rest works fine. It might be only grafana stack issue - Otel has profiles on experimental phase so I can't rely on it for a hard spec.

Maybe we should introduce a new function like this:

func withGracefulLifecycleLabel(ctx, lifecycleName) { 
    return pprof.WithLabels(ctx, pprof.Labels("graceful_lifecycle", lifecycleName))
}

terminateCtx = withGracefulLifecycleLabel(terminateCtx, "with-terminate")

Then we only need to manage the one name in withGracefulLifecycleLabel, and we can put more comments (with the spec link) on that function.

@wxiaoguang commented on GitHub (Dec 15, 2024): > The labels are included in pprof endpoints and having them compatible would enable someone interested to continuously gather profiles. Current labels prevent importing goroutine profiles, rest works fine. It might be only grafana stack issue - Otel has profiles on experimental phase so I can't rely on it for a hard spec. Maybe we should introduce a new function like this: ``` func withGracefulLifecycleLabel(ctx, lifecycleName) { return pprof.WithLabels(ctx, pprof.Labels("graceful_lifecycle", lifecycleName)) } terminateCtx = withGracefulLifecycleLabel(terminateCtx, "with-terminate") ``` Then we only need to manage the one name in `withGracefulLifecycleLabel`, and we can put more comments (with the spec link) on that function.

GiteaMirror commented

2025-11-02 10:54:17 -06:00

@wxiaoguang commented on GitHub (Dec 15, 2024):

Or introduce a const like const DescriptionPProfLabel and add comments there?

@wxiaoguang commented on GitHub (Dec 15, 2024): Or introduce a const like `const DescriptionPProfLabel` and add comments there?

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#13826