Fix profile screen

[Alevsk]

Profiling endpoint fixes

• Added support to download all profile tests
• profile.zip file was corrupted after download

Signed-off-by: Lenin Alevski alevsk.8772@gmail.com

[dvaldivia]

I think @kaankabalak was also working on this :S

[kaankabalak]

I think @kaankabalak was also working on this :S

Yes @dvaldivia, there was a problem with the ZIP file being corrupted for the Stop Profiling endpoint, so I sent my work over to @Alevsk and he fixed the corruption issue and sent the PR 👍

[kaankabalak]

@Alevsk, when I download the ZIP file when the type is selected as cpu or mem and unzip it, I see that a .pprof file appears briefly and then it disappears, resulting in a file with a name profile-127.0.0.1/9000-cpu without any extension. I think this might be an unexpected behavior as the download we have for the type selected as all includes this file with a .pprof extension.

Please refer to the video for further detail:

Screen.Recording.2022-03-11.at.5.26.22.PM.mov

And this is the content of the ZIP file when type is selected as all:

[Alevsk]

I think this in a operating system behavior, we are streaming directly what minio is returning to us, I'll check on my Linux box tonight

[prakashsvmx]

@kaankabalak
mc support profile start local --type cpu,mem - this could not be run via ui as the ui allows only one option or all options.

so the type can be one are more or all. so we can have a ui similar to trace

Current

Suggested types to be like (multi select)

@oscarocastellanos @dvaldivia please provide your feedback.

[oscarocastellanos]

@prakashsvmx @dvaldivia This is a good option, I think this is the best thing we can do.

[kaankabalak]

@kaankabalak mc support profile start local --type cpu,mem - this could not be run via ui as the ui allows only one option or all options.

so the type can be one are more or all. so we can have a ui similar to trace

Current

Suggested types to be like (multi select)

@oscarocastellanos @dvaldivia please provide your feedback.

Yes, I also think that this is a good idea, I'll work on this 👍

[dvaldivia]

We also need to make this stateful at UI level, so that the user can navigate away from the profile screen and come back and still get the download, right now if you navigate apart and come back, it shows you to start the profiling again

[kaankabalak]

We also need to make this stateful at UI level, so that the user can navigate away from the profile screen and come back and still get the download, right now if you navigate apart and come back, it shows you to start the profiling again

Hi @dvaldivia, as far as I can see, there is no API at the moment to check if Profiling is currently going on. We only seem to have the /start and /stop endpoints for Profiling. To implement this functionality, we would need an API that checks if a Profiling is currently going on, right? Thanks in advance for the help 👍

[dvaldivia]

@kaankabalak to keep it stateful we would need to start a websocket and keep it open in the back, this can tell the frontend we are running a profile at the moment, alternatively we can add the status API on MinIO, which I think makes sense since starting a profile and forgetting about it could degrade service cc @harshavardhana

[kaankabalak]

@kaankabalak mc support profile start local --type cpu,mem - this could not be run via ui as the ui allows only one option or all options.

so the type can be one are more or all. so we can have a ui similar to trace

Hi @prakashsvmx, I have just applied this change. The screen now looks as follows:

However, for the type argument in the profiling/start endpoint on our API currently supports either only a single value (i.e., {type: "cpu"}) or all the values (i.e., {type: "cpu,mem,block,mutex,trace,threads,goroutines"}).

Therefore, a combination of arguments like what @prakashsvmx has specified is not possible (for example, {type: "cpu,mem"} currently returns a status code of 422). @Alevsk, could you please advise on how we can fix this? Thanks in advance!

[Alevsk]

@kaankabalak mc support profile start local --type cpu,mem - this could not be run via ui as the ui allows only one option or all options.
so the type can be one are more or all. so we can have a ui similar to trace

Hi @prakashsvmx, I have just applied this change. The screen now looks as follows:

However, for the type argument in the profiling/start endpoint on our API currently supports either only a single value (i.e., {type: "cpu"}) or all the values (i.e., {type: "cpu,mem,block,mutex,trace,threads,goroutines"}).

Therefore, a combination of arguments like what @prakashsvmx has specified is not possible (for example, {type: "cpu,mem"} currently returns a status code of 422). @Alevsk, could you please advise on how we can fix this? Thanks in advance!

Let's remove that limitation, to do that on swagger-console.yml locate profilingStartRequest and change the property type from $ref: "#/definitions/profilerType" to just type: string, that way the user can send any value he wants (if the user sends a wrong value minio will throw an error and the profiling won't start which is good).

After making that change you need to run make swagger-gen to re-generate the swagger code

[kaankabalak]

However, for the type argument in the profiling/start endpoint on our API currently supports either only a single value (i.e., {type: "cpu"}) or all the values (i.e., {type: "cpu,mem,block,mutex,trace,threads,goroutines"}).

Therefore, a combination of arguments like what @prakashsvmx has specified is not possible (for example, {type: "cpu,mem"} currently returns a status code of 422). @Alevsk, could you please advise on how we can fix this? Thanks in advance!

Thanks @Alevsk, it works now 👍

[prakashsvmx]

@kaankabalak is the diagnostics expected to complete in certain minutes/seconds. ?

New Diagnostic button exists after Diagnostic is completed
this permission test fails sporadically most of the time. locally i could not replicate it.

[kaankabalak]

@kaankabalak is the diagnostics expected to complete in certain minutes/seconds. ?

New Diagnostic button exists after Diagnostic is completed this permission test fails sporadically most of the time. locally i could not replicate it.

No @prakashsvmx, we don't have any kind of restriction. This only happens on GitHub Actions and I am still investigating this. We saw that the test failed on headless mode for Node version 17, both locally and on GitHub Actions; so we reverted back to Node version 16, yet the failure still happens randomly.

[kaankabalak]

We will be adding the WebSockets functionality on a separate PR 👍