Skip to content

feat(spans): Remove references to old -if syntax - #120686

Open
wmak wants to merge 1 commit into
masterfrom
wmak/feat/remove-references-to-old-syntax
Open

feat(spans): Remove references to old -if syntax#120686
wmak wants to merge 1 commit into
masterfrom
wmak/feat/remove-references-to-old-syntax

Conversation

@wmak

@wmak wmak commented Jul 27, 2026

Copy link
Copy Markdown
Member

- This removes the old if syntax from the prebuilt dashboards
@wmak
wmak requested a review from a team as a code owner July 27, 2026 20:05
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 27, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 459a17f. Configure here.

columns: [],
conditions: SPAN_OP_FILTER,
orderby: `equation|avg_if(${SpanFields.SPAN_DURATION},${SpanFields.SPAN_OP},equals,queue.process)`,
orderby: `equation|avg_if(\`${SpanFields.SPAN_OP}:queue.process,\`${SpanFields.SPAN_DURATION},)`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Malformed avg_if equation syntax

High Severity

The new avg_if expressions place backticks and commas incorrectly, producing a filter like `span.op:queue.process,`span.duration, instead of `span.op:queue.process`,span.duration. The same broken form appears in the Average Processing Time widget and Consumer Transactions table, while queues.ts and laravelOverview.ts use the correct syntax. Those queue detail widgets will fail to query.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 459a17f. Configure here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks valid, the backtick is after the coma

name: '',
fields: [
`equation|avg_if(${SpanFields.SPAN_DURATION},${SpanFields.SPAN_OP},equals,queue.process)`,
`equation|avg_if(\`${SpanFields.SPAN_OP}:queue.process,\`${SpanFields.SPAN_DURATION},)`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The avg_if function calls in queueDetails.ts have a misplaced backtick and an extra trailing comma, creating a malformed query string for the dashboard widgets.
Severity: HIGH

Suggested Fix

Correct the avg_if function calls in queueDetails.ts. Move the closing backtick to be before the comma separating the arguments, and remove the trailing comma after the SpanFields.SPAN_DURATION variable. The correct format should be `equation|avg_if(\`${SpanFields.SPAN_OP}:queue.process\`,${SpanFields.SPAN_DURATION})`. Apply this fix to all identified occurrences.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/views/dashboards/utils/prebuiltConfigs/queues/queueDetails.ts#L43

Potential issue: In `queueDetails.ts`, several `avg_if` function calls are constructed
with incorrect syntax. Specifically, the backtick closing the filter condition is placed
after the comma, and an extra trailing comma is added after the duration field. This
results in a malformed query string like ``
`equation|avg_if(\`span.op:queue.process,\`span.duration,)` `` instead of the correct ``
`equation|avg_if(\`span.op:queue.process\`,span.duration)` ``. This malformed query will
be sent to the backend, likely causing the "Average Processing Time" and "Consumer
Transactions" dashboard widgets to either fail with an error or display incorrect data.

Also affects:

  • static/app/views/dashboards/utils/prebuiltConfigs/queues/queueDetails.ts:180~180
  • static/app/views/dashboards/utils/prebuiltConfigs/queues/queueDetails.ts:187~187

Did we get this right? 👍 / 👎 to inform future reviews.

@DominikB2014 DominikB2014 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Asides from the small cursor comment, lgtm!

@getsantry

getsantry Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry Bot added the Stale label Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants