feat: updated the logic for variable update queue (#6586)

[SagarRajput-7]

Summary

Under this change I have introduced the following things -

• A new approach using topological sort to replace the current variable queue approach, which contains many issues and redundancy
• With the new approach, multiple customer issues around the dashboard not loading are fixed. (issues mentioned below)

For more info refer to the Technical Analysis and new approach block of this notion doc - https://www.notion.so/signoz/Dashboard-Variables-8783d02224c0483b9e8a4b8218cb5ddc?pvs=4#2c0617c157a141f5a09ba261d9d15d4d

Test cases -

Related Issues / PR's

• https://github.com/SigNoz/engineering-pod/issues/1978
• https://github.com/SigNoz/engineering-pod/issues/1965 - (slack thread - https://signoz-team.slack.com/archives/C02C7P6KE06/p1731299054009669)

Screenshots

Comparison between local and prod env

• Staging Env - (No throttling) -

Screen.Recording.2024-12-04.at.10.15.42.AM.mov

Affected Areas and Manually Tested Areas

Tested before and after of -

• API sequencing, multiselect - single select behaviour
• tried multiple reload to ensure correct loads
• made a dashboard to test the dependency updates - https://stagingapp.signoz.io/dashboard/80125672-1709-4b19-aa41-30b188395b06?relativeTime=30m
• Tested customer issues around dashboard loading (mentioned above and also see the videos added)

---

Important

Replaces variable update queue with topological sort in DashboardVariableSelection.tsx to fix dashboard loading issues and adds utility functions for dependency management.

• Behavior:
  • Replaces variable update queue with topological sort in DashboardVariableSelection.tsx.
  • Fixes dashboard loading issues by ensuring correct variable update order.
• Utilities:
  • Adds buildDependencies, buildDependencyGraph, buildParentDependencyGraph, and onUpdateVariableNode to util.ts for managing variable dependencies.
  • Implements checkAPIInvocation to determine when API calls should be made based on dependencies.
• Tests:
  • Updates VariableItem.test.tsx to test new dependency logic.
  • Adds dashboardVariables.test.tsx to test utility functions for dependency management.

^{This description was created by for f56d0dd. It will automatically update as commits are pushed.}

[github-actions]

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

[github-actions]

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to bc43433 in 2 minutes and 1 seconds

More details

• Looked at 1111 lines of code in 6 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx:143

• Draft comment:
  Ensure updatedVariables is correctly filtered to avoid duplicates when merging with prev.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The setVariablesToGetUpdated function is called with a new array that includes the current state and the updated variables. However, this can lead to duplicate entries if updatedVariables already contains elements from prev. Using a Set is a good approach to ensure uniqueness, but it's important to ensure that updatedVariables is correctly filtered before merging.

2. frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.tsx:194

• Draft comment:
  Optimize the enabled condition in useQuery to avoid unnecessary checks when variableData.type is not 'QUERY'.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The useQuery hook's enabled condition checks if variableData is defined and if its type is 'QUERY'. However, it also checks checkAPIInvocation, which might not be necessary if variableData is not of type 'QUERY'. This could be optimized by restructuring the conditions to avoid unnecessary checks.

Workflow ID: wflow_OSrRgxqa7YZ44T4s

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[github-actions]

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

[ellipsis-dev]

👍 Looks good to me! Incremental review on f56d0dd in 1 minute and 3 seconds

More details

• Looked at 128 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:42

• Draft comment:
  Consider using Array.prototype.filter instead of Array.prototype.map to directly filter variables based on the condition, which would eliminate the need for the subsequent filter call to remove null values.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The suggestion is technically valid - using filter() could be more concise. However, the current implementation is clear and follows a common pattern. The map+filter pattern allows for clearer separation between transformation and filtering logic. The performance difference would be negligible.
  The current code may actually be more readable by separating the transformation from filtering. The suggested change could make the code more dense and harder to follow.
  While filter() could work, the current implementation is perfectly valid and may even be more maintainable by clearly separating the transformation and filtering steps.
  This comment suggests a minor optimization that doesn't clearly improve code quality and could potentially reduce readability. The current implementation is fine as is.

2. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:76

• Draft comment:
  Consider initializing the graph with only 'QUERY' type variables to avoid confusion or errors related to non-QUERY variables having dependencies.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The current implementation initializes empty arrays for all named variables, but only QUERY types can have dependencies. However, non-QUERY variables can still be referenced by QUERY variables, so they need to be in the graph. The comment's suggestion could actually break functionality by excluding valid dependency targets.
  Maybe there's a performance benefit to only tracking QUERY variables? Or maybe having non-QUERY variables in the graph could cause issues I haven't considered?
  The performance impact would be negligible, and having non-QUERY variables in the graph is actually necessary since they can be dependency targets. The current implementation is correct.
  The comment should be deleted as it suggests a change that would break functionality by preventing non-QUERY variables from being dependency targets.

3. frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx:78

• Draft comment:
  Avoid using inline styles in React components. Consider using external stylesheets, CSS classes, or styled components instead.
• Reason this comment was not posted:
  Comment was not on a valid diff hunk.

Workflow ID: wflow_tuwO3ZNRGqkOvQnA

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[SagarRajput-7]

Failing Checks -

commitLint failing because of the length of one of the PR commit titles - "feat: add jest test cases for new logic's utils, functions and processors - dashboardVariables"

Jest Failing due to lower %