Skip to content

Conversation

@konrad147
Copy link
Contributor

Overview

This PR introduces significant performance optimizations for the Alerting Triage page, focusing on reducing CPU overhead during data transformation and visualization rendering. The changes include lazy-loading visualization panels and implementing a more efficient data transformation algorithm.

Changes Summary

Performance Optimizations

  1. Lazy-loading visualization panels - Uses IntersectionObserver to render expensive visualization components only when they approach the viewport, preventing unnecessary CPU operations for off-screen elements.

  2. Optimized data transformation - Implements a new single-pass algorithm that is approximately 30x faster than the previous approach when processing large datasets.

  3. Reduced redundant transformations - Uses subscribeToState() instead of useState() to transform data only when it actually changes, avoiding 2-3 unnecessary transformation calls per update.

Files Changed

Modified Files

  • public/app/features/alerting/unified/triage/Workbench.tsx

    • Added isLoading prop support
    • Updated loading state handling
  • public/app/features/alerting/unified/triage/scene/AlertRuleSummary.tsx

    • Split component into AlertRuleSummaryViz (contains expensive hooks) and AlertRuleSummary (lazy-loading wrapper)
    • Implemented IntersectionObserver with 100px root margin to start loading when element is approaching viewport
    • Added fallback for browsers without IntersectionObserver support
  • public/app/features/alerting/unified/triage/scene/Workbench.tsx

    • Moved data transformation logic to separate file (dataTransform.ts)
    • Refactored to use subscribeToState() instead of useState() for more efficient state management
    • Added useTransition hook for non-blocking updates during data transformation
    • Improved loading state tracking

New Files

  • public/app/features/alerting/unified/triage/scene/dataTransform.ts

    • New optimized data transformation function convertToWorkbenchRows()
    • Single-pass algorithm that builds tree structure efficiently
    • Direct columnar access to field values for better performance
    • Handles grouping, empty values, and alert instance aggregation
  • public/app/features/alerting/unified/triage/scene/dataTransform.test.ts

    • Comprehensive test suite with 913+ lines of tests
    • Covers empty/invalid data handling, flat lists, single/multi-level grouping, empty values, and edge cases

Technical Details

IntersectionObserver Implementation

  • Uses 100px root margin to start loading when element is approaching viewport
  • Falls back to immediate rendering for browsers without IntersectionObserver support
  • Maintains layout space with placeholder div while not visible

Data Transformation Algorithm

  • Previous approach: Multiple passes through data, creating intermediate objects
  • New approach: Single-pass algorithm that builds tree structure directly
  • Performance: ~30x faster for large datasets
  • Uses direct columnar access to field values for optimal performance

State Management

  • Replaced useState() with subscribeToState() to avoid unnecessary re-renders
  • Uses useTransition() for non-blocking updates during expensive transformations
  • Only transforms data when actual data changes, not on every state update

@github-project-automation github-project-automation bot moved this to In review in Alerting Nov 4, 2025
@konrad147 konrad147 self-assigned this Nov 4, 2025
@konrad147 konrad147 added add to changelog no-backport Skip backport of PR labels Nov 4, 2025
@konrad147 konrad147 marked this pull request as ready for review November 4, 2025 12:46
@konrad147 konrad147 requested a review from a team as a code owner November 4, 2025 12:46
@konrad147 konrad147 requested review from gillesdemey, laurenashleigh and soniaAguilarPeiron and removed request for a team November 4, 2025 12:46
@github-actions github-actions bot added this to the 12.3.x milestone Nov 4, 2025
Comment on lines +33 to +39
const handleDrawerOpen = useCallback(() => {
setIsDrawerOpen(true);
};
}, []);

const handleDrawerClose = () => {
const handleDrawerClose = useCallback(() => {
setIsDrawerOpen(false);
};
}, []);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid OpenDrawerIconButton re-rendering

Comment on lines +41 to 62
useEffect(() => {
const transformData = (newState: typeof runner.state) => {
if (newState.data?.state !== 'Done' || !newState.data?.series) {
return;
}

// Get the groupBy from the scene directly to avoid having groupByVariable in the dependency array
let currentGroupByKeys: string[] = [];
const groupByVariable = sceneGraph.lookupVariable(VARIABLES.groupBy, runner);

if (groupByVariable && sceneUtils.isGroupByVariable(groupByVariable)) {
const value = groupByVariable.getValue();
if (Array.isArray(value)) {
currentGroupByKeys = value.map((value) => String(value));
}
}

const { series } = newState.data;
// Use transition for non-blocking update
startTransition(() => {
setRows(convertToWorkbenchRows(series, currentGroupByKeys));
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the way we convert time series to table rows. This process can be quite expensive for many rows and it's quite tricky to limit the number of recalculations relying only on const { data } = runner.useState(); because there are many things that are changing and triggering rerenders (filters, grouping, timerange)
The new approach is more performant, but also more complex.

Let me now what you think!

Comment on lines +48 to +56
let currentGroupByKeys: string[] = [];
const groupByVariable = sceneGraph.lookupVariable(VARIABLES.groupBy, runner);

if (groupByVariable && sceneUtils.isGroupByVariable(groupByVariable)) {
const value = groupByVariable.getValue();
if (Array.isArray(value)) {
currentGroupByKeys = value.map((value) => String(value));
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is run asynchronously (it's a callback when called when the new time series data is available) so using groupByKeys from line 20 could lead to reruning this effect multiple times and odd issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had written these tests first, for the old conversion algorithm. Then I swapped the algorithm to ensure it works the same way


// Single-pass implementation: 30x faster than previous approach
// Builds tree structure in one pass through data, avoiding intermediate row objects
export function convertToWorkbenchRows(series: DataFrame[], groupBy: string[] = []): WorkbenchRow[] {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This algorithm seem to be faster. I run a few benchmarks to compare its performance with the old one and I feel the results are worth the complexity

Test Old (ops/sec) Old (avg time) New (ops/sec) New (avg time) Speedup
No grouping 11.26 88.790 ms 384.62 2.600 ms 34.2x
Single groupBy (grafana_folder) 6.55 152.710 ms 59.31 16.860 ms 9.1x
Double groupBy (grafana_folder, alertstate) 6.13 163.020 ms 43.55 22.960 ms 7.1x

}

// Get required field value arrays (direct columnar access)
const alertnameIndex = fieldIndex.get('alertname');
Copy link
Contributor

Choose a reason for hiding this comment

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

Could these index values ever be 0? If so these will be falsey so may have to explicitly check if they're undefined?


const getStyles = (theme: GrafanaTheme2) => ({
iconButton: css({
transform: 'rotate(180deg)',
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we avoid calling useStyles on every render by using inline styling as the css is so simple?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

2 participants