diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/QuerySearch.tsx b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/QuerySearch.tsx index 599d976e989..776fe4de359 100644 --- a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/QuerySearch.tsx +++ b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/QuerySearch.tsx @@ -52,6 +52,8 @@ import type { SignalType } from 'types/api/v5/queryRange'; import { queryExamples, SUGGESTIONS_SECTION } from './constants'; import { combineInitialAndUserExpression, + dedupeOptionsByLabel, + getFieldContextPrefix, getRecentOptions, renderRecentDeleteButton, } from './utils'; @@ -222,6 +224,12 @@ function QuerySearch({ QueryKeyDataSuggestionsProps[] | null >(null); + // dedupe keySuggestions by label/name + const dedupedKeySuggestions = useMemo( + () => dedupeOptionsByLabel(keySuggestions || []), + [keySuggestions], + ); + const [showExamples] = useState(false); const [cursorPos, setCursorPos] = useState({ line: 0, ch: 0 }); @@ -246,9 +254,11 @@ function QuerySearch({ [key: string]: QueryKeyDataSuggestionsProps[]; }): any[] => Object.values(keys).flatMap((items: QueryKeyDataSuggestionsProps[]) => - items.map(({ name, fieldDataType }) => ({ + items.map(({ name, fieldDataType, fieldContext }) => ({ label: name, type: fieldDataType === 'string' ? 'keyword' : fieldDataType, + fieldContext, + fieldDataType, info: '', details: '', })), @@ -307,13 +317,17 @@ function QuerySearch({ if (response.data.data) { const { keys } = response.data.data; const options = generateOptions(keys); - // Use a Map to deduplicate by label and preserve order: new options take precedence + // Deduplicate by full variant identity (name + context + data type), NOT by + // label. deduping by label removes varient which is not expected. If we need + // to dedupe by label use dedupedKeySuggestions not dedupe the source itself + const variantId = (opt: QueryKeyDataSuggestionsProps): string => + `${opt.label}|${opt.fieldContext ?? ''}|${opt.fieldDataType ?? ''}`; const merged = new Map(); - options.forEach((opt) => merged.set(opt.label, opt)); + options.forEach((opt) => merged.set(variantId(opt), opt)); if (searchText && lastKeyRef.current !== searchText) { (keySuggestions || []).forEach((opt) => { - if (!merged.has(opt.label)) { - merged.set(opt.label, opt); + if (!merged.has(variantId(opt))) { + merged.set(variantId(opt), opt); } }); } @@ -919,8 +933,55 @@ function QuerySearch({ if (queryContext.isInKey) { const searchText = word?.text.toLowerCase().trim() ?? ''; + const fieldContextMatch = getFieldContextPrefix(searchText); + + if (fieldContextMatch) { + const { context: fieldContext, remainder } = fieldContextMatch; + + // Fetch the context's page when the prefix is typed exactly eg.("attribute.") + if (remainder === '' && lastFetchedKeyRef.current !== searchText) { + debouncedFetchKeySuggestions(searchText); + } + + //suggestions that actually do start with . + const nameMatches = (keySuggestions || []) + .filter((option) => option.label.toLowerCase().includes(searchText)) + .map((option) => ({ ...option, boost: 100 })); + + //suggestions which do not start with the prefix but qualifies for suggestion + const contextQualified = (keySuggestions || []) + .filter( + (option) => + option.fieldContext === fieldContext && + option.label.toLowerCase().includes(remainder), + ) + .map((option) => ({ + ...option, + label: `${fieldContext}.${option.label}`, + boost: 0, + })); + + const contextOptions = dedupeOptionsByLabel([ + ...nameMatches, + ...contextQualified, + ]); - options = (keySuggestions || []).filter((option) => + // If contextOptions is empty fetch again. + if ( + contextOptions.length === 0 && + lastFetchedKeyRef.current !== searchText + ) { + debouncedFetchKeySuggestions(searchText); + } + + return { + from: word?.from ?? 0, + to: word?.to ?? cursorPos.ch, + options: addSpaceToOptions(contextOptions), + }; + } + + options = dedupedKeySuggestions.filter((option) => option.label.toLowerCase().includes(searchText), ); @@ -969,9 +1030,26 @@ function QuerySearch({ // If we have a key context, add that info to the operator suggestions if (keyName) { - // Find the key details from suggestions - const keyDetails = (keySuggestions || []).find((k) => k.label === keyName); - const keyType = keyDetails?.type || ''; + const keyContextMatch = getFieldContextPrefix(keyName); + // key-suggestion can contain multiple variants of a single key + // In variants we capture ones that match the label to typed keyName exactly or, + // if it has a prefix fieldContext remove it and then match. + const variants = (keySuggestions || []).filter( + (k) => + k.label === keyName || + (keyContextMatch !== null && + k.fieldContext === keyContextMatch.context && + k.label === keyContextMatch.remainder), + ); + const variantTypes = new Set( + variants + .map((k) => + k.type === 'keyword' ? QUERY_BUILDER_KEY_TYPES.STRING : k.type, + ) + .filter(Boolean), + ); + //if there are multi-variant, show all suggestions else just the one + const keyType = variantTypes.size === 1 ? [...variantTypes][0] : ''; // Filter operators based on key type if (keyType) { @@ -1214,7 +1292,7 @@ function QuerySearch({ if (curChar === '(') { // In expression context, suggest keys, functions, or nested parentheses options = [ - ...(keySuggestions || []), + ...dedupedKeySuggestions, { label: '(', type: 'parenthesis', info: 'Open nested group' }, { label: 'NOT', type: 'operator', info: 'Negate expression' }, ...options.filter((opt) => opt.type === 'function'), diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/__tests__/utils.test.ts b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/__tests__/utils.test.ts new file mode 100644 index 00000000000..82b91fd47ba --- /dev/null +++ b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/__tests__/utils.test.ts @@ -0,0 +1,120 @@ +import { + combineInitialAndUserExpression, + dedupeOptionsByLabel, + getFieldContextPrefix, + getUserExpressionFromCombined, +} from '../utils'; + +describe('entityLogsExpression', () => { + describe('combineInitialAndUserExpression', () => { + it('returns user when initial is empty', () => { + expect(combineInitialAndUserExpression('', 'body contains error')).toBe( + 'body contains error', + ); + }); + + it('returns initial when user is empty', () => { + expect(combineInitialAndUserExpression('k8s.pod.name = "x"', '')).toBe( + 'k8s.pod.name = "x"', + ); + }); + + it('wraps user in parentheses with AND', () => { + expect( + combineInitialAndUserExpression('k8s.pod.name = "x"', 'body = "a"'), + ).toBe('k8s.pod.name = "x" AND (body = "a")'); + }); + }); + + describe('getUserExpressionFromCombined', () => { + it('returns empty when combined equals initial', () => { + expect( + getUserExpressionFromCombined('k8s.pod.name = "x"', 'k8s.pod.name = "x"'), + ).toBe(''); + }); + + it('extracts user from wrapped form', () => { + expect( + getUserExpressionFromCombined( + 'k8s.pod.name = "x"', + 'k8s.pod.name = "x" AND (body = "a")', + ), + ).toBe('body = "a"'); + }); + + it('extracts user from legacy AND without parens', () => { + expect( + getUserExpressionFromCombined( + 'k8s.pod.name = "x"', + 'k8s.pod.name = "x" AND body = "a"', + ), + ).toBe('body = "a"'); + }); + + it('returns full combined when initial is empty', () => { + expect(getUserExpressionFromCombined('', 'service.name = "a"')).toBe( + 'service.name = "a"', + ); + }); + }); +}); + +describe('getFieldContextPrefix', () => { + it('matches a complete context prefix with a remainder', () => { + expect(getFieldContextPrefix('attribute.status')).toStrictEqual({ + context: 'attribute', + remainder: 'status', + }); + }); + + it('matches a bare context prefix with empty remainder', () => { + expect(getFieldContextPrefix('resource.')).toStrictEqual({ + context: 'resource', + remainder: '', + }); + }); + + it('matches every backend field context', () => { + ['attribute', 'resource', 'span', 'body', 'log', 'metric'].forEach((ctx) => { + expect(getFieldContextPrefix(`${ctx}.x`)).toStrictEqual({ + context: ctx, + remainder: 'x', + }); + }); + }); + + it('does not match a partial context name', () => { + expect(getFieldContextPrefix('attr')).toBeNull(); + expect(getFieldContextPrefix('attribute')).toBeNull(); + }); + + it('does not match a non-context key with dots', () => { + expect(getFieldContextPrefix('status.code')).toBeNull(); + }); + + it('matches context case-insensitively but keeps remainder casing', () => { + expect(getFieldContextPrefix('Attribute.Status')).toStrictEqual({ + context: 'attribute', + remainder: 'Status', + }); + }); +}); + +describe('dedupeOptionsByLabel', () => { + it('keeps the first occurrence per label, preserving order', () => { + expect( + dedupeOptionsByLabel([ + { label: 'status.code', type: 'keyword' }, + { label: 'status.code', type: 'number' }, + { label: 'duration', type: 'number' }, + ]), + ).toStrictEqual([ + { label: 'status.code', type: 'keyword' }, + { label: 'duration', type: 'number' }, + ]); + }); + + it('returns an empty array for empty input', () => { + expect(dedupeOptionsByLabel([])).toStrictEqual([]); + }); +}); diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/__tests__/utils.ts b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/__tests__/utils.ts deleted file mode 100644 index 482194b5b65..00000000000 --- a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/__tests__/utils.ts +++ /dev/null @@ -1,58 +0,0 @@ -import { - combineInitialAndUserExpression, - getUserExpressionFromCombined, -} from '../utils'; - -describe('entityLogsExpression', () => { - describe('combineInitialAndUserExpression', () => { - it('returns user when initial is empty', () => { - expect(combineInitialAndUserExpression('', 'body contains error')).toBe( - 'body contains error', - ); - }); - - it('returns initial when user is empty', () => { - expect(combineInitialAndUserExpression('k8s.pod.name = "x"', '')).toBe( - 'k8s.pod.name = "x"', - ); - }); - - it('wraps user in parentheses with AND', () => { - expect( - combineInitialAndUserExpression('k8s.pod.name = "x"', 'body = "a"'), - ).toBe('k8s.pod.name = "x" AND (body = "a")'); - }); - }); - - describe('getUserExpressionFromCombined', () => { - it('returns empty when combined equals initial', () => { - expect( - getUserExpressionFromCombined('k8s.pod.name = "x"', 'k8s.pod.name = "x"'), - ).toBe(''); - }); - - it('extracts user from wrapped form', () => { - expect( - getUserExpressionFromCombined( - 'k8s.pod.name = "x"', - 'k8s.pod.name = "x" AND (body = "a")', - ), - ).toBe('body = "a"'); - }); - - it('extracts user from legacy AND without parens', () => { - expect( - getUserExpressionFromCombined( - 'k8s.pod.name = "x"', - 'k8s.pod.name = "x" AND body = "a"', - ), - ).toBe('body = "a"'); - }); - - it('returns full combined when initial is empty', () => { - expect(getUserExpressionFromCombined('', 'service.name = "a"')).toBe( - 'service.name = "a"', - ); - }); - }); -}); diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/constants.ts b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/constants.ts index 251ffdceea6..a07da15eb2d 100644 --- a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/constants.ts +++ b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/constants.ts @@ -1,6 +1,16 @@ export const RECENTS_SECTION = { name: 'Recent searches', rank: 1 } as const; export const SUGGESTIONS_SECTION = { name: 'Suggestions', rank: 2 } as const; +// TODO: move to using TelemetrytypesFieldContextDTO when we migrate getKeySuggestions API (Part of https://github.com/SigNoz/engineering-pod/issues/5289) +export const FIELD_CONTEXTS = [ + 'attribute', + 'resource', + 'span', + 'body', + 'log', + 'metric', +] as const; + // Custom CodeMirror Completion.type for recent-query entries. Used to discriminate // recents from regular autocomplete completions in renderers and event handlers. export const RECENT_COMPLETION_TYPE = 'recent'; diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/utils.ts b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/utils.ts index 8705fd3eb16..676e66e0e67 100644 --- a/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/utils.ts +++ b/frontend/src/components/QueryBuilderV2/QueryV2/QuerySearch/utils.ts @@ -9,11 +9,45 @@ import type { SignalType } from 'types/api/v5/queryRange'; import 'utils/timeUtils'; import { + FIELD_CONTEXTS, RECENT_COMPLETION_TYPE, RECENTS_DISPLAY_CAP, RECENTS_SECTION, } from './constants'; +export interface FieldContextPrefixMatch { + context: string; + remainder: string; +} + +// This function checks if the text(query key) starts with a fieldContext +// This util strictly checks that and returns context and remainder back. +// This helps differentiate if the typed key was prefixed with a context or +// was it an actual queryKey like (attribute.abc) +export function getFieldContextPrefix( + text: string, +): FieldContextPrefixMatch | null { + const lower = text.toLowerCase(); + const context = FIELD_CONTEXTS.find((ctx) => lower.startsWith(`${ctx}.`)); + return context ? { context, remainder: text.slice(context.length + 1) } : null; +} + +// Keeps the first occurrence per label, preserving order. Key suggestions hold +// one entry per (name, fieldContext, fieldDataType) variant; This means query builder +// could show multiple labels and this avoids that. +export function dedupeOptionsByLabel( + options: T[], +): T[] { + const seen = new Set(); + return options.filter((option) => { + if (seen.has(option.label)) { + return false; + } + seen.add(option.label); + return true; + }); +} + export function combineInitialAndUserExpression( initial: string, user: string, diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/QuerySearch.contextSuggestions.test.tsx b/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/QuerySearch.contextSuggestions.test.tsx new file mode 100644 index 00000000000..f2452cb2d0c --- /dev/null +++ b/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/QuerySearch.contextSuggestions.test.tsx @@ -0,0 +1,217 @@ +import { initialQueriesMap } from 'constants/queryBuilder'; +import { rest, server } from 'mocks-server/server'; +import { render, userEvent, waitFor } from 'tests/test-utils'; +import { DataSource } from 'types/common/queryBuilder'; + +import QuerySearch from '../QuerySearch/QuerySearch'; +import { mockCodeMirrorDomApis } from './codemirrorDomMocks'; + +const CM_EDITOR_SELECTOR = '.cm-editor .cm-content'; +const CM_OPTION_SELECTOR = '.cm-tooltip-autocomplete .cm-completionLabel'; + +beforeAll(() => { + mockCodeMirrorDomApis(); +}); + +jest.mock('hooks/useDarkMode', () => ({ + useIsDarkMode: (): boolean => false, +})); + +jest.mock('providers/Dashboard/store/useDashboardStore', () => ({ + useDashboardStore: (): { dashboardData: undefined } => ({ + dashboardData: undefined, + }), +})); + +jest.mock('hooks/queryBuilder/useQueryBuilder', () => { + const handleRunQuery = jest.fn(); + return { + __esModule: true, + useQueryBuilder: (): { handleRunQuery: () => void } => ({ handleRunQuery }), + handleRunQuery, + }; +}); + +// Keys fixture: status.code exists as 3 variants (attribute string/number + +// resource string); attribute.a.b.c is a key literally named with the prefix. +const KEYS_FIXTURE = { + 'status.code': [ + { + name: 'status.code', + fieldContext: 'attribute', + fieldDataType: 'string', + signal: 'logs', + }, + { + name: 'status.code', + fieldContext: 'attribute', + fieldDataType: 'number', + signal: 'logs', + }, + { + name: 'status.code', + fieldContext: 'resource', + fieldDataType: 'string', + signal: 'logs', + }, + ], + 'attribute.a.b.c': [ + { + name: 'attribute.a.b.c', + fieldContext: 'attribute', + fieldDataType: 'string', + signal: 'logs', + }, + ], + 'duration.nano': [ + { + name: 'duration.nano', + fieldContext: 'span', + fieldDataType: 'number', + signal: 'logs', + }, + ], +}; + +const fetchedSearchTexts: string[] = []; + +beforeEach(() => { + fetchedSearchTexts.length = 0; + server.use( + rest.get('http://localhost/api/v1/fields/keys', (req, res, ctx) => { + fetchedSearchTexts.push(req.url.searchParams.get('searchText') ?? ''); + return res( + ctx.status(200), + ctx.json({ + status: 'success', + data: { complete: true, keys: KEYS_FIXTURE }, + }), + ); + }), + rest.get('http://localhost/api/v1/fields/values', (_req, res, ctx) => + res( + ctx.status(200), + ctx.json({ + status: 'success', + data: { values: { stringValues: [], numberValues: [] } }, + }), + ), + ), + ); +}); + +async function renderAndType(text: string): Promise { + render( + void>} + queryData={initialQueriesMap.logs.builder.queryData[0]} + dataSource={DataSource.LOGS} + />, + ); + + await waitFor(() => { + expect(document.querySelector(CM_EDITOR_SELECTOR)).toBeInTheDocument(); + }); + + const editor = document.querySelector(CM_EDITOR_SELECTOR) as HTMLElement; + await userEvent.click(editor); + await userEvent.type(editor, text); + return editor; +} + +// Types a key, waits for its suggestion to render (proving the debounced key +// fetch resolved into state), then types a space to enter operator context. +async function typeKeyThenEnterOperatorContext( + key: string, + expectedKeyLabel: string, +): Promise { + const editor = await renderAndType(key); + + await waitFor(() => { + expect(visibleOptionLabels()).toContain(expectedKeyLabel); + }); + + // skipClick: a click would reset the caret to position 0 under the mocked + // DOM rects; we need the space appended at the end of the typed key. + await userEvent.type(editor, ' ', { skipClick: true }); + + await waitFor(() => { + expect(visibleOptionLabels()).toContain('='); + }); +} + +function visibleOptionLabels(): string[] { + return Array.from(document.querySelectorAll(CM_OPTION_SELECTOR)).map( + (el) => el.textContent ?? '', + ); +} + +describe('QuerySearch context-prefixed key suggestions', () => { + it('shows one deduped suggestion per key name in normal mode', async () => { + await renderAndType('statu'); + + await waitFor(() => { + expect(visibleOptionLabels()).toContain('status.code'); + }); + + const statusOptions = visibleOptionLabels().filter( + (label) => label === 'status.code', + ); + expect(statusOptions).toHaveLength(1); + }); + + it('shows context-scoped suggestions for a complete context prefix', async () => { + await renderAndType('attribute.'); + + await waitFor(() => { + expect(visibleOptionLabels()).toContain('attribute.status.code'); + }); + + const labels = visibleOptionLabels(); + // Literal key name match ranks first + expect(labels[0]).toBe('attribute.a.b.c'); + // Context-qualified form of the literal key is also present + expect(labels).toContain('attribute.attribute.a.b.c'); + // span-only key is not suggested under attribute. + expect(labels).not.toContain('attribute.duration.nano'); + }); + + it('fetches the context page when the prefix is typed exactly', async () => { + await renderAndType('attribute.'); + + await waitFor(() => { + expect(fetchedSearchTexts).toContain('attribute.'); + }); + }); +}); + +describe('QuerySearch operator suggestions by key type', () => { + it('shows all operators for a key with ambiguous types', async () => { + // status.code is string + number across variants → no type filtering + await typeKeyThenEnterOperatorContext('status.code', 'status.code'); + + const labels = visibleOptionLabels(); + expect(labels).toContain('>'); + expect(labels).toContain('LIKE'); + }); + + it('shows numeric operators for a single-type number key', async () => { + await typeKeyThenEnterOperatorContext('duration.nano', 'duration.nano'); + + const labels = visibleOptionLabels(); + expect(labels).toContain('>'); + expect(labels).not.toContain('LIKE'); + }); + + it('narrows operators when a context prefix disambiguates the type', async () => { + // status.code is ambiguous globally, but resource.status.code is string-only + await typeKeyThenEnterOperatorContext( + 'resource.status.code', + 'resource.status.code', + ); + + const labels = visibleOptionLabels(); + expect(labels).toContain('LIKE'); + expect(labels).not.toContain('>'); + }); +}); diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/QuerySearch.test.tsx b/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/QuerySearch.test.tsx index c08a7c969bb..be3e4f72ab9 100644 --- a/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/QuerySearch.test.tsx +++ b/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/QuerySearch.test.tsx @@ -8,78 +8,13 @@ import type { QueryKeyDataSuggestionsProps } from 'types/api/querySuggestions/ty import { DataSource } from 'types/common/queryBuilder'; import QuerySearch from '../QuerySearch/QuerySearch'; +import { mockCodeMirrorDomApis } from './codemirrorDomMocks'; const CM_EDITOR_SELECTOR = '.cm-editor .cm-content'; // Mock DOM APIs that CodeMirror needs beforeAll(() => { - // Mock getClientRects and getBoundingClientRect for Range objects - const mockRect: DOMRect = { - width: 100, - height: 20, - top: 0, - left: 0, - right: 100, - bottom: 20, - x: 0, - y: 0, - toJSON: (): DOMRect => mockRect, - } as DOMRect; - - // Create a minimal Range mock with only what CodeMirror actually uses - const createMockRange = (): Range => { - let startContainer: Node = document.createTextNode(''); - let endContainer: Node = document.createTextNode(''); - let startOffset = 0; - let endOffset = 0; - - const mockRange = { - // CodeMirror uses these for text measurement - getClientRects: (): DOMRectList => - ({ - length: 1, - item: (index: number): DOMRect | null => (index === 0 ? mockRect : null), - 0: mockRect, - *[Symbol.iterator](): Generator { - yield mockRect; - }, - }) as unknown as DOMRectList, - getBoundingClientRect: (): DOMRect => mockRect, - // CodeMirror calls these to set up text ranges - setStart: (node: Node, offset: number): void => { - startContainer = node; - startOffset = offset; - }, - setEnd: (node: Node, offset: number): void => { - endContainer = node; - endOffset = offset; - }, - // Minimal Range properties (TypeScript requires these) - get startContainer(): Node { - return startContainer; - }, - get endContainer(): Node { - return endContainer; - }, - get startOffset(): number { - return startOffset; - }, - get endOffset(): number { - return endOffset; - }, - get collapsed(): boolean { - return startContainer === endContainer && startOffset === endOffset; - }, - commonAncestorContainer: document.body, - }; - return mockRange as unknown as Range; - }; - - // Mock document.createRange to return a new Range instance each time - document.createRange = (): Range => createMockRange(); - - // Mock getBoundingClientRect for elements - Element.prototype.getBoundingClientRect = (): DOMRect => mockRect; + mockCodeMirrorDomApis(); }); jest.mock('hooks/useDarkMode', () => ({ diff --git a/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/codemirrorDomMocks.ts b/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/codemirrorDomMocks.ts new file mode 100644 index 00000000000..04595b69fae --- /dev/null +++ b/frontend/src/components/QueryBuilderV2/QueryV2/__tests__/codemirrorDomMocks.ts @@ -0,0 +1,71 @@ +// Mocks the DOM measurement APIs CodeMirror needs to render in jsdom +// (Range client rects + element bounding rects). Call from a beforeAll in +// specs that render the real CodeMirror editor. +export function mockCodeMirrorDomApis(): void { + const mockRect: DOMRect = { + width: 100, + height: 20, + top: 0, + left: 0, + right: 100, + bottom: 20, + x: 0, + y: 0, + toJSON: (): DOMRect => mockRect, + } as DOMRect; + + // Create a minimal Range mock with only what CodeMirror actually uses + const createMockRange = (): Range => { + let startContainer: Node = document.createTextNode(''); + let endContainer: Node = document.createTextNode(''); + let startOffset = 0; + let endOffset = 0; + + const mockRange = { + // CodeMirror uses these for text measurement + getClientRects: (): DOMRectList => + ({ + length: 1, + item: (index: number): DOMRect | null => (index === 0 ? mockRect : null), + 0: mockRect, + *[Symbol.iterator](): Generator { + yield mockRect; + }, + }) as unknown as DOMRectList, + getBoundingClientRect: (): DOMRect => mockRect, + // CodeMirror calls these to set up text ranges + setStart: (node: Node, offset: number): void => { + startContainer = node; + startOffset = offset; + }, + setEnd: (node: Node, offset: number): void => { + endContainer = node; + endOffset = offset; + }, + // Minimal Range properties (TypeScript requires these) + get startContainer(): Node { + return startContainer; + }, + get endContainer(): Node { + return endContainer; + }, + get startOffset(): number { + return startOffset; + }, + get endOffset(): number { + return endOffset; + }, + get collapsed(): boolean { + return startContainer === endContainer && startOffset === endOffset; + }, + commonAncestorContainer: document.body, + }; + return mockRange as unknown as Range; + }; + + // Mock document.createRange to return a new Range instance each time + document.createRange = (): Range => createMockRange(); + + // Mock getBoundingClientRect for elements + Element.prototype.getBoundingClientRect = (): DOMRect => mockRect; +} diff --git a/pkg/modules/cloudintegration/implcloudintegration/fs/definitions/gcp/memorystore_redis/assets/dashboards/overview.json b/pkg/modules/cloudintegration/implcloudintegration/fs/definitions/gcp/memorystore_redis/assets/dashboards/overview.json index 33e252c39cf..bd350ce7c7f 100644 --- a/pkg/modules/cloudintegration/implcloudintegration/fs/definitions/gcp/memorystore_redis/assets/dashboards/overview.json +++ b/pkg/modules/cloudintegration/implcloudintegration/fs/definitions/gcp/memorystore_redis/assets/dashboards/overview.json @@ -238,7 +238,7 @@ } }, "tags": [], - "title": "Signoz GCP Memorystory Redis overview", + "title": "Signoz GCP Memorystore Redis overview", "uploadedGrafana": false, "uuid": "019f85e9-3846-7dc9-9bce-b626b8a3eaff", "variables": { @@ -262,7 +262,7 @@ "type": "DYNAMIC" }, "dabcf41a-4121-4a03-8738-927576cc90bf": { - "allSelected": true, + "allSelected": false, "customValue": "", "defaultValue": "", "description": "GCP Project ID", @@ -390,7 +390,7 @@ "disabled": false, "legend": "", "name": "A", - "query": "rate({\"redis.googleapis.com/server/uptime\", project_id=\"$project_id\"}[5m])" + "query": "" } ], "queryType": "builder", diff --git a/pkg/modules/tag/impltag/module.go b/pkg/modules/tag/impltag/module.go index a90ad157ba3..5650b91fb53 100644 --- a/pkg/modules/tag/impltag/module.go +++ b/pkg/modules/tag/impltag/module.go @@ -45,17 +45,17 @@ func (m *module) createMany(ctx context.Context, orgID valuer.UUID, kind coretyp return []*tagtypes.Tag{}, nil } - toCreate, matched, err := m.resolve(ctx, orgID, kind, postable) + ordered, toCreate, err := m.resolve(ctx, orgID, kind, postable) if err != nil { return nil, err } - created, err := m.store.CreateOrGet(ctx, toCreate) - if err != nil { + // CreateOrGet fills new rows' IDs in place; ordered shares those pointers. + if _, err := m.store.CreateOrGet(ctx, toCreate); err != nil { return nil, err } - return append(matched, created...), nil + return ordered, nil } func (m *module) syncLinksForResource(ctx context.Context, orgID valuer.UUID, kind coretypes.Kind, resourceID valuer.UUID, tagIDs []valuer.UUID) error { diff --git a/pkg/modules/tag/impltag/resolve.go b/pkg/modules/tag/impltag/resolve.go index ec6d4ceba9c..12d4f5ed69d 100644 --- a/pkg/modules/tag/impltag/resolve.go +++ b/pkg/modules/tag/impltag/resolve.go @@ -13,10 +13,9 @@ import ( // the existing tags for an org. Lookup is case-insensitive on both key and // value (matching the storage uniqueness rule); when an existing row matches, // its display casing is reused. Inputs are deduped on (LOWER(key), LOWER(value)); -// the first input's casing wins on collisions. Returns: -// - toCreate: new Tag rows the caller should insert (with pre-generated IDs) -// - matched: existing rows the caller's input already pointed to. They -// already carry authoritative IDs from the store. +// the first input's casing wins on collisions. Returns the resolved tags in +// request order (deduped) plus the new subset to insert; the new tags share +// pointers with the ordered slice, so their IDs populate in place on insert. func (m *module) resolve(ctx context.Context, orgID valuer.UUID, kind coretypes.Kind, postable []tagtypes.PostableTag) ([]*tagtypes.Tag, []*tagtypes.Tag, error) { if len(postable) == 0 { return nil, nil, nil @@ -34,8 +33,8 @@ func (m *module) resolve(ctx context.Context, orgID valuer.UUID, kind coretypes. } seenInRequestAlready := make(map[string]struct{}, len(postable)) // postable can have the same tag multiple times + ordered := make([]*tagtypes.Tag, 0, len(postable)) toCreate := make([]*tagtypes.Tag, 0) - matched := make([]*tagtypes.Tag, 0) for _, p := range postable { key, value, err := tagtypes.ValidatePostableTag(p) @@ -49,11 +48,13 @@ func (m *module) resolve(ctx context.Context, orgID valuer.UUID, kind coretypes. seenInRequestAlready[lookup] = struct{}{} if existingTag, ok := lowercaseTagsMap[lookup]; ok { - matched = append(matched, existingTag) + ordered = append(ordered, existingTag) continue } - toCreate = append(toCreate, tagtypes.NewTag(orgID, kind, key, value)) + newTag := tagtypes.NewTag(orgID, kind, key, value) + ordered = append(ordered, newTag) + toCreate = append(toCreate, newTag) } - return toCreate, matched, nil + return ordered, toCreate, nil } diff --git a/pkg/modules/tag/impltag/resolve_test.go b/pkg/modules/tag/impltag/resolve_test.go index 0e4d1c16947..1752ee2df6b 100644 --- a/pkg/modules/tag/impltag/resolve_test.go +++ b/pkg/modules/tag/impltag/resolve_test.go @@ -20,14 +20,14 @@ func TestModule_Resolve(t *testing.T) { store := tagtypestest.NewStore() m := &module{store: store} - toCreate, matched, err := m.resolve(context.Background(), valuer.GenerateUUID(), testKind, nil) + ordered, toCreate, err := m.resolve(context.Background(), valuer.GenerateUUID(), testKind, nil) require.NoError(t, err) + assert.Empty(t, ordered) assert.Empty(t, toCreate) - assert.Empty(t, matched) assert.Zero(t, store.ListCallCount, "should not hit store when input is empty") }) - t.Run("creates missing pairs and reuses existing", func(t *testing.T) { + t.Run("creates missing pairs and reuses existing, in request order", func(t *testing.T) { orgID := valuer.GenerateUUID() dbTag := tagtypes.NewTag(orgID, testKind, "team", "Pulse") dbTag2 := tagtypes.NewTag(orgID, testKind, "Database", "redis") @@ -35,22 +35,28 @@ func TestModule_Resolve(t *testing.T) { store.Tags = []*tagtypes.Tag{dbTag, dbTag2} m := &module{store: store} - toCreate, matched, err := m.resolve(context.Background(), orgID, testKind, []tagtypes.PostableTag{ + ordered, toCreate, err := m.resolve(context.Background(), orgID, testKind, []tagtypes.PostableTag{ {Key: "team", Value: "events"}, // new {Key: "DATABASE", Value: "REDIS"}, // case-only conflict {Key: "Brand", Value: "New"}, // new }) require.NoError(t, err) + // ordered mirrors the request: new, existing (reused pointer), new. + require.Len(t, ordered, 3) + assert.Equal(t, "team", ordered[0].Key) + assert.Equal(t, "events", ordered[0].Value) + assert.Same(t, dbTag2, ordered[1], "case-only conflict reuses the existing pointer with its authoritative ID") + assert.Equal(t, "Brand", ordered[2].Key) + assert.Equal(t, "New", ordered[2].Value) + createdLowerKVs := []string{} for _, tg := range toCreate { createdLowerKVs = append(createdLowerKVs, strings.ToLower(tg.Key)+"\x00"+strings.ToLower(tg.Value)) } assert.ElementsMatch(t, []string{"team\x00events", "brand\x00new"}, createdLowerKVs, "only the two missing pairs should be returned for insertion") - - require.Len(t, matched, 1, "DATABASE:REDIS should hit the existing 'Database:redis' tag") - assert.Same(t, dbTag2, matched[0], "matched should return the existing pointer with its authoritative ID") + assert.Same(t, ordered[0], toCreate[0], "toCreate shares pointers with ordered so inserted IDs propagate") }) t.Run("dedupes inputs that map to the same lower(key)+lower(value)", func(t *testing.T) { @@ -58,14 +64,14 @@ func TestModule_Resolve(t *testing.T) { store := tagtypestest.NewStore() m := &module{store: store} - toCreate, matched, err := m.resolve(context.Background(), orgID, testKind, []tagtypes.PostableTag{ + ordered, toCreate, err := m.resolve(context.Background(), orgID, testKind, []tagtypes.PostableTag{ {Key: "Foo", Value: "Bar"}, {Key: "foo", Value: "bar"}, {Key: "FOO", Value: "BAR"}, }) require.NoError(t, err) - require.Empty(t, matched) + require.Len(t, ordered, 1, "duplicate inputs must collapse into a single tag") require.Len(t, toCreate, 1, "duplicate inputs must collapse into a single insert") assert.Equal(t, "Foo", toCreate[0].Key, "first input's casing wins") assert.Equal(t, "Bar", toCreate[0].Value, "first input's casing wins") @@ -78,15 +84,15 @@ func TestModule_Resolve(t *testing.T) { store.Tags = []*tagtypes.Tag{dbTag} m := &module{store: store} - toCreate, matched, err := m.resolve(context.Background(), orgID, testKind, []tagtypes.PostableTag{ + ordered, toCreate, err := m.resolve(context.Background(), orgID, testKind, []tagtypes.PostableTag{ {Key: "team", Value: "PULSE"}, }) require.NoError(t, err) assert.Empty(t, toCreate) - require.Len(t, matched, 1) - assert.Equal(t, "Team", matched[0].Key) - assert.Equal(t, "Pulse", matched[0].Value) + require.Len(t, ordered, 1) + assert.Equal(t, "Team", ordered[0].Key) + assert.Equal(t, "Pulse", ordered[0].Value) }) t.Run("propagates validation error from any input", func(t *testing.T) { diff --git a/pkg/modules/tag/impltag/store.go b/pkg/modules/tag/impltag/store.go index fa385527199..591f8b576a0 100644 --- a/pkg/modules/tag/impltag/store.go +++ b/pkg/modules/tag/impltag/store.go @@ -44,6 +44,7 @@ func (s *store) ListByResource(ctx context.Context, orgID valuer.UUID, kind core Where("tr.kind = ?", kind). Where("tr.resource_id = ?", resourceID). Where("tag.org_id = ?", orgID). + OrderExpr("tr.rank ASC"). Scan(ctx) if err != nil { return nil, err @@ -71,6 +72,7 @@ func (s *store) ListByResources(ctx context.Context, orgID valuer.UUID, kind cor Where("tr.kind = ?", kind). Where("tr.resource_id IN (?)", bun.In(resourceIDs)). Where("tag.org_id = ?", orgID). + OrderExpr("tr.rank ASC"). Scan(ctx) if err != nil { return nil, err @@ -112,11 +114,14 @@ func (s *store) CreateRelations(ctx context.Context, relations []*tagtypes.TagRe if len(relations) == 0 { return nil } + // On re-link (same tag, same resource) overwrite rank with the incoming + // position, so a reordered tag set persists its new order. _, err := s.sqlstore. BunDBCtx(ctx). NewInsert(). Model(&relations). - On("CONFLICT (kind, resource_id, tag_id) DO NOTHING"). + On("CONFLICT (kind, resource_id, tag_id) DO UPDATE"). + Set("rank = EXCLUDED.rank"). Exec(ctx) return err } diff --git a/pkg/signoz/provider.go b/pkg/signoz/provider.go index f8f0d2e6678..653acb6bc57 100644 --- a/pkg/signoz/provider.go +++ b/pkg/signoz/provider.go @@ -221,6 +221,7 @@ func NewSQLMigrationProviderFactories( sqlmigration.NewAddRoleTransactionGroupsFactory(sqlstore, sqlschema), sqlmigration.NewAddTagUniqueIndexFactory(sqlstore, sqlschema), sqlmigration.NewAddTelemetryTuplesFactory(sqlstore), + sqlmigration.NewAddTagRelationRankFactory(sqlstore, sqlschema), ) } diff --git a/pkg/sqlmigration/102_add_tag_relation_rank.go b/pkg/sqlmigration/102_add_tag_relation_rank.go new file mode 100644 index 00000000000..e1af9dbe39e --- /dev/null +++ b/pkg/sqlmigration/102_add_tag_relation_rank.go @@ -0,0 +1,72 @@ +package sqlmigration + +import ( + "context" + + "github.com/SigNoz/signoz/pkg/factory" + "github.com/SigNoz/signoz/pkg/sqlschema" + "github.com/SigNoz/signoz/pkg/sqlstore" + "github.com/uptrace/bun" + "github.com/uptrace/bun/migrate" +) + +type addTagRelationRank struct { + sqlstore sqlstore.SQLStore + sqlschema sqlschema.SQLSchema +} + +func NewAddTagRelationRankFactory(sqlstore sqlstore.SQLStore, sqlschema sqlschema.SQLSchema) factory.ProviderFactory[SQLMigration, Config] { + return factory.NewProviderFactory(factory.MustNewName("add_tag_relation_rank"), func(ctx context.Context, ps factory.ProviderSettings, c Config) (SQLMigration, error) { + return &addTagRelationRank{ + sqlstore: sqlstore, + sqlschema: sqlschema, + }, nil + }) +} + +func (migration *addTagRelationRank) Register(migrations *migrate.Migrations) error { + return migrations.Register(migration.Up, migration.Down) +} + +func (migration *addTagRelationRank) Up(ctx context.Context, db *bun.DB) error { + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return err + } + + defer func() { + _ = tx.Rollback() + }() + + table, uniqueConstraints, err := migration.sqlschema.GetTable(ctx, sqlschema.TableName("tag_relation")) + if err != nil { + return err + } + + rankColumn := &sqlschema.Column{ + Name: sqlschema.ColumnName("rank"), + DataType: sqlschema.DataTypeInteger, + Nullable: false, + } + sqls := migration.sqlschema.Operator().AddColumn(table, uniqueConstraints, rankColumn, 0) + + // AddColumn recreates the table for a NOT NULL column on SQLite, which drops + // standalone indices. GetTable only reports inline table constraints, so + // restore the standalone unique index from migration 082. + sqls = append(sqls, migration.sqlschema.Operator().CreateIndex(&sqlschema.UniqueIndex{ + TableName: sqlschema.TableName("tag_relation"), + ColumnNames: []sqlschema.ColumnName{"kind", "resource_id", "tag_id"}, + })...) + + for _, sql := range sqls { + if _, err := tx.ExecContext(ctx, string(sql)); err != nil { + return err + } + } + + return tx.Commit() +} + +func (migration *addTagRelationRank) Down(context.Context, *bun.DB) error { + return nil +} diff --git a/pkg/types/tagtypes/tag_relation.go b/pkg/types/tagtypes/tag_relation.go index b9939fefbef..1523612c6bb 100644 --- a/pkg/types/tagtypes/tag_relation.go +++ b/pkg/types/tagtypes/tag_relation.go @@ -16,23 +16,27 @@ type TagRelation struct { Kind coretypes.Kind `json:"kind" required:"true" bun:"kind,type:text,notnull"` ResourceID valuer.UUID `json:"resourceId" required:"true" bun:"resource_id,type:text,notnull"` TagID valuer.UUID `json:"tagId" required:"true" bun:"tag_id,type:text,notnull"` - CreatedAt time.Time `json:"createdAt" bun:"created_at,notnull"` + // Rank is the tag's position within its resource; reads order by it. + Rank int `json:"rank" bun:"rank,notnull"` + CreatedAt time.Time `json:"createdAt" bun:"created_at,notnull"` } -func NewTagRelation(kind coretypes.Kind, resourceID valuer.UUID, tagID valuer.UUID) *TagRelation { +func NewTagRelation(kind coretypes.Kind, resourceID valuer.UUID, tagID valuer.UUID, rank int) *TagRelation { return &TagRelation{ Identifiable: types.Identifiable{ID: valuer.GenerateUUID()}, Kind: kind, ResourceID: resourceID, TagID: tagID, + Rank: rank, CreatedAt: time.Now(), } } +// NewTagRelations ranks each tag by its position so reads preserve order. func NewTagRelations(kind coretypes.Kind, resourceID valuer.UUID, tagIDs []valuer.UUID) []*TagRelation { relations := make([]*TagRelation, 0, len(tagIDs)) - for _, tagID := range tagIDs { - relations = append(relations, NewTagRelation(kind, resourceID, tagID)) + for rank, tagID := range tagIDs { + relations = append(relations, NewTagRelation(kind, resourceID, tagID, rank)) } return relations } diff --git a/tests/integration/tests/dashboard/03_v2_dashboard.py b/tests/integration/tests/dashboard/03_v2_dashboard.py index e5c087aa8e5..218ee02e194 100644 --- a/tests/integration/tests/dashboard/03_v2_dashboard.py +++ b/tests/integration/tests/dashboard/03_v2_dashboard.py @@ -570,7 +570,11 @@ def test_dashboard_v2_lifecycle( # pylint: disable=too-many-locals,too-many-sta assert alpha["schemaVersion"] == "v6" assert alpha["source"] == "user" assert alpha["locked"] is False - assert {"key": "team", "value": "pulse"} in alpha["tags"] + # tags round-trip in the order sent — no reordering, no drift + assert alpha["tags"] == [ + {"key": "team", "value": "pulse"}, + {"key": "env", "value": "prod"}, + ] # ── stage 3: list everything ───────────────────────────────────────────── response = requests.get( @@ -590,6 +594,13 @@ def test_dashboard_v2_lifecycle( # pylint: disable=too-many-locals,too-many-sta "Epsilon Metrics", "Zeta Overview", } + # per-dashboard tags also round-trip in the order sent + delta = next(d for d in body["data"]["dashboards"] if d["spec"]["display"]["name"] == "Delta Storage") + assert delta["tags"] == [ + {"key": "team", "value": "storage"}, + {"key": "env", "value": "dev"}, + {"key": "tier", "value": "critical"}, + ] # top-level tags = org-wide distinct tag set, sorted case-insensitively # by (key, value). Asserting the exact list (not a set) locks in the sort. assert body["data"]["tags"] == [ @@ -1013,6 +1024,99 @@ def test_dashboard_v2_lifecycle( # pylint: disable=too-many-locals,too-many-sta assert response.json()["data"]["id"] == clone["id"] +def test_dashboard_v2_tag_order_round_trips( + signoz: SigNoz, + create_user_admin: Operation, # pylint: disable=unused-argument + get_token: Callable[[str, str], str], +): + """Tags must round-trip in the order the client sent them across every write + path — create, update, patch — so GitOps/Terraform state never drifts.""" + token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD) + + def tags_of(response: requests.Response) -> list[dict]: + return response.json()["data"]["tags"] + + # ── create with tags in a deliberate order; "env" repeats with two values ─ + created_order = [ + {"key": "team", "value": "pulse"}, + {"key": "env", "value": "prod"}, + {"key": "env", "value": "staging"}, + {"key": "tier", "value": "critical"}, + ] + response = requests.post( + signoz.self.host_configs["8080"].get(BASE_URL), + json={"schemaVersion": "v6", "name": "tag-order", "spec": {"display": {"name": "Tag Order"}}, "tags": created_order}, + headers={"Authorization": f"Bearer {token}"}, + timeout=5, + ) + assert response.status_code == HTTPStatus.CREATED, response.text + dashboard_id = response.json()["data"]["id"] + assert tags_of(response) == created_order + + # ── get echoes the same order ──────────────────────────────────────────── + response = requests.get( + signoz.self.host_configs["8080"].get(f"{BASE_URL}/{dashboard_id}"), + headers={"Authorization": f"Bearer {token}"}, + timeout=5, + ) + assert response.status_code == HTTPStatus.OK, response.text + assert tags_of(response) == created_order + + # ── update reordered; the two "env" values land at non-adjacent positions ─ + reordered = [ + {"key": "tier", "value": "critical"}, + {"key": "env", "value": "staging"}, + {"key": "team", "value": "pulse"}, + {"key": "env", "value": "prod"}, + ] + response = requests.put( + signoz.self.host_configs["8080"].get(f"{BASE_URL}/{dashboard_id}"), + json={"schemaVersion": "v6", "name": "tag-order", "spec": {"display": {"name": "Tag Order"}}, "tags": reordered}, + headers={"Authorization": f"Bearer {token}"}, + timeout=5, + ) + assert response.status_code == HTTPStatus.OK, response.text + assert tags_of(response) == reordered + + # ── patch appends a new tag (a third "env" value); it lands at the end ──── + response = requests.patch( + signoz.self.host_configs["8080"].get(f"{BASE_URL}/{dashboard_id}"), + json=[{"op": "add", "path": "/tags/-", "value": {"key": "env", "value": "dev"}}], + headers={"Authorization": f"Bearer {token}"}, + timeout=5, + ) + assert response.status_code == HTTPStatus.OK, response.text + assert tags_of(response) == reordered + [{"key": "env", "value": "dev"}] + + # ── update reorders everything and adds another new tag → all in the new order ─ + # "env" now carries three interleaved values + new_order = [ + {"key": "env", "value": "prod"}, + {"key": "team", "value": "pulse"}, + {"key": "env", "value": "dev"}, + {"key": "tier", "value": "critical"}, + {"key": "env", "value": "staging"}, + {"key": "team", "value": "storage"}, + ] + response = requests.put( + signoz.self.host_configs["8080"].get(f"{BASE_URL}/{dashboard_id}"), + json={"schemaVersion": "v6", "name": "tag-order", "spec": {"display": {"name": "Tag Order"}}, "tags": new_order}, + headers={"Authorization": f"Bearer {token}"}, + timeout=5, + ) + assert response.status_code == HTTPStatus.OK, response.text + assert tags_of(response) == new_order + + # ── and the final order persists on a fresh read ───────────────────────── + response = requests.get( + signoz.self.host_configs["8080"].get(f"{BASE_URL}/{dashboard_id}"), + headers={"Authorization": f"Bearer {token}"}, + timeout=5, + ) + assert response.status_code == HTTPStatus.OK, response.text + assert tags_of(response) == new_order + + def test_dashboard_v2_pin_limit( signoz: SigNoz, create_user_admin: Operation, # pylint: disable=unused-argument