[WebConsole] Implement Samply profile collection UI #5423
Conversation
9a32617 to
06565a8
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements a user interface for collecting and downloading Samply CPU profiles in the Feldera web console. The "Profiler" tab has been renamed to "Dataflow Visualizer" to differentiate it from the new Samply profiling functionality.
Changes:
- Added new Samply profile collection UI with configurable duration and visual progress indicator
- Implemented backend support for tracking in-progress profile collection with expected completion times
- Updated API to return status 202 (Accepted) when starting collection and 307 (Temporary Redirect) when profile is in progress
- Enhanced error message formatting by preserving newlines in error details
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| openapi.json | Updated API responses: 202 for profile start, 307 for in-progress, changed error details schema to Value |
| js-packages/web-console/src/lib/services/pipelineManager.ts | Added collectSamplyProfile and getLatestSamplyProfile wrapper functions |
| js-packages/web-console/src/lib/services/manager/types.gen.ts | Auto-generated type updates from OpenAPI changes |
| js-packages/web-console/src/lib/compositions/useToastNotification.ts | Added whitespace-pre-wrap to preserve newlines in error messages |
| js-packages/web-console/src/lib/compositions/usePipelineManager.svelte.ts | Integrated new Samply profile API functions |
| js-packages/web-console/src/lib/components/pipelines/editor/TabSamplyProfile.svelte | New component implementing profile collection UI with countdown timer and progress visualization |
| js-packages/web-console/src/lib/components/pipelines/editor/TabProfileVisualizer.svelte | Renamed label to "Dataflow Visualizer" |
| js-packages/web-console/src/lib/components/pipelines/editor/MonitoringPanel.svelte | Added Samply tab to monitoring panel |
| js-packages/web-console/src/lib/components/pipelines/editor/InteractionPanel.svelte | Added Samply tab to interaction panel |
| js-packages/web-console/src/lib/components/pipelines/editor/DownloadSupportBundle.svelte | Removed Oxford comma from tooltip |
| js-packages/web-console/package.json | Updated build-openapi command to use -p flag |
| js-packages/web-console/.prettierignore | Added .svelte-kit/ and build/ directories |
| crates/pipeline-manager/src/api/endpoints/pipeline_interaction.rs | Updated OpenAPI documentation for status codes |
| crates/feldera-types/src/error.rs | Changed error details schema from Object to Value |
| crates/adapters/src/server/error.rs | Changed error details schema from Object to Value |
| crates/adapters/src/server.rs | Implemented InProgress state handling and immediate error detection |
| crates/adapters/src/samply.rs | Added InProgress variant to SamplyProfile enum |
| crates/adapters/src/controller.rs | Removed newline escaping in error messages |
| let isCollecting = $state(false) | ||
| let startTime = $state<number | null>(null) | ||
| let expectedCompletion = $state<number | null>(null) | ||
| let profileReady = $state(false) |
There was a problem hiding this comment.
The profileReady state variable is set to true when the countdown completes (line 109) but is never used anywhere in the component. This state variable should either be removed if it's not needed, or used to provide visual feedback to the user that the profile is ready for download.
| let profileReady = $state(false) |
| if (profile.expectedInSeconds) { | ||
| // Profile is still being collected, update countdown with the server's expected time | ||
| const now = Date.now() | ||
| expectedCompletion = now + profile.expectedInSeconds * 1000 | ||
| // Ensure collecting state is active | ||
| if (!isCollecting) { | ||
| isCollecting = true | ||
| profileReady = false | ||
| startTime = now | ||
| startCountdown() | ||
| } |
There was a problem hiding this comment.
When synchronizing the countdown with the server's expected time in the download handler, the duration variable should also be updated to match the remaining time. Without this, if a user comes back later and clicks download while a profile is being collected, the UI progress bar and time display will be inconsistent because they're calculated based on the duration variable (see lines 44-61), not the actual remaining time from the server.
|
|
||
| Ok(HttpResponse::Ok().finish()) | ||
| // Wait to check if it errored out immediately | ||
| sleep(Duration::from_millis(600)).await; |
There was a problem hiding this comment.
The hard-coded 600ms wait time lacks documentation explaining why this specific duration was chosen. Consider adding a comment explaining that this delay allows the system to detect if profiling fails immediately, or extracting this as a named constant with a descriptive name.
| </script> | ||
|
|
||
| {#snippet label()} | ||
| <span class=""> CPU Profile </span> |
There was a problem hiding this comment.
The tab label "CPU Profile" is inconsistent with the tab name "Samply" used in the tab configuration. Consider using a more consistent label such as "Samply Profile" or aligning the tab name with "CPU Profile" to avoid confusion.
| <span class=""> CPU Profile </span> | |
| <span class=""> Samply Profile </span> |
| // Set the state to InProgress with expected completion time | ||
| let expected_after = chrono::Utc::now() + chrono::Duration::seconds(duration as i64); | ||
| *state_samply_profile.lock().unwrap() = SamplyProfile::InProgress { expected_after }; | ||
|
|
||
| spawn(async move { | ||
| let result = controller.async_samply_profile(duration).await; | ||
| state_samply_profile.lock().unwrap().update(result); | ||
| }); |
There was a problem hiding this comment.
There is no protection against concurrent profile collection requests. If multiple requests to start profiling are made while a profile is already being collected, the state will be overwritten and multiple profiling tasks will run concurrently, competing to update the shared state. Consider adding a check to return an error (e.g., 409 Conflict) if a profile collection is already in progress (when state is InProgress).
06565a8 to
83e0e21
Compare
Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
83e0e21 to
27c3c4c
Compare
|
"dataflow graph" is a completely static notion, it does not have any relationship with performance. |
I thought "profiler" was perfect? What are we trying to disambiguate? |
|
I agree with @ryzhyk . "Profiler" was perfect. |
I renamed the "Profiler" tab to "Dataflow visualizer" to disambiguate the new functionality.
I also updated API response status code of some Samply responses, added a way to retrieve the estimate for the profile already being collected, and added a way to catch if the profile collection failed immediately by waiting for a set time after the profile was requested