Skip to content

ui: fix filtering for flame chart visualization#6228

Merged
manojVivek merged 6 commits intomainfrom
worktree-flamechart-filtering
Mar 17, 2026
Merged

ui: fix filtering for flame chart visualization#6228
manojVivek merged 6 commits intomainfrom
worktree-flamechart-filtering

Conversation

@metalmatze
Copy link
Member

@metalmatze metalmatze commented Mar 10, 2026

Note: This is temporarily disabled for Parca as the samples strips data filtering is not feasible with the backend right now.

Problem

Two related issues prevented filtering from working in the flame chart view:

  1. Filter UI was hiddenProfileFilters was wrapped in !isFlamechartVizOnly in Toolbars/index.tsx, so the filter button never appeared when flame chart was the sole visualization.

  2. Filters weren't sent in the queryProfileFlameChart called useQuery() without passing protoFilters, so even if the UI had been visible, any applied filters would have been silently ignored.

Screenshots

Before without filterting

unfiltered

After with filterting (for malloc)

filtered

Notes

cc @manojVivek — removing the guard you added in #6192, now that the filters are actually wired up to the query.

@alwaysmeticulous
Copy link

alwaysmeticulous bot commented Mar 10, 2026

✅ Meticulous spotted 0 visual differences across 315 screens tested: view results.

Meticulous evaluated ~4 hours of user flows against your PR.

Expected differences? Click here. Last updated for commit 1d46c7c. This comment will update as new commits are pushed.

const isGraphViz = dashboardItems?.includes('flamegraph');
const isGraphVizOnly = dashboardItems?.length === 1 && isGraphViz;
const isFlamechartViz = dashboardItems?.includes('flamechart');
const isFlamechartVizOnly = dashboardItems?.length === 1 && isFlamechartViz;
Copy link
Contributor

Choose a reason for hiding this comment

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

@claude remove this unused variable.

Copy link
Contributor

@manojVivek manojVivek left a comment

Choose a reason for hiding this comment

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

Looking great!

Had it disabled as I was not sure if the backend was handling it well.

Copy link
Contributor

@manojVivek manojVivek left a comment

Choose a reason for hiding this comment

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

From the screenshot, it looks like the data in the samples strips is not filtered for the query. Is that fine?

@metalmatze
Copy link
Member Author

From the screenshot, it looks like the data in the samples strips is not filtered for the query. Is that fine?

Good catch! We should do that too! I just didn't realize it was happening.

@manojVivek manojVivek requested review from a team as code owners March 16, 2026 07:28
@manojVivek manojVivek merged commit e7f842e into main Mar 17, 2026
35 checks passed
@manojVivek manojVivek deleted the worktree-flamechart-filtering branch March 17, 2026 05:03
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