Compare commits

..

12 Commits

Author SHA1 Message Date
Piyush Singariya
5565906960 chore: separate frontend from backend changes 2026-05-13 18:14:52 +05:30
Piyush Singariya
923de53f92 Merge remote-tracking branch 'origin/main' into postprocess-json-logs 2026-05-13 18:07:50 +05:30
SagarRajput-7
d15065b808 feat(authz): enable multi role assignment for members page (#11269)
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
* feat(authz): enable multi role assignment for members page

* feat(authz): enable multi role assignemnt for users

---------

Co-authored-by: vikrantgupta25 <vikrant@signoz.io>
2026-05-13 11:16:59 +00:00
Nageshbansal
757c4e8ea9 chore: revert the v0.123.0 release (#11286) 2026-05-13 10:45:12 +00:00
Piyush Singariya
e93c857bdf fix: table view 2026-05-11 10:55:37 +05:30
Piyush Singariya
6a96bf489c Merge branch 'main' into postprocess-json-logs 2026-05-11 10:43:15 +05:30
Piyush Singariya
297ff0a1d6 Merge branch 'main' into postprocess-json-logs 2026-05-08 15:11:49 +05:30
Piyush Singariya
0ad2a49b5b Merge branch 'main' into postprocess-json-logs 2026-05-07 15:46:51 +05:30
Piyush Singariya
bcaccff2eb Merge branch 'main' into postprocess-json-logs 2026-05-05 17:50:54 +05:30
Piyush Singariya
71d27b7022 chore: update in e2e tests 2026-05-05 17:35:19 +05:30
Piyush Singariya
7ed9627ae5 fix: message postprocessing 2026-05-05 17:32:06 +05:30
Piyush Singariya
2a747df764 fix: backend changes for message key postprocessing 2026-05-05 16:56:32 +05:30
25 changed files with 263 additions and 317 deletions

View File

@@ -152,7 +152,6 @@ telemetrystore:
max_result_rows: 0
ignore_data_skipping_indices: ""
secondary_indices_enable_bulk_filtering: false
max_query_size: 350000
##################### Prometheus #####################
prometheus:

View File

@@ -190,7 +190,7 @@ services:
# - ../common/clickhouse/storage.xml:/etc/clickhouse-server/config.d/storage.xml
signoz:
!!merge <<: *db-depend
image: signoz/signoz:v0.123.0
image: signoz/signoz:v0.122.0
ports:
- "8080:8080" # signoz port
# - "6060:6060" # pprof port

View File

@@ -117,7 +117,7 @@ services:
# - ../common/clickhouse/storage.xml:/etc/clickhouse-server/config.d/storage.xml
signoz:
!!merge <<: *db-depend
image: signoz/signoz:v0.123.0
image: signoz/signoz:v0.122.0
ports:
- "8080:8080" # signoz port
volumes:

View File

@@ -181,7 +181,7 @@ services:
# - ../common/clickhouse/storage.xml:/etc/clickhouse-server/config.d/storage.xml
signoz:
!!merge <<: *db-depend
image: signoz/signoz:${VERSION:-v0.123.0}
image: signoz/signoz:${VERSION:-v0.122.0}
container_name: signoz
ports:
- "8080:8080" # signoz port

View File

@@ -109,7 +109,7 @@ services:
# - ../common/clickhouse/storage.xml:/etc/clickhouse-server/config.d/storage.xml
signoz:
!!merge <<: *db-depend
image: signoz/signoz:${VERSION:-v0.123.0}
image: signoz/signoz:${VERSION:-v0.122.0}
container_name: signoz
ports:
- "8080:8080" # signoz port

View File

@@ -103,7 +103,7 @@ function EditMemberDrawer({
const { user: currentUser } = useAppContext();
const [localDisplayName, setLocalDisplayName] = useState('');
const [localRole, setLocalRole] = useState('');
const [localRoles, setLocalRoles] = useState<string[]>([]);
const [isSaving, setIsSaving] = useState(false);
const [saveErrors, setSaveErrors] = useState<SaveError[]>([]);
const [showDeleteConfirm, setShowDeleteConfirm] = useState(false);
@@ -141,7 +141,7 @@ function EditMemberDrawer({
} = useRoles();
const {
fetchedRoleIds,
currentRoles: currentMemberRoles,
isLoading: isMemberRolesLoading,
applyDiff,
} = useMemberRoleManager(member?.id ?? '', open && !!member?.id);
@@ -188,16 +188,24 @@ function EditMemberDrawer({
if (!member?.id) {
roleSessionRef.current = null;
} else if (member.id !== roleSessionRef.current && !isMemberRolesLoading) {
setLocalRole(fetchedRoleIds[0] ?? '');
setLocalRoles(
currentMemberRoles.map((r) => r.id).filter(Boolean) as string[],
);
roleSessionRef.current = member.id;
}
}, [member?.id, fetchedRoleIds, isMemberRolesLoading]);
}, [member?.id, currentMemberRoles, isMemberRolesLoading]);
const isDirty =
member !== null &&
fetchedUser != null &&
(localDisplayName !== fetchedDisplayName ||
localRole !== (fetchedRoleIds[0] ?? ''));
JSON.stringify([...localRoles].sort()) !==
JSON.stringify(
currentMemberRoles
.map((r) => r.id)
.filter(Boolean)
.sort(),
));
const { mutateAsync: updateMyUser } = useUpdateMyUserV2();
const { mutateAsync: updateUser } = useUpdateUser();
@@ -272,7 +280,14 @@ function EditMemberDrawer({
setIsSaving(true);
try {
const nameChanged = localDisplayName !== fetchedDisplayName;
const rolesChanged = localRole !== (fetchedRoleIds[0] ?? '');
const rolesChanged =
JSON.stringify([...localRoles].sort()) !==
JSON.stringify(
currentMemberRoles
.map((r) => r.id)
.filter(Boolean)
.sort(),
);
const namePromise = nameChanged
? isSelf
@@ -286,7 +301,7 @@ function EditMemberDrawer({
const [nameResult, rolesResult] = await Promise.allSettled([
namePromise,
rolesChanged
? applyDiff([localRole].filter(Boolean), availableRoles)
? applyDiff([...localRoles], availableRoles)
: Promise.resolve([]),
]);
@@ -305,10 +320,7 @@ function EditMemberDrawer({
context: 'Roles update',
apiError: toSaveApiError(rolesResult.reason),
onRetry: async (): Promise<void> => {
const failures = await applyDiff(
[localRole].filter(Boolean),
availableRoles,
);
const failures = await applyDiff([...localRoles], availableRoles);
setSaveErrors((prev) => {
const rest = prev.filter((e) => e.context !== 'Roles update');
return [
@@ -353,9 +365,9 @@ function EditMemberDrawer({
isDirty,
isSelf,
localDisplayName,
localRole,
localRoles,
fetchedDisplayName,
fetchedRoleIds,
currentMemberRoles,
updateMyUser,
updateUser,
applyDiff,
@@ -503,10 +515,15 @@ function EditMemberDrawer({
>
<div className="edit-member-drawer__input-wrapper edit-member-drawer__input-wrapper--disabled">
<div className="edit-member-drawer__disabled-roles">
{localRole ? (
<Badge color="vanilla">
{availableRoles.find((r) => r.id === localRole)?.name ?? localRole}
</Badge>
{localRoles.length > 0 ? (
localRoles.map((roleId) => {
const role = availableRoles.find((r) => r.id === roleId);
return (
<Badge key={roleId} color="vanilla">
{role?.name ?? roleId}
</Badge>
);
})
) : (
<span className="edit-member-drawer__email-text"></span>
)}
@@ -517,14 +534,15 @@ function EditMemberDrawer({
) : (
<RolesSelect
id="member-role"
mode="multiple"
roles={availableRoles}
loading={rolesLoading}
isError={rolesError}
error={rolesErrorObj}
onRefetch={refetchRoles}
value={localRole}
onChange={(role): void => {
setLocalRole(role ?? '');
value={localRoles}
onChange={(roles): void => {
setLocalRoles(roles);
setSaveErrors((prev) =>
prev.filter(
(err) =>
@@ -532,8 +550,7 @@ function EditMemberDrawer({
),
);
}}
placeholder="Select role"
allowClear={false}
placeholder="Select roles"
/>
)}
</div>

View File

@@ -5,7 +5,9 @@ import {
useCreateResetPasswordToken,
useDeleteUser,
useGetResetPasswordToken,
useGetRolesByUserID,
useGetUser,
useRemoveUserRoleByUserIDAndRoleID,
useSetRoleByUserID,
useUpdateMyUserV2,
useUpdateUser,
@@ -23,11 +25,16 @@ import EditMemberDrawer, { EditMemberDrawerProps } from '../EditMemberDrawer';
jest.mock('api/generated/services/users', () => ({
useDeleteUser: jest.fn(),
useGetUser: jest.fn(),
useGetRolesByUserID: jest.fn(),
useRemoveUserRoleByUserIDAndRoleID: jest.fn(),
useUpdateUser: jest.fn(),
useUpdateMyUserV2: jest.fn(),
useSetRoleByUserID: jest.fn(),
useGetResetPasswordToken: jest.fn(),
useCreateResetPasswordToken: jest.fn(),
getGetRolesByUserIDQueryKey: ({ id }: { id: string }): string[] => [
`/api/v2/users/${id}/roles`,
],
}));
jest.mock('api/ErrorResponseHandlerForGeneratedAPIs', () => ({
@@ -98,6 +105,7 @@ jest.mock('react-use', () => ({
const ROLES_ENDPOINT = '*/api/v1/roles';
const mockDeleteMutate = jest.fn();
const mockRemoveMutateAsync = jest.fn();
const mockCreateTokenMutateAsync = jest.fn();
const showErrorModal = jest.fn();
@@ -186,6 +194,14 @@ describe('EditMemberDrawer', () => {
isLoading: false,
refetch: jest.fn(),
});
(useGetRolesByUserID as jest.Mock).mockReturnValue({
data: { data: [managedRoles[0]] },
isLoading: false,
});
(useRemoveUserRoleByUserIDAndRoleID as jest.Mock).mockReturnValue({
mutateAsync: mockRemoveMutateAsync.mockResolvedValue({}),
isLoading: false,
});
(useUpdateUser as jest.Mock).mockReturnValue({
mutateAsync: jest.fn().mockResolvedValue({}),
isLoading: false,
@@ -296,7 +312,7 @@ describe('EditMemberDrawer', () => {
expect(onClose).not.toHaveBeenCalled();
});
it('selecting a different role calls setRole with the new role name', async () => {
it('adding a new role calls setRole without removing existing ones', async () => {
const onComplete = jest.fn();
const user = userEvent.setup({ pointerEventsCheck: 0 });
const mockSet = jest.fn().mockResolvedValue({});
@@ -308,7 +324,7 @@ describe('EditMemberDrawer', () => {
renderDrawer({ onComplete });
// Open the roles dropdown and select signoz-editor
// signoz-admin is already selected; add signoz-editor on top
await user.click(screen.getByLabelText('Roles'));
await user.click(await screen.findByTitle('signoz-editor'));
@@ -321,34 +337,31 @@ describe('EditMemberDrawer', () => {
pathParams: { id: 'user-1' },
data: { name: 'signoz-editor' },
});
expect(mockRemoveMutateAsync).not.toHaveBeenCalled();
expect(onComplete).toHaveBeenCalled();
});
});
it('does not call removeRole when the role is changed', async () => {
it('deselecting a role calls removeRole with the role id', async () => {
const onComplete = jest.fn();
const user = userEvent.setup({ pointerEventsCheck: 0 });
const mockSet = jest.fn().mockResolvedValue({});
(useSetRoleByUserID as jest.Mock).mockReturnValue({
mutateAsync: mockSet,
isLoading: false,
});
renderDrawer({ onComplete });
// Switch from signoz-admin to signoz-viewer using single-select
await user.click(screen.getByLabelText('Roles'));
await user.click(await screen.findByTitle('signoz-viewer'));
// signoz-admin appears as a selected tag — click its remove button to deselect
const adminTag = await screen.findByTitle('signoz-admin');
const removeBtn = adminTag.querySelector(
'.ant-select-selection-item-remove',
) as Element;
await user.click(removeBtn);
const saveBtn = screen.getByRole('button', { name: /save member details/i });
await waitFor(() => expect(saveBtn).not.toBeDisabled());
await user.click(saveBtn);
await waitFor(() => {
expect(mockSet).toHaveBeenCalledWith({
pathParams: { id: 'user-1' },
data: { name: 'signoz-viewer' },
expect(mockRemoveMutateAsync).toHaveBeenCalledWith({
pathParams: { id: 'user-1', roleId: managedRoles[0].id },
});
expect(onComplete).toHaveBeenCalled();
});

View File

@@ -1,8 +1,19 @@
import { useCallback, useMemo } from 'react';
import { useQueryClient } from 'react-query';
import type { AuthtypesRoleDTO } from 'api/generated/services/sigNoz.schemas';
import { useGetUser, useSetRoleByUserID } from 'api/generated/services/users';
import {
getGetRolesByUserIDQueryKey,
useGetRolesByUserID,
useRemoveUserRoleByUserIDAndRoleID,
useSetRoleByUserID,
} from 'api/generated/services/users';
import { retryOn429 } from 'utils/errorUtils';
const enum PromiseStatus {
Fulfilled = 'fulfilled',
Rejected = 'rejected',
}
export interface MemberRoleUpdateFailure {
roleName: string;
error: unknown;
@@ -10,7 +21,7 @@ export interface MemberRoleUpdateFailure {
}
interface UseMemberRoleManagerResult {
fetchedRoleIds: string[];
currentRoles: AuthtypesRoleDTO[];
isLoading: boolean;
applyDiff: (
localRoleIds: string[],
@@ -22,66 +33,96 @@ export function useMemberRoleManager(
userId: string,
enabled: boolean,
): UseMemberRoleManagerResult {
const { data: fetchedUser, isLoading } = useGetUser(
const queryClient = useQueryClient();
const { data, isLoading } = useGetRolesByUserID(
{ id: userId },
{ query: { enabled: !!userId && enabled } },
);
const currentUserRoles = useMemo(
() => fetchedUser?.data?.userRoles ?? [],
[fetchedUser],
);
const fetchedRoleIds = useMemo(
() =>
currentUserRoles
.map((ur) => ur.role?.id ?? ur.roleId)
.filter((id): id is string => Boolean(id)),
[currentUserRoles],
const currentRoles = useMemo<AuthtypesRoleDTO[]>(
() => data?.data ?? [],
[data?.data],
);
const { mutateAsync: setRole } = useSetRoleByUserID({
mutation: { retry: retryOn429 },
});
const { mutateAsync: removeRole } = useRemoveUserRoleByUserIDAndRoleID({
mutation: { retry: retryOn429 },
});
const invalidateRoles = useCallback(
() =>
queryClient.invalidateQueries(getGetRolesByUserIDQueryKey({ id: userId })),
[userId, queryClient],
);
const applyDiff = useCallback(
async (
localRoleIds: string[],
availableRoles: AuthtypesRoleDTO[],
): Promise<MemberRoleUpdateFailure[]> => {
const currentRoleIdSet = new Set(fetchedRoleIds);
const desiredRoleIdSet = new Set(localRoleIds.filter(Boolean));
const toAdd = availableRoles.filter(
(r) => r.id && desiredRoleIdSet.has(r.id) && !currentRoleIdSet.has(r.id),
const currentRoleIds = new Set(
currentRoles.map((r) => r.id).filter(Boolean),
);
const desiredRoleIds = new Set(
localRoleIds.filter((id) => id != null && id !== ''),
);
/// TODO: re-enable deletes once BE for this is streamlined
const allOps = [
...toAdd.map((role) => ({
roleName: role.name ?? 'unknown',
const addedRoles = availableRoles.filter(
(r) => r.id && desiredRoleIds.has(r.id) && !currentRoleIds.has(r.id),
);
const removedRoles = currentRoles.filter(
(r) => r.id && !desiredRoleIds.has(r.id),
);
const allOperations = [
...addedRoles.map((role) => ({
role,
run: (): ReturnType<typeof setRole> =>
setRole({
pathParams: { id: userId },
data: { name: role.name ?? '' },
}),
})),
...removedRoles.map((role) => ({
role,
run: (): ReturnType<typeof removeRole> =>
removeRole({ pathParams: { id: userId, roleId: role.id ?? '' } }),
})),
];
const results = await Promise.allSettled(allOps.map((op) => op.run()));
const results = await Promise.allSettled(
allOperations.map((op) => op.run()),
);
const successCount = results.filter(
(r) => r.status === PromiseStatus.Fulfilled,
).length;
if (successCount > 0) {
await invalidateRoles();
}
const failures: MemberRoleUpdateFailure[] = [];
results.forEach((result, i) => {
if (result.status === 'rejected') {
const { roleName, run } = allOps[i];
failures.push({ roleName, error: result.reason, onRetry: run });
results.forEach((result, index) => {
if (result.status === PromiseStatus.Rejected) {
const { role, run } = allOperations[index];
failures.push({
roleName: role.name ?? 'unknown',
error: result.reason,
onRetry: async (): Promise<void> => {
await run();
await invalidateRoles();
},
});
}
});
return failures;
},
[userId, fetchedRoleIds, setRole],
[userId, currentRoles, setRole, removeRole, invalidateRoles],
);
return { fetchedRoleIds, isLoading, applyDiff };
return { currentRoles, isLoading, applyDiff };
}

2
go.mod
View File

@@ -11,6 +11,7 @@ require (
github.com/SigNoz/signoz-otel-collector v0.144.3
github.com/antlr4-go/antlr/v4 v4.13.1
github.com/antonmedv/expr v1.15.3
github.com/bytedance/sonic v1.14.1
github.com/cespare/xxhash/v2 v2.3.0
github.com/coreos/go-oidc/v3 v3.17.0
github.com/dgraph-io/ristretto/v2 v2.3.0
@@ -112,7 +113,6 @@ require (
github.com/aws/aws-sdk-go-v2/service/sts v1.41.6 // indirect
github.com/aws/smithy-go v1.24.2 // indirect
github.com/bytedance/gopkg v0.1.3 // indirect
github.com/bytedance/sonic v1.14.1 // indirect
github.com/bytedance/sonic/loader v0.3.0 // indirect
github.com/cloudwego/base64x v0.1.6 // indirect
github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 // indirect

View File

@@ -874,45 +874,22 @@ func (module *setter) AddUserRole(ctx context.Context, orgID, userID valuer.UUID
return errors.NewInvalidInputf(errors.CodeInvalidInput, "role name not found: %s", roleName)
}
// check if user already has this role
existingUserRoles, err := module.getter.GetRolesByUserID(ctx, existingUser.ID)
if err != nil {
return err
}
existingRoles := make([]string, len(existingUserRoles))
for idx, role := range existingUserRoles {
existingRoles[idx] = role.Role.Name
}
// grant via authz (idempotent)
if err := module.authz.ModifyGrant(
// grant via authz (additive, idempotent — OpenFGA uses OnDuplicate: "ignore")
if err := module.authz.Grant(
ctx,
orgID,
existingRoles,
[]string{roleName},
authtypes.MustNewSubject(coretypes.NewResourceUser(), existingUser.ID.StringValue(), existingUser.OrgID, nil),
); err != nil {
return err
}
// create user_role entry
// create user_role entry (swallow AlreadyExists for idempotency — DB has unique constraint on user_id+role_id)
userRoles := authtypes.NewUserRoles(userID, foundRoles)
err = module.store.RunInTx(ctx, func(ctx context.Context) error {
err = module.userRoleStore.DeleteUserRoles(ctx, existingUser.ID)
if err != nil {
if err := module.userRoleStore.CreateUserRoles(ctx, userRoles); err != nil {
if !errors.Ast(err, errors.TypeAlreadyExists) {
return err
}
err := module.userRoleStore.CreateUserRoles(ctx, userRoles)
if err != nil {
return err
}
return nil
})
if err != nil {
return err
}
return module.tokenizer.DeleteIdentity(ctx, userID)

View File

@@ -12,8 +12,10 @@ import (
"time"
"github.com/ClickHouse/clickhouse-go/v2/lib/driver"
"github.com/SigNoz/signoz/pkg/errors"
qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5"
"github.com/SigNoz/signoz/pkg/types/telemetrytypes"
"github.com/bytedance/sonic"
)
var (
@@ -22,6 +24,8 @@ var (
// written clickhouse query. The column alias indcate which value is
// to be considered as final result (or target).
legacyReservedColumnTargetAliases = []string{"__result", "__value", "result", "res", "value"}
CodeFailUnmarshalJSONColumn = errors.MustNewCode("fail_unmarshal_json_column")
)
// consume reads every row and shapes it into the payload expected for the
@@ -393,11 +397,16 @@ func readAsRaw(rows driver.Rows, queryName string) (*qbtypes.RawData, error) {
// de-reference the typed pointer to any
val := reflect.ValueOf(cellPtr).Elem().Interface()
// Post-process JSON columns: normalize into String value
// Post-process JSON columns: unmarshal bytes into map[string]any
if strings.HasPrefix(strings.ToUpper(colTypes[i].DatabaseTypeName()), "JSON") {
switch x := val.(type) {
case []byte:
val = string(x)
var m map[string]any
err := sonic.Unmarshal(x, &m)
if err != nil {
return nil, errors.WrapInternalf(err, CodeFailUnmarshalJSONColumn, "failed to unmarshal JSON column %s", name)
}
val = m
default:
// already a structured type (map[string]any, []any, etc.)
}

View File

@@ -12,9 +12,12 @@ import (
"github.com/SigNoz/govaluate"
"github.com/SigNoz/signoz/pkg/errors"
"github.com/SigNoz/signoz/pkg/flagger"
"github.com/SigNoz/signoz/pkg/types/featuretypes"
"github.com/SigNoz/signoz/pkg/querybuilder"
qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5"
"github.com/SigNoz/signoz/pkg/types/telemetrytypes"
"github.com/SigNoz/signoz/pkg/valuer"
)
// queryInfo holds common query properties.
@@ -50,7 +53,7 @@ func getQueryName(spec any) string {
return getqueryInfo(spec).Name
}
func (q *querier) postProcessResults(ctx context.Context, results map[string]any, req *qbtypes.QueryRangeRequest) (map[string]any, error) {
func (q *querier) postProcessResults(ctx context.Context, orgID valuer.UUID, results map[string]any, req *qbtypes.QueryRangeRequest) (map[string]any, error) {
// Convert results to typed format for processing
typedResults := make(map[string]*qbtypes.Result)
for name, result := range results {
@@ -69,6 +72,7 @@ func (q *querier) postProcessResults(ctx context.Context, results map[string]any
case qbtypes.QueryBuilderQuery[qbtypes.LogAggregation]:
if result, ok := typedResults[spec.Name]; ok {
result = postProcessBuilderQuery(q, result, spec, req)
result = q.postProcessLogBody(ctx, orgID, result, req)
typedResults[spec.Name] = result
}
case qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]:
@@ -1045,3 +1049,33 @@ func (q *querier) calculateFormulaStep(expression string, req *qbtypes.QueryRang
return result
}
// postProcessLogBody removes the "message" key from the body map when it is empty.
// Only runs for raw list queries with the use_json_body feature enabled.
func (q *querier) postProcessLogBody(ctx context.Context, orgID valuer.UUID, result *qbtypes.Result, req *qbtypes.QueryRangeRequest) *qbtypes.Result {
if req.RequestType != qbtypes.RequestTypeRaw {
return result
}
if !q.fl.BooleanOrEmpty(ctx, flagger.FeatureUseJSONBody, featuretypes.NewFlaggerEvaluationContext(orgID)) {
return result
}
rawData, ok := result.Value.(*qbtypes.RawData)
if !ok {
return result
}
for _, row := range rawData.Rows {
bodyMap, ok := row.Data["body"].(map[string]any)
if !ok {
continue
}
if msg, exists := bodyMap["message"]; exists {
switch v := msg.(type) {
case string:
if v == "" {
delete(bodyMap, "message")
}
}
}
}
return result
}

View File

@@ -16,6 +16,7 @@ import (
"github.com/SigNoz/signoz/pkg/errors"
"github.com/SigNoz/signoz/pkg/factory"
"github.com/SigNoz/signoz/pkg/flagger"
"github.com/SigNoz/signoz/pkg/prometheus"
"github.com/SigNoz/signoz/pkg/query-service/utils"
"github.com/SigNoz/signoz/pkg/querybuilder"
@@ -35,6 +36,7 @@ var (
type querier struct {
logger *slog.Logger
fl flagger.Flagger
telemetryStore telemetrystore.TelemetryStore
metadataStore telemetrytypes.MetadataStore
promEngine prometheus.Prometheus
@@ -62,10 +64,12 @@ func New(
meterStmtBuilder qbtypes.StatementBuilder[qbtypes.MetricAggregation],
traceOperatorStmtBuilder qbtypes.TraceOperatorStatementBuilder,
bucketCache BucketCache,
flagger flagger.Flagger,
) *querier {
querierSettings := factory.NewScopedProviderSettings(settings, "github.com/SigNoz/signoz/pkg/querier")
return &querier{
logger: querierSettings.Logger(),
fl: flagger,
telemetryStore: telemetryStore,
metadataStore: metadataStore,
promEngine: promEngine,
@@ -684,7 +688,7 @@ func (q *querier) run(
}
gomaps.Copy(results, preseededResults)
processedResults, err := q.postProcessResults(ctx, results, req)
processedResults, err := q.postProcessResults(ctx, orgID, results, req)
if err != nil {
return nil, err
}

View File

@@ -7,6 +7,7 @@ import (
cmock "github.com/srikanthccv/ClickHouse-go-mock"
"github.com/SigNoz/signoz/pkg/flagger/flaggertest"
"github.com/SigNoz/signoz/pkg/instrumentation/instrumentationtest"
"github.com/SigNoz/signoz/pkg/telemetrystore"
"github.com/SigNoz/signoz/pkg/telemetrystore/telemetrystoretest"
@@ -44,14 +45,15 @@ func TestQueryRange_MetricTypeMissing(t *testing.T) {
providerSettings,
nil, // telemetryStore
metadataStore,
nil, // prometheus
nil, // traceStmtBuilder
nil, // logStmtBuilder
nil, // auditStmtBuilder
nil, // metricStmtBuilder
nil, // meterStmtBuilder
nil, // traceOperatorStmtBuilder
nil, // bucketCache
nil, // prometheus
nil, // traceStmtBuilder
nil, // logStmtBuilder
nil, // auditStmtBuilder
nil, // metricStmtBuilder
nil, // meterStmtBuilder
nil, // traceOperatorStmtBuilder
nil, // bucketCache
flaggertest.New(t), // flagger
)
req := &qbtypes.QueryRangeRequest{
@@ -116,6 +118,7 @@ func TestQueryRange_MetricTypeFromStore(t *testing.T) {
nil, // meterStmtBuilder
nil, // traceOperatorStmtBuilder
nil, // bucketCache
flaggertest.New(t), // flagger
)
req := &qbtypes.QueryRangeRequest{

View File

@@ -186,5 +186,6 @@ func newProvider(
meterStmtBuilder,
traceOperatorStmtBuilder,
bucketCache,
flagger,
), nil
}

View File

@@ -53,6 +53,7 @@ func prepareQuerierForMetrics(t *testing.T, telemetryStore telemetrystore.Teleme
nil, // meterStmtBuilder
nil, // traceOperatorStmtBuilder
nil, // bucketCache
flagger,
), metadataStore
}
@@ -102,6 +103,7 @@ func prepareQuerierForLogs(t *testing.T, telemetryStore telemetrystore.Telemetry
nil, // meterStmtBuilder
nil, // traceOperatorStmtBuilder
nil, // bucketCache
fl,
)
}
@@ -146,5 +148,6 @@ func prepareQuerierForTraces(t *testing.T, telemetryStore telemetrystore.Telemet
nil, // meterStmtBuilder
nil, // traceOperatorStmtBuilder
nil, // bucketCache
fl,
)
}

View File

@@ -47,7 +47,6 @@ type QuerySettings struct {
MaxResultRows int `mapstructure:"max_result_rows"`
IgnoreDataSkippingIndices string `mapstructure:"ignore_data_skipping_indices"`
SecondaryIndicesEnableBulkFiltering bool `mapstructure:"secondary_indices_enable_bulk_filtering"`
MaxClickHouseQuerySize int `mapstructure:"max_query_size"`
}
func NewConfigFactory() factory.ConfigFactory {

View File

@@ -54,10 +54,6 @@ func (h *provider) BeforeQuery(ctx context.Context, _ *telemetrystore.QueryEvent
settings["ignore_data_skipping_indices"] = h.settings.IgnoreDataSkippingIndices
}
if h.settings.MaxClickHouseQuerySize != 0 {
settings["max_query_size"] = h.settings.MaxClickHouseQuerySize
}
if ctx.Value("clickhouse_max_threads") != nil {
if maxThreads, ok := ctx.Value("clickhouse_max_threads").(int); ok {
settings["max_threads"] = maxThreads

View File

@@ -1,7 +1,5 @@
package querybuildertypesv5
import "github.com/SigNoz/signoz/pkg/errors"
type ClickHouseQuery struct {
// name of the query
Name string `json:"name"`
@@ -17,21 +15,3 @@ type ClickHouseQuery struct {
func (q ClickHouseQuery) Copy() ClickHouseQuery {
return q
}
// Validate performs preliminary validation on ClickHouseQuery.
func (q ClickHouseQuery) Validate() error {
if q.Query == "" {
return errors.NewInvalidInputf(
errors.CodeInvalidInput,
"ClickHouse SQL query is required",
)
}
if len(q.Query) > MaxClickHouseQueryLength {
return errors.NewInvalidInputf(
errors.CodeInvalidInput,
"ClickHouse SQL query exceeds maximum allowed length of %d characters",
MaxClickHouseQueryLength,
)
}
return nil
}

View File

@@ -1,7 +1,5 @@
package querybuildertypesv5
import "github.com/SigNoz/signoz/pkg/errors"
type PromQuery struct {
// name of the query
Name string `json:"name"`
@@ -21,21 +19,3 @@ type PromQuery struct {
func (q PromQuery) Copy() PromQuery {
return q // shallow copy is sufficient
}
// Validate performs preliminary validation on PromQuery.
func (q PromQuery) Validate() error {
if q.Query == "" {
return errors.NewInvalidInputf(
errors.CodeInvalidInput,
"PromQL query is required",
)
}
if len(q.Query) > MaxPromQLQueryLength {
return errors.NewInvalidInputf(
errors.CodeInvalidInput,
"PromQL query exceeds maximum allowed length of %d characters",
MaxPromQLQueryLength,
)
}
return nil
}

View File

@@ -10,12 +10,6 @@ import (
"github.com/SigNoz/signoz/pkg/types/telemetrytypes"
)
const (
MaxFilterExpressionLength = 300_000
MaxClickHouseQueryLength = 300_000
MaxPromQLQueryLength = 300_000
)
// getQueryIdentifier returns a friendly identifier for a query based on its type and name/content.
func getQueryIdentifier(envelope QueryEnvelope, index int) string {
name := envelope.GetQueryName()
@@ -161,10 +155,6 @@ func (q *QueryBuilderQuery[T]) Validate(opts ...ValidationOption) error {
return err
}
if err := q.validateFilterExpression(); err != nil {
return err
}
return nil
}
@@ -375,20 +365,6 @@ func (q *QueryBuilderQuery[T]) validateFunctions() error {
return nil
}
func (q *QueryBuilderQuery[T]) validateFilterExpression() error {
if q.Filter == nil || q.Filter.Expression == "" {
return nil
}
if len(q.Filter.Expression) > MaxFilterExpressionLength {
return errors.NewInvalidInputf(
errors.CodeInvalidInput,
"filter expression exceeds maximum allowed length of %d characters",
MaxFilterExpressionLength,
)
}
return nil
}
func (q *QueryBuilderQuery[T]) validateSecondaryAggregations() error {
for i, secAgg := range q.SecondaryAggregations {
// Secondary aggregation expression can be empty - we allow it per requirements
@@ -667,7 +643,13 @@ func validateQueryEnvelope(envelope QueryEnvelope, opts ...ValidationOption) err
"invalid PromQL spec",
)
}
return spec.Validate()
if spec.Query == "" {
return errors.NewInvalidInputf(
errors.CodeInvalidInput,
"PromQL query is required",
)
}
return nil
case QueryTypeClickHouseSQL:
spec, ok := envelope.Spec.(ClickHouseQuery)
if !ok {
@@ -676,7 +658,13 @@ func validateQueryEnvelope(envelope QueryEnvelope, opts ...ValidationOption) err
"invalid ClickHouse SQL spec",
)
}
return spec.Validate()
if spec.Query == "" {
return errors.NewInvalidInputf(
errors.CodeInvalidInput,
"ClickHouse SQL query is required",
)
}
return nil
default:
return errors.NewInvalidInputf(
errors.CodeInvalidInput,

View File

@@ -78,7 +78,6 @@ def create_signoz(
"SIGNOZ_ALERTMANAGER_SIGNOZ_ROUTE_GROUP__WAIT": "1s",
"SIGNOZ_ALERTMANAGER_SIGNOZ_ROUTE_GROUP__INTERVAL": "5s",
"SIGNOZ_CLOUDINTEGRATION_AGENT_VERSION": "v0.0.8",
"SIGNOZ_TELEMETRYSTORE_CLICKHOUSE_SETTINGS_MAX__QUERY__SIZE": "350000",
}
| sqlstore.env
| clickhouse.env

View File

@@ -129,11 +129,11 @@ def test_get_user_roles(
assert "type" in role
def test_assign_role_replaces_previous(
def test_assign_role_is_additive(
signoz: types.SigNoz,
get_token: Callable[[str, str], str],
):
"""Verify POST /api/v2/users/{id}/roles replaces existing role."""
"""Verify POST /api/v2/users/{id}/roles ADDS a role alongside existing ones and is idempotent."""
admin_token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
response = requests.get(
signoz.self.host_configs["8080"].get("/api/v2/users/me"),
@@ -144,6 +144,8 @@ def test_assign_role_replaces_previous(
me = response.json()["data"]
user_id = me["id"]
# User currently has signoz-admin from test_change_role.
# Assign signoz-editor — should be additive, admin stays.
response = requests.post(
signoz.self.host_configs["8080"].get(f"/api/v2/users/{user_id}/roles"),
json={"name": "signoz-editor"},
@@ -160,8 +162,29 @@ def test_assign_role_replaces_previous(
assert response.status_code == HTTPStatus.OK
roles = response.json()["data"]
names = {r["name"] for r in roles}
assert len(names) == 2
assert "signoz-editor" in names
assert "signoz-admin" not in names
assert "signoz-admin" in names
# Idempotency: assigning the same role again succeeds without duplicates
response = requests.post(
signoz.self.host_configs["8080"].get(f"/api/v2/users/{user_id}/roles"),
json={"name": "signoz-editor"},
headers={"Authorization": f"Bearer {admin_token}"},
timeout=5,
)
assert response.status_code == HTTPStatus.OK
response = requests.get(
signoz.self.host_configs["8080"].get(f"/api/v2/users/{user_id}/roles"),
headers={"Authorization": f"Bearer {admin_token}"},
timeout=5,
)
assert response.status_code == HTTPStatus.OK
roles = response.json()["data"]
editor_count = sum(1 for r in roles if r["name"] == "signoz-editor")
assert editor_count == 1
assert len(roles) == 2
def test_get_users_by_role(
@@ -202,7 +225,7 @@ def test_remove_role(
signoz: types.SigNoz,
get_token: Callable[[str, str], str],
):
"""Verify DELETE /api/v2/users/{id}/roles/{roleId} removes the role."""
"""Verify DELETE /api/v2/users/{id}/roles/{roleId} removes only the specified role."""
admin_token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
response = requests.get(
signoz.self.host_configs["8080"].get("/api/v2/users/me"),
@@ -237,7 +260,10 @@ def test_remove_role(
)
assert response.status_code == HTTPStatus.OK
roles_after = response.json()["data"]
assert len(roles_after) == 0
names_after = {r["name"] for r in roles_after}
assert len(roles_after) == 1
assert "signoz-editor" not in names_after
assert "signoz-admin" in names_after
def test_user_with_roles_reflects_change(
@@ -262,7 +288,8 @@ def test_user_with_roles_reflects_change(
assert response.status_code == HTTPStatus.OK
data = response.json()["data"]
role_names = {ur["role"]["name"] for ur in data["userRoles"]}
assert len(role_names) == 0
assert len(role_names) == 1
assert "signoz-admin" in role_names
def test_admin_cannot_assign_role_to_self(

View File

@@ -1,124 +0,0 @@
"""
Integration tests for the v5 ClickHouse SQL query length cap and the
raised ClickHouse `max_query_size` setting (350000) configured in
pkg/telemetrystore/config.go.
"""
from collections.abc import Callable
from datetime import UTC, datetime, timedelta
from http import HTTPStatus
from fixtures import types
from fixtures.auth import USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD
from fixtures.querier import make_query_request
# Mirrors MaxClickHouseQueryLength in
# pkg/types/querybuildertypes/querybuildertypesv5/validation.go
MAX_CLICKHOUSE_QUERY_LENGTH = 300_000
# ClickHouse's built-in default `max_query_size`. SigNoz raises this to
# 350000 via telemetrystore settings.
CLICKHOUSE_BUILTIN_MAX_QUERY_SIZE = 262_144
def test_clickhouse_query_at_validation_cap_is_accepted(
signoz: types.SigNoz,
create_user_admin: None, # pylint: disable=unused-argument
get_token: Callable[[str, str], str],
) -> None:
token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
prefix = "SELECT 1 /* "
suffix = " */"
query = prefix + ("x" * (MAX_CLICKHOUSE_QUERY_LENGTH - len(prefix) - len(suffix))) + suffix
assert len(query) == MAX_CLICKHOUSE_QUERY_LENGTH
now = datetime.now(tz=UTC)
response = make_query_request(
signoz,
token,
start_ms=int((now - timedelta(minutes=5)).timestamp() * 1000),
end_ms=int(now.timestamp() * 1000),
queries=[
{
"type": "clickhouse_sql",
"spec": {"name": "A", "query": query, "disabled": False},
}
],
request_type="raw",
)
assert response.status_code == HTTPStatus.OK, response.text
def test_clickhouse_query_above_validation_cap_is_rejected(
signoz: types.SigNoz,
create_user_admin: None, # pylint: disable=unused-argument
get_token: Callable[[str, str], str],
) -> None:
token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
prefix = "SELECT 1 /* "
suffix = " */"
target_len = MAX_CLICKHOUSE_QUERY_LENGTH + 1
query = prefix + ("x" * (target_len - len(prefix) - len(suffix))) + suffix
assert len(query) > MAX_CLICKHOUSE_QUERY_LENGTH
now = datetime.now(tz=UTC)
response = make_query_request(
signoz,
token,
start_ms=int((now - timedelta(minutes=5)).timestamp() * 1000),
end_ms=int(now.timestamp() * 1000),
queries=[
{
"type": "clickhouse_sql",
"spec": {"name": "A", "query": query, "disabled": False},
}
],
request_type="raw",
)
assert response.status_code == HTTPStatus.BAD_REQUEST, response.text
assert "exceeds maximum allowed length" in response.text
def test_clickhouse_query_above_builtin_max_query_size_parses(
signoz: types.SigNoz,
create_user_admin: None, # pylint: disable=unused-argument
get_token: Callable[[str, str], str],
) -> None:
"""
Verifies that the raised `max_query_size` setting is applied on every
ClickHouse query: a payload above ClickHouse's built-in parser limit
(262144 bytes) but below the v5 validation cap (300000) must succeed
end-to-end. Without the setting override the ClickHouse parser would
reject this and the server would return 500.
"""
token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
target_len = CLICKHOUSE_BUILTIN_MAX_QUERY_SIZE + 10_000
prefix = "SELECT 1 /* "
suffix = " */"
query = prefix + ("x" * (target_len - len(prefix) - len(suffix))) + suffix
assert len(query) > CLICKHOUSE_BUILTIN_MAX_QUERY_SIZE
assert len(query) < MAX_CLICKHOUSE_QUERY_LENGTH
now = datetime.now(tz=UTC)
response = make_query_request(
signoz,
token,
start_ms=int((now - timedelta(minutes=5)).timestamp() * 1000),
end_ms=int(now.timestamp() * 1000),
queries=[
{
"type": "clickhouse_sql",
"spec": {"name": "A", "query": query, "disabled": False},
}
],
request_type="raw",
)
assert response.status_code == HTTPStatus.OK, response.text

View File

@@ -20,7 +20,7 @@ from fixtures.querier import (
def _get_bodies(response: requests.Response) -> list[dict[str, Any]]:
return [json.loads(row["data"]["body"]) for row in get_rows(response)]
return [row["data"]["body"] for row in get_rows(response)]
def _run_query_case(signoz: types.SigNoz, token: str, now: datetime, case: dict[str, Any]) -> None:
@@ -1183,7 +1183,7 @@ def test_message_searches(
token = get_token(email=USER_ADMIN_EMAIL, password=USER_ADMIN_PASSWORD)
def _body_messages(response: requests.Response) -> list[str]:
return [json.loads(row["data"]["body"]).get("message", "") for row in get_rows(response)]
return [row["data"]["body"].get("message", "") for row in get_rows(response)]
payment_messages = {
"Payment processed successfully",