Compare commits

...

7 Commits

Author SHA1 Message Date
Vinícius Lourenço
4480087ef2 chore(docs): add authz guide 2026-07-28 14:56:49 -03:00
Gaurav Tewari
591837152e fix(warning-popover): background color to l2 & message alignment (#12302)
Some checks failed
build-staging / prepare (push) Has been cancelled
Release Drafter / update_release_draft (push) Has been cancelled
build-staging / js-build (push) Has been cancelled
build-staging / go-build (push) Has been cancelled
build-staging / staging (push) Has been cancelled
* fix: update css styles

* feat: add warning popover

* chore: revert bg color change

* feat: update styles

* chore: css conventios

---------

Co-authored-by: Gaurav Tewari <tewarig@users.noreply.github.com>
2026-07-28 17:16:05 +00:00
Pandey
2c5b4184c4 feat(querier): log user-authored clickhouse sql after running them through the parser (#12301)
Some checks failed
build-staging / prepare (push) Has been cancelled
build-staging / js-build (push) Has been cancelled
build-staging / go-build (push) Has been cancelled
build-staging / staging (push) Has been cancelled
Release Drafter / update_release_draft (push) Has been cancelled
* fix(querier): restrict user-authored clickhouse sql to read-only selects

Validate every user-authored ClickHouse statement before it runs: exactly one
statement, SELECT only, no table functions and no readonly override. Validation
runs on the rendered statement, since substituted variable values are user input
too.

Apply it to both entry points that reach the telemetry store with user SQL, the
clickhouse_sql query type in query_range and the dashboard variables query, and
replace the substring blacklist in the latter, which ran before substitution.

Also run these statements with readonly = 2 as a backstop. The connection is
shared with write paths, so it is opt-in per query and writers never set it.

* fix(querier): address review on clickhouse sql validation

Rename the validator to ValidateReadOnlySelect and collapse the multi-line error
constructions onto single lines.

Carry the parse failure in the message rather than as a wrapped cause. Neither
renderer surfaces the cause: render.Error reads only the message off the base
error, and RespondError reads Error(), which returns the cause alone and drops
the message. Both now show the same text with a 400.

Add unit tests covering statement kinds, table functions nested in joins, CTEs,
subqueries and unions, and readonly overrides.

* fix(querier): reject internal databases in user-authored clickhouse sql

Reading system or information_schema exposes grants, users and server metadata,
and ClickHouse read-only mode does not prevent it, so reject any table reference
into them.

Update the dashboard variables test for the new rejection message and cover both
a statement smuggled through a variable value and a system table read.

* fix(querier): reject unterminated block comments before parsing

The parser loops forever on an unterminated block comment, so validating a query
containing one would spin a request goroutine at full CPU instead of rejecting
it. Detect it up front, skipping comment markers that sit inside string literals.

Cover the parser panicking on a settings clause with no value in its own test,
which asserts the input still panics the parser so the case cannot quietly stop
exercising the recover.

* fix(querier): drop the block comment guard now the parser handles it

The parser no longer loops on an unterminated block comment, so the hand-rolled
scan that worked around it is redundant, and it was the riskier of the two: it
duplicated the lexer's handling of string literals and could have rejected a
legitimate query. Leave the parser as the single source of truth.

The panic case likewise no longer panics, so its test can no longer cover the
recover and is removed. The recover stays as insurance, since this reaches
user-authored SQL and the parser has regressed this way before.

Cover INTERSECT and EXCEPT, which the parser only started accepting in this
version and which reach a second query through the same rules.

* fix(querier): log clickhouse sql validation failures instead of rejecting

The parser's grammar has gaps against SQL that ClickHouse accepts, so rejecting
whatever it cannot read would break working dashboards and alerts. Sampling a
week of production clickhouse_sql found roughly one query in eighteen tripping
one of three gaps, none of them reading anything a telemetry query should not.

Log the failure with the rendered query instead, so the gaps can be told apart
from statements that genuinely break the rules before anything is enforced.

Wrap the parse error rather than formatting it in, so a caller can recover the
parser's *ParseError and read the position off it. The tests use that to pin the
construct each gap stops at, alongside the rewrite that the parser does accept.

* chore(querier): tighten the clickhouse sql validation comments

Condense the comments to single lines, move the recover note inside the defer it
explains, separate the visitor cases, and shorten the log message. Report how
many statements were found when rejecting a multi-statement query.

* fix(querier): log invalid clickhouse sql on the dashboard variables path too

The two callers disagreed: query_range logged and carried on, while dashboard
variables rejected. Given the parser trips on roughly one real query in eighteen,
that path could refuse a legitimate variable query, and being a rejection it left
nothing behind to show it had happened.

Both now go through LogIfStatementIsNotValid. prepareQuery is back to rendering
and nothing else, so the cases that expected it to police the statement move to
where the rules actually live.

* fix(querier): drop the read-only clickhouse session

Setting readonly on the session was defence in depth for a validator that now
only logs, so it guarded nothing while adding a context key and a settings branch
to a connection that is shared with every write path.

Restore the dashboard variables handler to what it was and call the validation
alongside, rather than reworking the rendering to accommodate it.

* chore(querybuilder): drop the redundant suffix from the failing case names

The test they sit in is already named _Fail.
2026-07-28 14:54:42 +00:00
Gaurav Tewari
0b69f5f677 fix: smooth Only/Toggle hover reveal in quick-filters checkbox (#12248)
The Only and Toggle buttons were revealed by overlapping hover triggers
(`.value` for Toggle, `.checkbox-value-section` for Only), so hovering the
label matched both rules and showed the two buttons side by side, shifting
the row layout — the visible hover "jerk".

- Scope the Toggle trigger to the checkbox (`.check-box:hover ~ ...`) so it
  no longer overlaps the label region; Only and Toggle are now mutually
  exclusive by cursor position.
- Stack both buttons in a single grid cell (`.value-actions`) so revealing
  one swaps in place instead of shifting the row.
- Add a fade + slide entry/exit animation (opacity + translateX with
  `@starting-style` and `display` `allow-discrete`), disabled under
  prefers-reduced-motion.

Co-authored-by: Gaurav Tewari <tewarig@users.noreply.github.com>
2026-07-28 12:21:39 +00:00
Abhi kumar
a28b95da35 fix(dashboard-v2): keep new-panel Save enabled after running the query (#12311)
* fix(dashboard-v2): keep new-panel Save enabled after running the query

A new panel's Save button re-disabled after Stage & Run: the query-dirty
baseline was derived from the live draft.spec.queries, which the run commits
the current query into, so the baseline drifted onto the just-run query and
isQueryDirty flipped back to false. Freeze the seed used for the new-panel
baseline so a run can't reset it. Existing panels are unaffected (they anchor
to savedQueries).

* fix(dashboard-v2): always allow Save and surface the real save-failure message

Two follow-ups to the new-panel Save regression:

- Decouple the Save button from the dirty check so it is never disabled on
  account of "no changes"; Save is gated only by read-only / in-flight. The
  dirty flag still drives the discard-on-close confirmation.
- On a failed save, show the server's error message in the toast description
  instead of only the generic "Failed to save panel".

* feat(dashboard-v2): show the "Unsaved Changes" badge only when the panel is dirty

Gate the panel-editor header badge on isDirty so it appears only when there
are unsaved edits, and import Badge from the @signozhq/ui/badge subpath.

* chore: replaced toast with the error modal

* fix: dashboard delete calling v1 api
2026-07-28 11:44:55 +00:00
Ashwin Bhatkal
b4b8a9fda0 fix(api): guard ErrorResponseHandlerV2 against non-envelope error bodies (#12209)
* fix(api): guard ErrorResponseHandlerV2 against non-envelope error bodies

The handler assumed every non-2xx response carried the V2 error envelope
and read response.data.error.code directly. During a deployment the
gateway returns a 5xx with an HTML/empty body, so response.data.error is
undefined and the handler threw its own TypeError, masking the real
failure and crashing the caller.

Type the inbound body as unknown and narrow it with an isErrorV2Resp type
guard; when the body isn't a V2 envelope, synthesize an APIError from the
HTTP status instead of throwing. Callers already pass
AxiosError<ErrorV2Resp>, which stays assignable to AxiosError<unknown>.

* refactor(api): keep ErrorV2Resp signature, launder response.data via unknown

Restore the AxiosError<ErrorV2Resp> signature so it documents that this is
the V2 error handler, and confine the runtime uncertainty to the one spot
that matters: launder response.data through a local unknown binding before
narrowing it with the isErrorV2Resp guard.

* fix(api): use UPSTREAM_UNAVAILABLE code for non-envelope error bodies

Address review: when the response body isn't a V2 envelope (e.g. a gateway
5xx during a deploy), throw a stable UPSTREAM_UNAVAILABLE code instead of
the stringified HTTP status, and trim the guard comment.

* fix(api): use UPSTREAM_UNAVAILABLE fallback code in convertToApiError

When the response body carries no error code, fall back to a stable
UPSTREAM_UNAVAILABLE code instead of a stringified HTTP status, mirroring
the ErrorResponseHandlerV2 fallback.

* test(dashboard-v2): update panelStatus fallback code to UPSTREAM_UNAVAILABLE

convertToApiError now returns UPSTREAM_UNAVAILABLE (not a stringified
status) when the response carries no error code; update the panelStatus
fallback assertion to match.

* fix(api): guard generated-API handler + strengthen V2 handler tests

- Guard the deprecated ErrorResponseHandlerForGeneratedAPIs against a
  non-envelope response body (gateway 5xx with HTML/empty body), mirroring
  ErrorResponseHandlerV2 — falls back to UPSTREAM_UNAVAILABLE instead of
  throwing on response.data.error.code.
- Parametrize the V2 handler tests into an { error, expected } table and
  assert the sub-error 'errors' messages, which several UI surfaces rely on.

* revert(api): drop generated-API handler guard from this PR

The deprecated ErrorResponseHandlerForGeneratedAPIs guard broke toAPIError's
defaultMessage fallback (which relied on the handler crashing), regressing the
error UX in ServiceAccount/Roles screens. Moved to a stacked PR + tracked in
engineering-pod#5761. Keeps this PR scoped to ErrorResponseHandlerV2 +
convertToApiError, and the strengthened V2 handler tests remain.

* fix(api): guard deprecated generated-API handler against non-envelope bodies (#12228)
2026-07-28 11:31:26 +00:00
Ashwin Bhatkal
de59c123e1 fix(metrics-explorer): fetch related dashboards from the v3 API (#12312)
The metric details drawer resolved "N dashboards" through
`GET /api/v2/metrics/dashboards`, whose backend walks the raw `data["widgets"]`
array and so only matches v1-schema dashboards. Since #12249 migrated stored
dashboards to the Perses v6 schema and made `/dashboard/:id` render V2
unconditionally, that endpoint has nothing left to match and the popover
silently shows no dashboards.

Switch to `useGetMetricDashboardsV2` (`GET /api/v3/metrics/dashboards`, added in
#11784), which parses the Perses spec. Response items change from
`MetricDashboard` (`widgetId`/`widgetName`) to `DashboardPanelRef`
(`panelId`/`panelName`); the popover only reads `dashboardId`/`dashboardName`,
so the render path is unchanged.

Alerts stay on `/api/v2/metrics/alerts` — there is no v3 counterpart, and it
reads the rule store rather than dashboard schema, so it is unaffected.
2026-07-28 10:33:56 +00:00
35 changed files with 1778 additions and 122 deletions

File diff suppressed because one or more lines are too long

After

Width:  |  Height:  |  Size: 145 KiB

File diff suppressed because one or more lines are too long

After

Width:  |  Height:  |  Size: 55 KiB

File diff suppressed because one or more lines are too long

After

Width:  |  Height:  |  Size: 88 KiB

File diff suppressed because one or more lines are too long

After

Width:  |  Height:  |  Size: 64 KiB

View File

@@ -0,0 +1,136 @@
# AuthZ Guide
How to structure a page so it works with the permission system.
We are migrating from the `ADMIN | EDITOR | VIEWER` roles to per-action checks. Instead of granting `VIEWER` and
exposing everything, a user can now be granted access to a single resource, eg: `Logs` only.
- [Prerequisites](#prerequisites)
- [Core rules](#core-rules)
- [Page patterns](#page-patterns)
- [What "blocked" means](#what-blocked-means)
- [Migration checklist](#migration-checklist)
- [Testing](#testing)
## Prerequisites
Check whether the resource your page represents is supported: see
[permissions.type.ts](../src/lib/authz/hooks/useAuthZ/permissions.type.ts) for the current resources and their allowed
verbs.
**If the resource is not listed there, skip authz for now**. The backend does not enforce it yet, so any frontend
check would be decorative. Revisit once the resource is generated into that file (it is auto-generated from the
backend).
## Core rules
These hold for every page. The per-pattern sections below only add to them.
1. **A page is always reachable, regardless of permission.** The intended action must be known before permission can
be checked, so the route renders first and individual pieces are gated.
2. **`update` requires both `read` and `update`.** A user who can write but cannot read the current value must not get
an edit affordance.
3. **`delete` is independent of `read`.** Delete controls stay visible for a user who holds `delete` but not `read`.
4. **Never gate per row.** If a user can `list`, render every row. Check `read` only when the row is opened (drawer or
detail route).
5. **Gate the narrowest thing that works**, a button over a section, a section over a page.
6. **A resource may be gated while a sub-resource is not.** A user without `read` on Service Accounts can still hold
`create` on API Keys, so blocking the outer container would hide work they are allowed to do.
7. **Verbs not covered here** (`attach`, `detach`, `assignee`) behave like `delete`: gate the control that triggers
them, not the surrounding content.
## Page patterns
### List page
![Visual rules for structuring a list page](./assets/list-example.svg)
Without `list`, but with any of `read` / `create` / `update`, only the table is blocked:
- Title, description, search filters and action buttons stay visible.
- Filters and any control that drives the table are non-interactive.
- The create button stays enabled if the user holds `create`.
### Edit page
![Visual rules for structuring an edit page](./assets/edit-example.svg)
Without `read`:
- Block the content with `withAuthZContent`, not the whole route.
- Keep delete visible (rule 3).
- Keep update blocked (rule 2).
### Drawer
![Visual rules for structuring a drawer](./assets/drawer-example.svg)
Without `read`:
- Block the drawer body, not the drawer itself.
- Prefer routing that carries the resource ID in the URL, so delete stays reachable without `read`.
- Keep update blocked (rule 2).
- Watch for sub-resources the user may still be allowed to act on (rule 6).
### Create
Without `create`:
| Entry point | Gate with |
| --------------------- | ------------------------------------------------------------- |
| Button | `AuthZButton` |
| Dedicated create page | `withAuthZPage` |
| Create drawer opened directly (deep link) | `withAuthZContent` on the body + `AuthZButton` on footer actions |
### Delete
Gate the control with `AuthZButton`, or `AuthZTooltip` for a non-button trigger such as an icon or menu item.
### Quick filters
![Visual rules for structuring a page with quick filters](./assets/quick-filters-example.svg)
## What "blocked" means
Blocking is always a visible denial, never a silent removal. Use the components in
[`lib/authz/components`](../src/lib/authz/components/README.md) rather than hand-rolling a check, they carry the
denial message and the loading state.
| Scope | Component | Denied state |
| --------------- | ----------------------------------------- | ------------------------------------------- |
| Button | `AuthZButton` | Disabled + tooltip |
| Any element | `AuthZTooltip` | Disabled child + tooltip |
| Section | `withAuthZContent` / `AuthZGuardContent` | Inline `PermissionDeniedCallout` |
| Page / route | `withAuthZPage` / `AuthZGuardPage` | `PermissionDeniedFullPage` |
| Custom fallback | `withAuthZ` / `AuthZGuard` | Whatever you pass as `fallback` |
Prefer the HOC (`withAuthZ*`); reach for the JSX guard (`AuthZGuard*`) when the gate depends on conditional rendering
and an HOC cannot express it. The components README has the full decision tree and how to build the `checks` array.
## Migration checklist
Use the existing components. Create a new one only if none fit.
- [ ] Can I apply a `withAuthZ*` variant directly, or must I extract the content into a component first?
- If extraction is needed, declare the new content component in the same file to keep the diff small, then move it
to its own file in a follow-up commit or PR.
- [ ] Is the layout structured to respect the [core rules](#core-rules)?
- If not, raise it in the frontend Slack channel before reshaping the page.
- [ ] Am I gating a shared component rather than a page or section?
- If so, it must stay functional with no permission. Example: the Query Builder works without field suggestions when
the user cannot read them.
## Testing
### Devtool
Press `Cmd + K` (`Ctrl + K` on Windows/Linux) to open the shortcuts, search for `AuthZ`, and pick the first
result. The devtool simulates granted and denied permissions across the UI.
Available only in local development, it is stripped from production builds.
### Unit tests
[`lib/authz/utils/README.md`](../src/lib/authz/utils/README.md) covers the `*.authz.test.tsx` naming convention, the
MSW handlers (`setupAuthzAdmin`, `setupAuthzDenyAll`, `setupAuthzDeny`, `setupAuthzAllow`,
`setupAuthzGrantByPrefix`), and how to test the loading state.

View File

@@ -2,6 +2,29 @@ import { RenderErrorResponseDTO } from 'api/generated/services/sigNoz.schemas';
import { AxiosError } from 'axios';
import APIError from 'types/api/error';
// The wire shape these handlers can actually rely on. The generated
// RenderErrorResponseDTO marks code/message/url/errors as required, but the
// server omits any of them even on valid errors (e.g. a 400 with just a
// message), so a present `error` object is all the guard can guarantee.
type ErrorEnvelope = {
error: {
code?: string;
message?: string;
url?: string;
errors?: { message?: string }[];
};
};
function isErrorEnvelope(data: unknown): data is ErrorEnvelope {
return (
typeof data === 'object' &&
data !== null &&
'error' in data &&
typeof (data as ErrorEnvelope).error === 'object' &&
(data as ErrorEnvelope).error !== null
);
}
// @deprecated Use convertToApiError instead
export function ErrorResponseHandlerForGeneratedAPIs(
error: AxiosError<RenderErrorResponseDTO>,
@@ -10,15 +33,29 @@ export function ErrorResponseHandlerForGeneratedAPIs(
// The request was made and the server responded with a status code
// that falls out of the range of 2xx
if (response) {
// The body isn't guaranteed to be an error envelope — e.g. a gateway 5xx
// with an HTML/empty body during a deploy. Verify the shape before reading
// it; otherwise synthesize a consistent error from the status.
const data: unknown = response.data;
if (isErrorEnvelope(data)) {
const { code, message, url, errors } = data.error;
throw new APIError({
httpStatusCode: response.status || 500,
error: {
code: code ?? '',
message: message ?? '',
url: url ?? '',
errors: (errors ?? []).map((e) => ({ message: e.message ?? '' })),
},
});
}
throw new APIError({
httpStatusCode: response.status || 500,
error: {
code: response.data.error.code,
message: response.data.error.message,
url: response.data.error.url ?? '',
errors: (response.data.error.errors ?? []).map((e) => ({
message: e.message ?? '',
})),
code: 'UPSTREAM_UNAVAILABLE',
message: error.message || 'Something went wrong',
url: '',
errors: [],
},
});
}
@@ -62,9 +99,7 @@ export function convertToApiError(
return new APIError({
httpStatusCode: response?.status || error.status || 500,
error: {
code:
errorData?.code ||
String(response?.status || error.code || 'unknown_error'),
code: errorData?.code || 'UPSTREAM_UNAVAILABLE',
message:
errorData?.message ||
response?.statusText ||

View File

@@ -2,19 +2,38 @@ import { AxiosError } from 'axios';
import { ErrorV2Resp } from 'types/api';
import APIError from 'types/api/error';
function isErrorV2Resp(data: unknown): data is ErrorV2Resp {
return (
typeof data === 'object' &&
data !== null &&
'error' in data &&
typeof (data as ErrorV2Resp).error?.code === 'string'
);
}
// reference - https://axios-http.com/docs/handling_errors
export function ErrorResponseHandlerV2(error: AxiosError<ErrorV2Resp>): never {
const { response, request } = error;
// The request was made and the server responded with a status code
// that falls out of the range of 2xx
if (response) {
// response.data isn't guaranteed to be a V2 envelope (e.g. a gateway 5xx
// with an HTML/empty body during a deploy), so verify the shape first.
const data: unknown = response.data;
if (isErrorV2Resp(data)) {
const { code, message, url, errors } = data.error;
throw new APIError({
httpStatusCode: response.status || 500,
error: { code, message, url, errors },
});
}
throw new APIError({
httpStatusCode: response.status || 500,
error: {
code: response.data.error.code,
message: response.data.error.message,
url: response.data.error.url,
errors: response.data.error.errors,
code: 'UPSTREAM_UNAVAILABLE',
message: error.message || 'Something went wrong',
url: '',
errors: [],
},
});
}

View File

@@ -0,0 +1,126 @@
import { ErrorResponseHandlerV2 } from 'api/ErrorResponseHandlerV2';
import { AxiosError } from 'axios';
import { ErrorV2Resp } from 'types/api';
import APIError from 'types/api/error';
function asAxiosError(partial: Partial<AxiosError>): AxiosError<ErrorV2Resp> {
return partial as AxiosError<ErrorV2Resp>;
}
// ErrorResponseHandlerV2 always throws — capture the APIError so assertions stay
// unconditional.
function runHandler(error: AxiosError<ErrorV2Resp>): APIError {
try {
ErrorResponseHandlerV2(error);
} catch (thrown) {
return thrown as APIError;
}
throw new Error('expected ErrorResponseHandlerV2 to throw');
}
type ExpectedError = {
httpStatusCode: number;
code: string;
message: string;
errors: { message: string }[];
};
// One row per response shape the handler must normalize. New shapes (with
// different bodies) can be added here without a new test block.
const cases: {
name: string;
error: AxiosError<ErrorV2Resp>;
expected: ExpectedError;
}[] = [
{
name: 'well-formed V2 error envelope',
error: asAxiosError({
message: 'Request failed with status code 400',
response: {
status: 400,
data: {
error: {
code: 'bad_request',
message: 'Invalid dashboard payload',
url: 'https://signoz.io/docs',
errors: [{ message: 'name is required' }, { message: 'name too long' }],
},
},
} as AxiosError['response'],
}),
expected: {
httpStatusCode: 400,
code: 'bad_request',
message: 'Invalid dashboard payload',
errors: [{ message: 'name is required' }, { message: 'name too long' }],
},
},
{
// Regression: during a deployment the gateway returns a 5xx with a
// non-envelope body. Reading response.data.error.code used to throw a
// TypeError from inside the handler itself. See engineering-pod#5760.
name: '5xx with a non-envelope HTML body',
error: asAxiosError({
message: 'Request failed with status code 503',
response: {
status: 503,
data: '<html><body>503 Service Temporarily Unavailable</body></html>',
} as AxiosError['response'],
}),
expected: {
httpStatusCode: 503,
code: 'UPSTREAM_UNAVAILABLE',
message: 'Request failed with status code 503',
errors: [],
},
},
{
name: '5xx with an empty body',
error: asAxiosError({
message: 'Request failed with status code 502',
response: {
status: 502,
data: undefined,
} as AxiosError['response'],
}),
expected: {
httpStatusCode: 502,
code: 'UPSTREAM_UNAVAILABLE',
message: 'Request failed with status code 502',
errors: [],
},
},
{
name: 'no response received (network error)',
error: asAxiosError({
message: 'Network Error',
code: 'ERR_NETWORK',
name: 'AxiosError',
request: {},
}),
expected: {
httpStatusCode: 500,
code: 'ERR_NETWORK',
message: 'Network Error',
errors: [],
},
},
];
describe('ErrorResponseHandlerV2', () => {
it.each(cases)(
'normalizes $name into a consistent APIError',
({ error, expected }) => {
const apiError = runHandler(error);
expect(apiError).toBeInstanceOf(APIError);
expect(apiError.getHttpStatusCode()).toBe(expected.httpStatusCode);
expect(apiError.getErrorCode()).toBe(expected.code);
expect(apiError.getErrorMessage()).toBe(expected.message);
// The sub-error messages feed several parts of the UI, so assert them.
expect(apiError.getErrorDetails().error.errors).toStrictEqual(
expected.errors,
);
},
);
});

View File

@@ -92,52 +92,76 @@
color: var(--l3-foreground);
}
.only-btn {
cursor: not-allowed;
color: var(--l3-foreground);
}
.only-btn,
.toggle-btn {
cursor: not-allowed;
color: var(--l3-foreground);
}
}
.only-btn {
display: none;
// Stack "Only"/"All" and "Toggle" in a single cell so revealing
// one swaps in place instead of shifting the row layout.
.value-actions {
display: grid;
align-items: center;
justify-items: end;
> * {
grid-area: 1 / 1;
}
}
.only-btn,
.toggle-btn {
display: none;
}
.toggle-btn:hover {
background-color: unset;
}
.only-btn:hover {
background-color: unset;
}
}
.checkbox-value-section:hover {
.toggle-btn {
display: none;
}
.only-btn {
display: flex;
align-items: center;
justify-content: center;
height: 21px;
opacity: 0;
transform: translateX(4px);
// `display` is animated as a discrete property so the button
// stays mounted through its fade-out on exit.
transition:
opacity 0.16s ease,
transform 0.16s ease,
display 0.16s allow-discrete;
&:hover {
background-color: unset;
}
}
}
// Hovering the label/value area reveals the "Only"/"All" action.
.checkbox-value-section:hover .only-btn {
display: flex;
opacity: 1;
transform: translateX(0);
// Entry animation start state (fires on the display switch).
@starting-style {
opacity: 0;
transform: translateX(4px);
}
}
// Hovering the checkbox reveals the "Toggle" action.
.check-box:hover ~ .checkbox-value-section .toggle-btn {
display: flex;
opacity: 1;
transform: translateX(0);
@starting-style {
opacity: 0;
transform: translateX(4px);
}
}
}
.value:hover {
@media (prefers-reduced-motion: reduce) {
.only-btn,
.toggle-btn {
display: flex;
align-items: center;
justify-content: center;
height: 21px;
transition: none;
}
}
}

View File

@@ -53,12 +53,14 @@ function CheckboxValueRow({
</Typography.Text>
</TooltipSimple>
)}
<Button type="text" className="only-btn">
{onlyButtonLabel}
</Button>
<Button type="text" className="toggle-btn">
Toggle
</Button>
<div className="value-actions">
<Button type="text" className="only-btn">
{onlyButtonLabel}
</Button>
<Button type="text" className="toggle-btn">
Toggle
</Button>
</div>
</div>
</div>
);

View File

@@ -1,6 +1,11 @@
.warning-popover-overlay {
--antd-arrow-background-color: var(--l2-background);
}
.warning-content {
display: flex;
flex-direction: column;
background-color: var(--l2-background);
// === SECTION: Summary (Top)
&__summary-section {
@@ -10,16 +15,25 @@
&__summary {
display: flex;
align-items: flex-start;
justify-content: space-between;
gap: var(--spacing-8);
padding: 16px;
}
&__summary-left {
display: flex;
align-items: baseline;
align-items: flex-start;
gap: 8px;
}
&__icon-wrapper {
display: flex;
align-items: center;
flex-shrink: 0;
height: var(--spacing-12);
}
&__summary-text {
display: flex;
flex-direction: column;

View File

@@ -2,6 +2,7 @@ import { ReactNode, useMemo } from 'react';
import { Color } from '@signozhq/design-tokens';
import { Button, Popover, PopoverProps } from 'antd';
import ErrorIcon from 'assets/Error';
import cx from 'classnames';
import OverlayScrollbar from 'components/OverlayScrollbar/OverlayScrollbar';
import { BookOpenText, ChevronsDown, TriangleAlert } from '@signozhq/icons';
import KeyValueLabel from 'periscope/components/KeyValueLabel';
@@ -30,16 +31,18 @@ export function WarningContent({ warning }: WarningContentProps): JSX.Element {
{/* Summary Header */}
<section className="warning-content__summary-section">
<header className="warning-content__summary">
<div className="warning-content__summary-left">
<div className="warning-content__icon-wrapper">
<ErrorIcon />
</div>
{(warningCode || warningMessage) && (
<div className="warning-content__summary-left">
<div className="warning-content__icon-wrapper">
<ErrorIcon />
</div>
<div className="warning-content__summary-text">
<h2 className="warning-content__warning-code">{warningCode}</h2>
<p className="warning-content__warning-message">{warningMessage}</p>
<div className="warning-content__summary-text">
<h2 className="warning-content__warning-code">{warningCode}</h2>
<p className="warning-content__warning-message">{warningMessage}</p>
</div>
</div>
</div>
)}
{warningUrl && (
<div className="warning-content__summary-right">
@@ -154,6 +157,10 @@ function WarningPopover({
overlayInnerStyle={{ padding: 0 }}
autoAdjustOverflow
{...popoverProps}
overlayClassName={cx(
'warning-popover-overlay',
popoverProps.overlayClassName,
)}
>
{children || (
<TriangleAlert

View File

@@ -6,7 +6,7 @@ import { Skeleton } from 'antd';
import { Typography } from '@signozhq/ui/typography';
import {
useGetMetricAlerts,
useGetMetricDashboards,
useGetMetricDashboardsV2,
} from 'api/generated/services/metrics';
import { QueryParams } from 'constants/query';
import ROUTES from 'constants/routes';
@@ -38,7 +38,7 @@ function DashboardsAndAlertsPopover({
data: dashboardsData,
isLoading: isLoadingDashboards,
isError: isErrorDashboards,
} = useGetMetricDashboards(
} = useGetMetricDashboardsV2(
{
metricName,
},
@@ -55,7 +55,8 @@ function DashboardsAndAlertsPopover({
const dashboards = useMemo(() => {
const currentDashboards = dashboardsData?.data.dashboards ?? [];
// Remove duplicate dashboards
// The API returns one entry per referencing panel, so a dashboard repeats
// once per panel that uses the metric.
return currentDashboards.filter(
(dashboard, index, self) =>
index === self.findIndex((t) => t.dashboardId === dashboard.dashboardId),

View File

@@ -23,7 +23,7 @@ const useGetMetricAlertsMock = jest.spyOn(
);
const useGetMetricDashboardsMock = jest.spyOn(
metricsExplorerHooks,
'useGetMetricDashboards',
'useGetMetricDashboardsV2',
);
describe('DashboardsAndAlertsPopover', () => {
@@ -153,11 +153,15 @@ describe('DashboardsAndAlertsPopover', () => {
);
});
it('renders unique dashboards even when there are duplicates', async () => {
it('collapses multiple panels of the same dashboard into one entry', async () => {
useGetMetricDashboardsMock.mockReturnValue(
getMockDashboardsData({
data: {
dashboards: [MOCK_DASHBOARD_1, MOCK_DASHBOARD_2, MOCK_DASHBOARD_1],
dashboards: [
MOCK_DASHBOARD_1,
MOCK_DASHBOARD_2,
{ ...MOCK_DASHBOARD_1, panelId: '3', panelName: 'Panel 3' },
],
},
}),
);

View File

@@ -2,7 +2,7 @@ import * as metricsExplorerHooks from 'api/generated/services/metrics';
import {
GetMetricAlerts200,
GetMetricAttributes200,
GetMetricDashboards200,
GetMetricDashboardsV2200,
GetMetricHighlights200,
GetMetricMetadata200,
MetrictypesTemporalityDTO,
@@ -40,14 +40,14 @@ export function getMockMetricHighlightsData(
export const MOCK_DASHBOARD_1 = {
dashboardName: 'Dashboard 1',
dashboardId: '1',
widgetId: '1',
widgetName: 'Widget 1',
panelId: '1',
panelName: 'Panel 1',
};
export const MOCK_DASHBOARD_2 = {
dashboardName: 'Dashboard 2',
dashboardId: '2',
widgetId: '2',
widgetName: 'Widget 2',
panelId: '2',
panelName: 'Panel 2',
};
export const MOCK_ALERT_1 = {
alertName: 'Alert 1',
@@ -59,7 +59,7 @@ export const MOCK_ALERT_2 = {
};
export function getMockDashboardsData(
overrides?: Partial<GetMetricDashboards200>,
overrides?: Partial<GetMetricDashboardsV2200>,
{
isLoading = false,
isError = false,
@@ -67,7 +67,7 @@ export function getMockDashboardsData(
isLoading?: boolean;
isError?: boolean;
} = {},
): ReturnType<typeof metricsExplorerHooks.useGetMetricDashboards> {
): ReturnType<typeof metricsExplorerHooks.useGetMetricDashboardsV2> {
return {
data: {
data: {
@@ -79,7 +79,7 @@ export function getMockDashboardsData(
isLoading,
isError,
} as ReturnType<typeof metricsExplorerHooks.useGetMetricDashboards>;
} as ReturnType<typeof metricsExplorerHooks.useGetMetricDashboardsV2>;
}
export function getMockAlertsData(

View File

@@ -11,3 +11,9 @@
border: 1px solid var(--l3-border) !important;
box-shadow: none !important;
}
/* Highlights the dashboard name inside the delete-confirm title. */
.deleteName {
color: var(--danger-background);
font-weight: var(--font-weight-medium);
}

View File

@@ -27,16 +27,13 @@ import logEvent from 'api/common/logEvent';
import { cloneDashboardV2 } from 'api/generated/services/dashboard';
import type { DashboardtypesGettableDashboardV2DTO } from 'api/generated/services/sigNoz.schemas';
import ROUTES from 'constants/routes';
import { useDeleteDashboard } from 'hooks/dashboard/useDeleteDashboard';
import { useSafeNavigate } from 'hooks/useSafeNavigate';
import history from 'lib/history';
import { DashboardDetailEvents } from 'pages/DashboardPageV2/constants/events';
import { useAppContext } from 'providers/App/App';
import { useErrorModal } from 'providers/ErrorModalProvider';
import APIError from 'types/api/error';
import { USER_ROLES } from 'types/roles';
import ConfirmDeleteDialog from '../../components/ConfirmDeleteDialog/ConfirmDeleteDialog';
import DisabledControlTooltip from '../../components/DisabledControlTooltip/DisabledControlTooltip';
import DisabledMenuItemLabel from '../../components/DisabledMenuItemLabel/DisabledMenuItemLabel';
import DashboardSettings from '../../DashboardSettings';
@@ -45,6 +42,7 @@ import SectionTitleModal from '../../PanelsAndSectionsLayout/Section/SectionTitl
import JsonEditorDrawer from '../JsonEditorDrawer/JsonEditorDrawer';
import SettingsDrawer from '../SettingsDrawer';
import styles from './DashboardActions.module.scss';
import { useDeleteDashboardAction } from './useDeleteDashboardAction';
import { DASHBOARD_LOCKED_REASON } from '../../hooks/useDashboardEditGuard';
import { useDashboardStore } from '../../store/useDashboardStore';
@@ -84,8 +82,12 @@ function DashboardActions({
const [isCloning, setIsCloning] = useState<boolean>(false);
const [isNewSectionOpen, setIsNewSectionOpen] = useState<boolean>(false);
const [isDeleteOpen, setIsDeleteOpen] = useState<boolean>(false);
const deleteDashboardMutation = useDeleteDashboard(dashboard.id);
const { contextHolder: deleteConfirmHolder, confirmDeleteDashboard } =
useDeleteDashboardAction({
dashboardId: dashboard.id,
dashboardName: title,
panelCount: Object.keys(dashboard.spec.panels).length,
});
// Open the settings drawer when something in the tree requests it (e.g. the
// variables bar's "Add variable" button).
@@ -132,19 +134,6 @@ function DashboardActions({
}
}, [dashboard.id, title, safeNavigate, showErrorModal]);
const handleConfirmDelete = useCallback((): void => {
deleteDashboardMutation.mutate(undefined, {
onSuccess: () => {
void logEvent(DashboardDetailEvents.Deleted, {
dashboardId: dashboard.id,
panelCount: Object.keys(dashboard.spec.panels).length,
});
setIsDeleteOpen(false);
history.replace(ROUTES.ALL_DASHBOARD);
},
});
}, [deleteDashboardMutation, dashboard.id, dashboard.spec.panels]);
const handleOpenSettings = useCallback((): void => {
void logEvent(DashboardDetailEvents.SettingsOpened, {
dashboardId: dashboard.id,
@@ -248,7 +237,7 @@ function DashboardActions({
icon: <Trash2 size={14} />,
danger: true,
disabled: isLocked,
onClick: (): void => setIsDeleteOpen(true),
onClick: confirmDeleteDashboard,
},
);
}
@@ -266,6 +255,7 @@ function DashboardActions({
handleClone,
onLockToggle,
handleEnterFullScreen,
confirmDeleteDashboard,
]);
return (
@@ -348,14 +338,7 @@ function DashboardActions({
isOpen={isJsonEditorOpen}
onClose={(): void => setIsJsonEditorOpen(false)}
/>
<ConfirmDeleteDialog
open={isDeleteOpen}
title={`Delete dashboard"?`}
description={`Are you sure you want to delete this dashboard - "${title}"? This action cannot be undone.`}
isLoading={deleteDashboardMutation.isLoading}
onConfirm={handleConfirmDelete}
onClose={(): void => setIsDeleteOpen(false)}
/>
{deleteConfirmHolder}
<SectionTitleModal
open={isNewSectionOpen}
heading="New section"

View File

@@ -0,0 +1,89 @@
import { type ReactNode, useCallback } from 'react';
import { useQueryClient } from 'react-query';
import { toast } from '@signozhq/ui/sonner';
import { Typography } from '@signozhq/ui/typography';
import logEvent from 'api/common/logEvent';
import {
invalidateListDashboardsForUserV2,
useDeleteDashboardV2,
} from 'api/generated/services/dashboard';
import { useDeleteConfirm } from 'components/DeleteConfirmModal/useDeleteConfirm';
import ROUTES from 'constants/routes';
import { useDashboardPreferencesStore } from 'hooks/dashboard/useDashboardPreference';
import history from 'lib/history';
import { DashboardDetailEvents } from 'pages/DashboardPageV2/constants/events';
import { useErrorModal } from 'providers/ErrorModalProvider';
import APIError from 'types/api/error';
import styles from './DashboardActions.module.scss';
interface UseDeleteDashboardActionArgs {
dashboardId: string;
dashboardName: string;
panelCount: number;
}
interface UseDeleteDashboardAction {
/** Must be rendered in the calling component for the modal to appear. */
contextHolder: ReactNode;
confirmDeleteDashboard: () => void;
}
/**
* Deletes the open dashboard through the v2 endpoint behind the shared
* destructive confirmation, then drops its local preferences, refreshes the
* dashboard list cache and sends the user back to the list.
*/
export function useDeleteDashboardAction({
dashboardId,
dashboardName,
panelCount,
}: UseDeleteDashboardActionArgs): UseDeleteDashboardAction {
const queryClient = useQueryClient();
const { showErrorModal } = useErrorModal();
const { contextHolder, confirmDelete } = useDeleteConfirm();
const removePreferences = useDashboardPreferencesStore(
(state) => state.removePreferences,
);
const { mutate: deleteDashboard } = useDeleteDashboardV2({
mutation: {
onSuccess: async (): Promise<void> => {
void logEvent(DashboardDetailEvents.Deleted, { dashboardId, panelCount });
removePreferences(dashboardId);
await invalidateListDashboardsForUserV2(queryClient);
toast.success('Dashboard deleted successfully');
history.replace(ROUTES.ALL_DASHBOARD);
},
onError: (error: unknown): void => {
showErrorModal(error as APIError);
},
},
});
const confirmDeleteDashboard = useCallback((): void => {
confirmDelete({
title: (
<Typography.Title level={5}>
Are you sure you want to delete the
<Typography.Text className={styles.deleteName}>
{' '}
{dashboardName}{' '}
</Typography.Text>
dashboard?
</Typography.Title>
),
content: 'This action cannot be undone.',
// Keeps the Delete button loading until the mutation settles, then closes.
onConfirm: () =>
new Promise<void>((resolve) => {
deleteDashboard(
{ pathParams: { id: dashboardId } },
{ onSettled: () => resolve() },
);
}),
});
}, [confirmDelete, dashboardName, deleteDashboard, dashboardId]);
return { contextHolder, confirmDeleteDashboard };
}

View File

@@ -1,5 +1,6 @@
import { useCallback } from 'react';
import { SolidAlertTriangle, X } from '@signozhq/icons';
import { Badge } from '@signozhq/ui/badge';
import { Button } from '@signozhq/ui/button';
import { DialogWrapper } from '@signozhq/ui/dialog';
import { Divider } from '@signozhq/ui/divider';
@@ -13,6 +14,7 @@ import DisabledControlTooltip from '../../components/DisabledControlTooltip/Disa
import styles from './Header.module.scss';
interface HeaderProps {
/** Unsaved edits exist — shows the "Unsaved Changes" badge and gates the discard confirmation on close (not the Save button). */
isDirty: boolean;
isSaving: boolean;
showSwitchToView?: boolean;
@@ -66,6 +68,11 @@ function Header({
/>
<Divider type="vertical" />
<Typography.Text>Configure panel</Typography.Text>
{isDirty && (
<Badge color="warning" data-testid="panel-editor-v2-unsaved-badge">
Unsaved Changes
</Badge>
)}
</div>
<div className={styles.actions}>
<HeaderRightSection
@@ -88,7 +95,7 @@ function Header({
variant="solid"
color="primary"
data-testid="panel-editor-v2-save"
disabled={readOnly || !isDirty || isSaving}
disabled={readOnly || isSaving}
loading={!readOnly && isSaving}
onClick={readOnly ? undefined : onSave}
>

View File

@@ -1,3 +1,4 @@
import type { ComponentProps } from 'react';
import { MemoryRouter } from 'react-router-dom';
import { render, screen } from '@testing-library/react';
import { TooltipProvider } from '@signozhq/ui/tooltip';
@@ -23,7 +24,9 @@ jest.mock('api/common/logEvent', () => ({
const mockUseIsAIAssistantEnabled = useIsAIAssistantEnabled as jest.Mock;
function renderHeader(): void {
function renderHeader(
props: Partial<ComponentProps<typeof Header>> = {},
): void {
// AppLayout supplies the TooltipProvider in the app; the header is rendered bare here.
render(
<MemoryRouter>
@@ -33,6 +36,7 @@ function renderHeader(): void {
isSaving={false}
onSave={jest.fn()}
onClose={jest.fn()}
{...props}
/>
</TooltipProvider>
</MemoryRouter>,
@@ -66,4 +70,34 @@ describe('PanelEditor Header', () => {
).not.toBeInTheDocument();
expect(screen.getByTestId('panel-editor-v2-save')).toBeInTheDocument();
});
it('keeps Save enabled even when there are no unsaved edits', () => {
mockUseIsAIAssistantEnabled.mockReturnValue(false);
renderHeader({ isDirty: false });
expect(screen.getByTestId('panel-editor-v2-save')).toBeEnabled();
});
it('disables Save only while read-only or saving', () => {
mockUseIsAIAssistantEnabled.mockReturnValue(false);
renderHeader({ isDirty: true, readOnly: true, readOnlyReason: 'Locked' });
expect(screen.getByTestId('panel-editor-v2-save')).toBeDisabled();
});
it('shows the Unsaved Changes badge only when there are unsaved edits', () => {
mockUseIsAIAssistantEnabled.mockReturnValue(false);
renderHeader({ isDirty: false });
expect(
screen.queryByTestId('panel-editor-v2-unsaved-badge'),
).not.toBeInTheDocument();
renderHeader({ isDirty: true });
expect(
screen.getByTestId('panel-editor-v2-unsaved-badge'),
).toBeInTheDocument();
});
});

View File

@@ -1,5 +1,6 @@
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { toast } from '@signozhq/ui/sonner';
import type { DashboardtypesPanelDTO } from 'api/generated/services/sigNoz.schemas';
import { PANEL_TYPES } from 'constants/queryBuilder';
import { getPanelDefinition } from 'pages/DashboardPageV2/DashboardContainer/Panels/registry';
@@ -100,6 +101,12 @@ jest.mock('@signozhq/ui/resizable', () => ({
jest.mock('@signozhq/ui/sonner', () => ({
toast: { success: jest.fn(), error: jest.fn() },
}));
const mockShowErrorModal = jest.fn();
jest.mock('providers/ErrorModalProvider', () => ({
useErrorModal: (): { showErrorModal: jest.Mock } => ({
showErrorModal: mockShowErrorModal,
}),
}));
// Children mocked to capture props (and expose a Save trigger / footer slot).
const mockHeaderProps = jest.fn();
@@ -261,13 +268,13 @@ describe('PanelEditorContainer composition', () => {
);
});
it('keeps a query-less new panel unsaveable but still serializes its seed query', () => {
it('keeps a query-less new panel clean but still serializes its seed query', () => {
setup(makePanel('signoz/TimeSeriesPanel'), { isNew: true });
expect(mockUseQuerySync).toHaveBeenCalledWith(
expect.objectContaining({ alwaysSerializeQuery: true }),
);
// No query and no edits yet → nothing to save, so Save stays disabled.
// No query and no edits yet → not dirty, so closing won't prompt to discard.
expect(mockHeaderProps).toHaveBeenCalledWith(
expect.objectContaining({ isDirty: false }),
);
@@ -290,7 +297,7 @@ describe('PanelEditorContainer composition', () => {
);
});
it('marks a new panel that already has a query saveable (e.g. list auto-runs one)', () => {
it('marks a new panel that already has a query dirty (e.g. list auto-runs one)', () => {
const seededQuery = {
spec: { plugin: { kind: 'signoz/BuilderQuery', spec: { signal: 'logs' } } },
};
@@ -327,6 +334,24 @@ describe('PanelEditorContainer composition', () => {
expect(mockSave).toHaveBeenCalledWith(panel.spec);
});
it('surfaces a save failure through the error modal', async () => {
// The raw thrown value flows straight to the modal, which normalizes it to an
// APIError itself (that normalization is covered by ErrorModalProvider's tests).
const failure = {
response: {
status: 400,
data: { error: { code: 'INVALID', message: 'Panel name already exists' } },
},
};
mockSave.mockRejectedValueOnce(failure);
setup(makePanel('signoz/TimeSeriesPanel'));
await userEvent.click(screen.getByTestId('editor-save'));
await waitFor(() => expect(mockShowErrorModal).toHaveBeenCalledWith(failure));
expect(toast.error).not.toHaveBeenCalled();
});
it('marks the saved panel to be scrolled into view on the dashboard', async () => {
setup(makePanel('signoz/TimeSeriesPanel'));

View File

@@ -117,6 +117,49 @@ describe('usePanelEditorQuerySync (real query builder)', () => {
},
);
it('a NEW panel stays query-dirty after Stage & Run commits the edited query into the draft', async () => {
// Repro of the P0: new panel → edit → Run commits the query into the draft; the
// dirty baseline must not drift onto it, or Save re-disables.
const editedInUrl: Query = {
...initialQueriesMap[DataSource.METRICS],
id: 'edited-new-panel',
builder: {
...initialQueriesMap[DataSource.METRICS].builder,
queryData: [
{
...initialQueriesMap[DataSource.METRICS].builder.queryData[0],
legend: 'edited-legend',
},
],
},
};
const committed = toPerses(editedInUrl, panelType);
const { result, rerender } = renderHook(
({ draftQueries }: { draftQueries: DashboardtypesQueryDTO[] }) =>
usePanelEditorQuerySync({
draft: makePanel(draftQueries),
panelType,
setSpec: jest.fn(),
refetch: jest.fn(),
alwaysSerializeQuery: true,
signal: DataSource.METRICS as unknown as TelemetrytypesSignalDTO,
// savedQueries omitted — new panel.
}),
{
wrapper: makeUrlWrapper(editedInUrl),
initialProps: { draftQueries: [] as DashboardtypesQueryDTO[] },
},
);
// The edited query (from the URL) diverges from the seeded default → dirty.
await waitFor(() => expect(result.current.isQueryDirty).toBe(true));
// Stage & Run commits the edited query into the draft → must stay dirty.
rerender({ draftQueries: committed });
expect(result.current.isQueryDirty).toBe(true);
});
it('an untouched panel with a minimal/older stored query is NOT dirty (drift fix)', async () => {
// An older saved query carries only a few fields; the builder re-emits many more
// (source, stepInterval, filter, spaceAggregation, …). Comparing raw would read

View File

@@ -79,6 +79,11 @@ export function usePanelEditorQuerySync({
// in-editor edit in the URL survives a refresh / browser Back-Forward.
useShareBuilderUrl({ defaultValue: seedQuery });
// Frozen seed for the new-panel dirty baseline: Stage & Run commits the live query
// into `draft.spec.queries`, so re-reading `seedQuery` would drift the baseline onto
// the run query and Save would re-disable right after a run.
const initialSeedRef = useRef(seedQuery);
// Commit the live query into the draft (what the preview fetches).
const commitQuery = useCallback(
(query: Query): boolean => {
@@ -152,16 +157,19 @@ export function usePanelEditorQuerySync({
// the V5 envelope level. Anchoring to `savedQueries` (not the builder-seed) keeps a
// handed-off / URL-restored edit reading as dirty; routing both sides through the same
// `fromPerses → toPerses` round-trip stops builder-added defaults (absent from an older
// stored query) reading an untouched panel as modified. New panel: fall back to seed.
// stored query) reading an untouched panel as modified. New panel: fall back to the
// frozen initial seed (see `initialSeedRef`).
const baselineEnvelopes = useMemo(
() =>
toQueryEnvelopes(
toPerses(
savedQueries ? fromPerses(savedQueries, panelType) : seedQuery,
savedQueries
? fromPerses(savedQueries, panelType)
: initialSeedRef.current,
panelType,
),
),
[savedQueries, seedQuery, panelType],
[savedQueries, panelType],
);
const isQueryDirty = useMemo(
() =>

View File

@@ -19,6 +19,7 @@ import {
SectionKind,
} from 'pages/DashboardPageV2/DashboardContainer/Panels/types/sections';
import { getBuilderQueries } from 'pages/DashboardPageV2/DashboardContainer/Panels/utils/getBuilderQueries';
import { useErrorModal } from 'providers/ErrorModalProvider';
import { getExecStats } from '../queryV5/v5ResponseData';
import { usePanelInteractions } from '../PanelsAndSectionsLayout/Panel/hooks/usePanelInteractions';
@@ -159,9 +160,10 @@ function PanelEditorContainer({
return section?.controls;
}, [panelDefinition]);
// New panels are savable once seeded with a query (List auto-seeds one). Read the
// seed `panel`, not the live `draft` — the staged-query sync commits the seed into
// the draft on open, which would falsely dirty an untouched query-less new panel.
// Unsaved-edits flag driving the discard confirmation on close (Save is always
// enabled). Read the seed `panel`, not the live `draft` — the staged-query sync
// commits the seed into the draft on open, which would falsely dirty an untouched
// query-less new panel.
const isDirty = useMemo(
() => isSpecDirty || isQueryDirty || (isNew && panel.spec.queries.length > 0),
[isSpecDirty, isQueryDirty, isNew, panel.spec.queries.length],
@@ -226,6 +228,7 @@ function PanelEditorContainer({
});
const setScrollTargetId = useScrollIntoViewStore((s) => s.setScrollTargetId);
const { showErrorModal } = useErrorModal();
const onSave = useCallback(async (): Promise<void> => {
if (!isEditable) {
@@ -236,12 +239,22 @@ function PanelEditorContainer({
const savedPanelId = await save(buildSaveSpec(draft.spec));
// Reveal the saved panel once the dashboard re-renders.
setScrollTargetId(savedPanelId);
toast.success('Panel saved');
toast.success('Panel saved', {
position: 'top-center',
});
onSaved();
} catch {
toast.error('Failed to save panel');
} catch (err) {
showErrorModal(err);
}
}, [isEditable, save, buildSaveSpec, draft.spec, setScrollTargetId, onSaved]);
}, [
isEditable,
save,
buildSaveSpec,
draft.spec,
setScrollTargetId,
onSaved,
showErrorModal,
]);
// Leaving an existing panel's editor (without saving) still returns to it, so
// the dashboard lands on that panel rather than scrolled to the top. A new,

View File

@@ -51,7 +51,7 @@ describe('panelStatusFromError', () => {
it('falls back to the error message when there is no structured body', () => {
expect(panelStatusFromError(new Error('boom'))).toStrictEqual({
code: 'unknown_error',
code: 'UPSTREAM_UNAVAILABLE',
message: 'boom',
docsUrl: undefined,
messages: [],

View File

@@ -12,13 +12,13 @@ import {
deleteDashboardV2,
invalidateListDashboardsForUserV2,
} from 'api/generated/services/dashboard';
import { useDeleteConfirm } from 'components/DeleteConfirmModal/useDeleteConfirm';
import { DashboardListEvents } from 'pages/DashboardsListPageV2/constants/events';
import { useAppContext } from 'providers/App/App';
import { useErrorModal } from 'providers/ErrorModalProvider';
import APIError from 'types/api/error';
import { USER_ROLES } from 'types/roles';
import { useDeleteConfirm } from '../DeleteConfirmModal/useDeleteConfirm';
import styles from './ActionsPopover.module.scss';
interface Props {

View File

@@ -6,11 +6,11 @@ import { Typography } from '@signozhq/ui/typography';
import { Bookmark, PenLine, Plus, Search, Trash2 } from '@signozhq/icons';
import cx from 'classnames';
import { useDeleteConfirm } from 'components/DeleteConfirmModal/useDeleteConfirm';
import { DashboardListEvents } from 'pages/DashboardsListPageV2/constants/events';
import type { SavedView } from '../../types';
import { type BuiltinView } from '../../utils/views';
import { useDeleteConfirm } from '../DeleteConfirmModal/useDeleteConfirm';
import ViewNamePopover from './ViewNamePopover';
import styles from './ViewsRail.module.scss';

View File

@@ -42,7 +42,12 @@ export function toAPIError(
try {
ErrorResponseHandlerForGeneratedAPIs(error);
} catch (apiError) {
if (apiError instanceof APIError) {
// UPSTREAM_UNAVAILABLE means the handler couldn't find a backend error code
// (non-envelope body); prefer the caller's context-specific defaultMessage.
if (
apiError instanceof APIError &&
apiError.getErrorCode() !== 'UPSTREAM_UNAVAILABLE'
) {
return apiError;
}
}

View File

@@ -100,9 +100,20 @@ func (q *chSQLQuery) renderVars(query string, vars map[string]qbtypes.VariableIt
return newQuery.String(), nil
}
// Statement renders the SQL without executing it, for the preview path.
func (q *chSQLQuery) Statement(_ context.Context) (*qbtypes.Statement, error) {
func (q *chSQLQuery) render(ctx context.Context) (string, error) {
rendered, err := q.renderVars(q.query.Query, q.vars, q.fromMS, q.toMS)
if err != nil {
return "", err
}
querybuilder.LogIfStatementIsNotValid(ctx, q.logger, rendered)
return rendered, nil
}
// Statement renders the SQL without executing it, for the preview path.
func (q *chSQLQuery) Statement(ctx context.Context) (*qbtypes.Statement, error) {
rendered, err := q.render(ctx)
if err != nil {
return nil, err
}
@@ -124,7 +135,7 @@ func (q *chSQLQuery) Execute(ctx context.Context) (*qbtypes.Result, error) {
elapsed += p.Elapsed
}))
query, err := q.renderVars(q.query.Query, q.vars, q.fromMS, q.toMS)
query, err := q.render(ctx)
if err != nil {
return nil, err
}
@@ -135,11 +146,11 @@ func (q *chSQLQuery) Execute(ctx context.Context) (*qbtypes.Result, error) {
}
defer rows.Close()
// TODO: map the errors from ClickHouse to our error types
payload, err := consume(rows, q.kind, nil, qbtypes.Step{}, q.query.Name)
if err != nil {
return nil, err
}
return &qbtypes.Result{
Type: q.kind,
Value: payload,

View File

@@ -11,6 +11,7 @@ import (
"github.com/SigNoz/signoz/pkg/errors"
"github.com/SigNoz/signoz/pkg/flagger"
"github.com/SigNoz/signoz/pkg/modules/thirdpartyapi"
"github.com/SigNoz/signoz/pkg/querybuilder"
"github.com/SigNoz/signoz/pkg/queryparser"
"log/slog"
@@ -1092,6 +1093,8 @@ func (aH *APIHandler) queryDashboardVarsV2(w http.ResponseWriter, r *http.Reques
return
}
querybuilder.LogIfStatementIsNotValid(r.Context(), aH.logger, query)
dashboardVars, err := aH.reader.QueryDashboardVars(r.Context(), query)
if err != nil {
RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil)

View File

@@ -0,0 +1,76 @@
package querybuilder
import (
"context"
"log/slog"
"strings"
chparser "github.com/AfterShip/clickhouse-sql-parser/parser"
"github.com/SigNoz/signoz/pkg/errors"
)
// internalDatabases hold server metadata, credentials and grants rather than telemetry.
var internalDatabases = map[string]struct{}{
"system": {},
"information_schema": {},
}
// The parser's grammar has gaps against SQL that ClickHouse itself accepts. See TestErrIfStatementIsNotValid_ShouldPassButFails.
func ErrIfStatementIsNotValid(query string) (err error) {
defer func() {
// The parser has a history of panicking on malformed input rather than returning an error.
if recovered := recover(); recovered != nil {
err = errors.NewInvalidInputf(errors.CodeInvalidInput, "invalid ClickHouse SQL (recovered): %v", recovered)
}
}()
stmts, parseErr := chparser.NewParser(query).ParseStmts()
if parseErr != nil {
// Wrapped rather than formatted in, so that callers can recover the parser's *ParseError and read the position off it.
return errors.WrapInvalidInputf(parseErr, errors.CodeInvalidInput, "invalid ClickHouse SQL: %s", parseErr.Error())
}
if len(stmts) != 1 {
return errors.NewInvalidInputf(errors.CodeInvalidInput, "ClickHouse SQL must contain exactly one statement, found %d statements", len(stmts))
}
selectQuery, ok := stmts[0].(*chparser.SelectQuery)
if !ok {
return errors.NewInvalidInputf(errors.CodeInvalidInput, "only SELECT statements are allowed in ClickHouse SQL queries")
}
visitor := &chparser.DefaultASTVisitor{Visit: func(node chparser.Expr) error {
switch expr := node.(type) {
case *chparser.TableFunctionExpr:
// Source table functions remain usable in ClickHouse read-only mode.
return errors.NewInvalidInputf(errors.CodeInvalidInput, "ClickHouse table functions are not allowed in SQL queries: %s", chparser.Format(expr.Name))
case *chparser.TableIdentifier:
// Reading these is unaffected by ClickHouse read-only mode.
if expr.Database == nil {
return nil
}
if _, ok := internalDatabases[strings.ToLower(expr.Database.Name)]; ok {
return errors.NewInvalidInputf(errors.CodeInvalidInput, "the ClickHouse %s database is not allowed in SQL queries", expr.Database.Name)
}
case *chparser.SettingExpr:
// A query-level setting takes precedence over the context setting.
if strings.EqualFold(expr.Name.Name, "readonly") {
return errors.NewInvalidInputf(errors.CodeInvalidInput, "the ClickHouse readonly setting cannot be overridden")
}
}
return nil
}}
return selectQuery.Accept(visitor)
}
// TODO(@therealpandey): remove this and move to ErrIfStatementIsNotValid.
func LogIfStatementIsNotValid(ctx context.Context, logger *slog.Logger, query string) {
if err := ErrIfStatementIsNotValid(query); err != nil {
logger.WarnContext(ctx, "clickhouse sql is not valid", errors.Attr(err), slog.String("query", query))
}
}

View File

@@ -0,0 +1,154 @@
package querybuilder
import (
"testing"
chparser "github.com/AfterShip/clickhouse-sql-parser/parser"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestErrIfStatementIsNotValid_Pass(t *testing.T) {
testCases := []struct {
name string
query string
}{
// Shapes a telemetry read is allowed to take.
{"Select", "SELECT region AS r, zone FROM metrics WHERE metric_name = 'cpu' GROUP BY region, zone"},
{"TrailingSemicolon", "SELECT count() FROM signoz_logs.distributed_logs_v2;"},
{"CommonTableExpression", "WITH t AS (SELECT fingerprint FROM signoz_metrics.time_series_v4) SELECT * FROM t"},
{"Join", "SELECT * FROM t1 LEFT JOIN t2 ON t1.a = t2.b"},
{"GlobalIn", "SELECT a FROM t WHERE a GLOBAL IN (SELECT b FROM t2)"},
{"Union", "SELECT * FROM t UNION ALL SELECT * FROM t2"},
{"Intersect", "SELECT * FROM t INTERSECT SELECT * FROM t2"},
{"WindowFunction", "SELECT sum(v) OVER (PARTITION BY a ORDER BY t) FROM t"},
{"UnrelatedSetting", "SELECT * FROM t SETTINGS max_threads = 4"},
{"TerminatedBlockComment", "SELECT /* keep me */ count() FROM t"},
{"BlockCommentMarkerInsideStringLiteral", "SELECT count() FROM t WHERE body = '/* not a comment'"},
// The parser used to loop forever on this; it now reads the comment to the end of
// the input, so this doubles as a canary for that regression.
{"TrailingUnterminatedBlockComment", "SELECT count() FROM t /* unterminated"},
// The rule keys on the database, not on the table name.
{"TableNamedSystemInTelemetryDatabase", "SELECT * FROM signoz_logs.system"},
{"SignedLiteralAfterClosingParen", "SELECT (toUnixTimestamp(now()) - 3600)*1000000000"},
// order by interval
{"OrderByInterval", "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS interval ORDER BY interval"},
{"OrderByIntervalAndDirection", "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS `interval` ORDER BY `interval` ASC"},
{"TrimFunction", "SELECT trimBoth('/api/endpoint/', '/');"},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
err := ErrIfStatementIsNotValid(testCase.query)
assert.NoError(t, err)
})
}
}
func TestErrIfStatementIsNotValid_Fail(t *testing.T) {
testCases := []struct {
name string
query string
}{
// Not a single statement, or not a statement at all.
{"Empty", ""},
{"UnterminatedBlockCommentOnly", "/* x"},
{"Unparseable", "SELECT FROM WHERE"},
{"MultipleStatements", "SELECT 1; DROP TABLE signoz_logs.logs_v2"},
// Parses, but is not a SELECT.
{"Drop", "DROP TABLE signoz_logs.logs_v2"},
{"Insert", "INSERT INTO signoz_logs.logs_v2 SELECT * FROM signoz_logs.logs_v2"},
{"AlterDelete", "ALTER TABLE signoz_logs.logs_v2 DELETE WHERE 1 = 1"},
{"CreateTable", "CREATE TABLE evil (a Int) ENGINE = Memory"},
{"Grant", "GRANT ALL ON *.* TO admin"},
{"Set", "SET readonly = 0"},
// These the parser rejects outright rather than classifying.
{"ShowGrants", "SHOW GRANTS"},
{"IntoOutfile", "SELECT * FROM t INTO OUTFILE '/tmp/x.csv'"},
// Table functions, which read through something other than a telemetry table.
{"UrlTableFunction", "SELECT * FROM url('http://attacker.example/x', CSV, 'a String')"},
{"FileTableFunction", "SELECT * FROM file('/etc/passwd', CSV, 'a String')"},
{"ExecutableTableFunction", "SELECT * FROM executable('script.sh', CSV, 'a String')"},
{"TableFunctionInJoin", "SELECT * FROM t1 JOIN url('http://x', CSV, 'a String') u ON 1 = 1"},
{"TableFunctionInCommonTableExpression", "WITH c AS (SELECT * FROM url('http://x', CSV, 'a String')) SELECT * FROM c"},
{"TableFunctionInWhereSubquery", "SELECT * FROM t WHERE a IN (SELECT * FROM file('/etc/passwd', CSV, 'a String'))"},
{"TableFunctionInUnion", "SELECT * FROM t UNION ALL SELECT * FROM url('http://x', CSV, 'a String')"},
// Internal databases, which hold grants and server metadata rather than telemetry.
{"SystemUsers", "SELECT * FROM system.users"},
{"SystemUppercase", "SELECT * FROM SYSTEM.USERS"},
{"SystemQuoted", "SELECT count() FROM `system`.`tables`"},
{"SystemInSubquery", "SELECT * FROM (SELECT name FROM system.parts)"},
{"SystemInJoin", "SELECT * FROM signoz_logs.distributed_logs_v2 AS l JOIN system.users AS u ON 1 = 1"},
{"SystemInIntersect", "SELECT * FROM t INTERSECT SELECT * FROM system.users"},
{"InformationSchema", "SELECT * FROM information_schema.tables"},
// A query-level setting takes precedence over the one the caller applies.
{"ReadonlySettingOverride", "SELECT * FROM t SETTINGS readonly = 0"},
{"ReadonlySettingOverrideAmongOthers", "SELECT * FROM t SETTINGS max_threads = 4, readonly = 0"},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
err := ErrIfStatementIsNotValid(testCase.query)
assert.Error(t, err)
})
}
}
// Queries the parser cannot read. ClickHouse runs all of them.
func TestErrIfStatementIsNotValid_ShouldPassButFails(t *testing.T) {
testCases := []struct {
name string
query string
// The construct the parser stops after, which is the one it cannot read.
expectedStopsAfter string
// The same construct written so the parser accepts it.
fix string
}{
{
name: "IntervalAliasInOrderBy",
query: "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS interval, resource_string_service$$name AS `service.name`, attributes_string['http.route'] AS `http.route`, quantile(0.95)(duration_nano) / 1000000000 AS value FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_string_service$$name = 'svc-a' AND resources_string['deployment.environment'] = 'dev' AND attributes_string['http.route'] = '/v1' AND http_method = 'POST' AND timestamp BETWEEN toDateTime(1784601720) AND toDateTime(1784602620) AND ts_bucket_start BETWEEN 1784601720 - 1800 AND 1784602620 GROUP BY `service.name`, `http.route`, interval ORDER BY interval ASC",
expectedStopsAfter: "ORDER BY interval ASC",
fix: "SELECT count() AS interval FROM t ORDER BY `interval` ASC",
},
{
name: "IntervalAliasInOrderByDesc",
query: "SELECT count() AS value, toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS interval, serviceName, resourceTagsMap['deployment.environment'] AS environment, exceptionStacktrace FROM signoz_traces.distributed_signoz_error_index_v2 WHERE exceptionType != 'OSError' AND resourceTagsMap['deployment.environment'] = 'staging' AND timestamp BETWEEN toDateTime(1785186300) AND toDateTime(1785186600) GROUP BY serviceName, interval, environment, exceptionStacktrace ORDER BY interval DESC",
expectedStopsAfter: "ORDER BY interval DESC",
fix: "SELECT count() AS interval FROM t ORDER BY `interval` DESC",
},
{
name: "StandardTrimSyntax",
query: "SELECT toStartOfInterval(fromUnixTimestamp64Nano(timestamp), INTERVAL 5 MINUTE) AS interval, resources_string['host.name'] as host_name, toFloat64(countIf( lower(trim(BOTH ' ' FROM replaceOne( JSONExtractString(body, 'Action'), 'health_status: ', '' ))) IN ('unhealthy','starting','failing') )) as value FROM signoz_logs.distributed_logs_v2 WHERE timestamp BETWEEN 1784602320000000000 AND 1784602620000000000 AND ts_bucket_start BETWEEN 1784602320 - 300 AND 1784602620 AND JSONExtractString(body, 'Type') = 'container' AND JSONExtractString(body, 'Actor', 'Attributes', 'name') IS NOT NULL AND resources_string['host.name'] IS NOT NULL AND resources_string['host.name'] = 'aihub-nightly' GROUP BY interval, host_name ORDER BY interval, host_name",
expectedStopsAfter: "trim(BOTH '",
fix: "SELECT trimBoth(replaceOne( JSONExtractString(body, 'Action'), 'health_status: ', '' ), ' ')",
},
{
name: "SignedLiteralAfterClosingParen",
query: "SELECT now() AS ts, toFloat64(count()) AS value FROM ( SELECT attributes_string['TableName'] AS T, attributes_string['MissingId'] AS M, max(fromUnixTimestamp64Nano(timestamp)) AS last_seen, dateDiff('minute', min(fromUnixTimestamp64Nano(timestamp)), max(fromUnixTimestamp64Nano(timestamp))) AS age_min FROM signoz_logs.distributed_logs_v2 WHERE body='missing_map_record' AND timestamp >= (toUnixTimestamp(now())-3600)*1000000000 GROUP BY T, M ) WHERE age_min >= 20 AND last_seen >= now() - toIntervalMinute(8)",
expectedStopsAfter: "(toUnixTimestamp(now())",
fix: "SELECT (toUnixTimestamp(now()) - 3600) * 1000000000",
},
{
name: "SignedLiteralAfterClosingParenMinimal",
query: "SELECT (1)-1",
expectedStopsAfter: "(1)",
fix: "SELECT (1) - 1",
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
err := ErrIfStatementIsNotValid(testCase.query)
var parseErr *chparser.ParseError
require.ErrorAs(t, err, &parseErr, "expected a parser failure rather than a rule violation")
// The parser reports the offset it stopped at, which sits just past the construct
// it choked on, so the text leading up to it is what needs looking at.
consumed := testCase.query[:parseErr.Pos]
assert.Equal(t, testCase.expectedStopsAfter, consumed[max(0, len(consumed)-len(testCase.expectedStopsAfter)):])
assert.NoError(t, ErrIfStatementIsNotValid(testCase.fix))
})
}
}

View File

@@ -1 +0,0 @@
package querybuilder