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

profiles: use single sample_type per Profile #610

Closed
wants to merge 1 commit into from

Conversation

florianl
Copy link
Contributor

@florianl florianl commented Dec 11, 2024

Each message Profile should hold only a single kind of sample_type. This allows easier, parallel and more directed processing of each message Profile.

Still, a single OTel profiling protocol message can have multiple message Profile each with its individual sample_type. This lifts the limitation from the original pprof format, where a single message Profile can and needs to be able to hold different kinds of samples, as it does not have an enveloping message like message ScopeProfiles in the OTel profiling protocol.

ping @open-telemetry/profiling-maintainers @jhalliday

Each `message Profile` should hold only a single kind of `sample_type`. This allows easier, parallel and more directed processing of each `message Profile`.

Still, a single OTel profiling protocol message can have multiple `message Profile` each with its individual `sample_type`. This lifts the limitation from the original pprof format, where a single `message Profile` can and needs to be able to hold different kinds of samples, as it does not have an enveloping message like `message ScopeProfiles` in the OTel profiling protocol.
@aalexand
Copy link

If we make this change, it should probably be accompanied by moving up various tables up to ScopeProfiles as well since otherwise there would be duplication of the data in those tables.

Also, a question on the API structure: one use case / role that pprof's profile proto plays today is a simple container for a single one-shot data collection. For example, in https://go.dev/blog/pprof the example code looks like

var cpuprofile = flag.String("cpuprofile", "", "write cpu profile to file")

func main() {
    flag.Parse()
    if *cpuprofile != "" {
        f, err := os.Create(*cpuprofile)
        if err != nil {
            log.Fatal(err)
        }
        pprof.StartCPUProfile(f)
        defer pprof.StopCPUProfile()
    }
    ...

and this code writes out the data in the profile proto format, of the message type Profile.

In the current OTel type hierarchy, what type plays a similar role? If tomorrow some language runtime decides to be OTel profiling first, what type would we expect it to produce? Before this change I would say that it's the Profile type. With this change I think it's now ScopeProfiles which is kind of an clumsy name for that purpose.

Curious what others think on this.

@florianl
Copy link
Contributor Author

Closing this proposal after discussing it in the recent Profiling SIG meeting.

@florianl florianl closed this Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants