From 86a12469a7ec6dcef004fce8512b587737c99e0b Mon Sep 17 00:00:00 2001 From: L2D2Grafana Date: Wed, 10 Dec 2025 11:54:01 -0800 Subject: [PATCH] Logs: abstract url migration and add unit tests --- public/app/features/explore/Logs/Logs.tsx | 122 +------ .../Logs/utils/columnMigration.test.ts | 344 ++++++++++++++++++ .../explore/Logs/utils/columnMigration.ts | 135 +++++++ 3 files changed, 498 insertions(+), 103 deletions(-) create mode 100644 public/app/features/explore/Logs/utils/columnMigration.test.ts create mode 100644 public/app/features/explore/Logs/utils/columnMigration.ts diff --git a/public/app/features/explore/Logs/Logs.tsx b/public/app/features/explore/Logs/Logs.tsx index bda62ec98d1..8a7c86edc97 100644 --- a/public/app/features/explore/Logs/Logs.tsx +++ b/public/app/features/explore/Logs/Logs.tsx @@ -51,7 +51,6 @@ import store from 'app/core/store'; import { createAndCopyShortLink, getLogsPermalinkRange } from 'app/core/utils/shortLinks'; import { ControlledLogRows } from 'app/features/logs/components/ControlledLogRows'; import { InfiniteScroll } from 'app/features/logs/components/InfiniteScroll'; -import { LOG_LINE_BODY_FIELD_NAME, TABLE_LINE_FIELD_NAME } from 'app/features/logs/components/LogDetailsBody'; import { LogRows } from 'app/features/logs/components/LogRows'; import { LogRowContextModal } from 'app/features/logs/components/log-context/LogRowContextModal'; import { LogLineContext } from 'app/features/logs/components/panel/LogLineContext'; @@ -84,6 +83,7 @@ import { LogsMetaRow } from './LogsMetaRow'; import LogsNavigation from './LogsNavigation'; import { LogsTableWrap, getLogsTableHeight } from './LogsTableWrap'; import { LogsVolumePanelList } from './LogsVolumePanelList'; +import { migrateLegacyColumns } from './utils/columnMigration'; import { SETTING_KEY_ROOT, SETTINGS_KEYS, visualisationTypeKey } from './utils/logs'; import { getExploreBaseUrl } from './utils/url'; @@ -352,117 +352,33 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { // Parse URL to check for legacy columns const urlParams = location.getSearchObject(); const [urlState] = parseURL(urlParams); - const urlPane = urlState.panes[exploreId]; + + // Find the pane - exploreId might not match the URL pane key directly + const urlPane = urlState.panes[exploreId] ?? Object.values(urlState.panes)[0]; if (!urlPane?.panelsState?.logs) { return; } - // Check for legacy columns in URL panelsState - const logsState = urlPane.panelsState.logs; - const hasColumns = 'columns' in logsState; - if (!hasColumns) { + // Get current displayedFields to use as defaults for merge + const currentDisplayedFields = displayedFields; + + // Use migration utility to parse and transform legacy columns + const mergedFields = migrateLegacyColumns(urlPane.panelsState.logs, currentDisplayedFields); + if (!mergedFields) { return; } - // Use Object.getOwnPropertyDescriptor to safely access the property - const columnsDescriptor = Object.getOwnPropertyDescriptor(logsState, 'columns'); - const columnsValue = columnsDescriptor?.value; - - // Type guard to validate columns format - let legacyColumns: Record | string[] | undefined = undefined; - if (Array.isArray(columnsValue)) { - legacyColumns = columnsValue; - } else if (typeof columnsValue === 'object' && columnsValue !== null && !Array.isArray(columnsValue)) { - const values = Object.values(columnsValue); - if (values.length > 0 && values.every((v) => typeof v === 'string')) { - // Create a new object to avoid type assertion - const record: Record = {}; - for (const [key, value] of Object.entries(columnsValue)) { - if (typeof value === 'string') { - record[key] = value; - } - } - legacyColumns = record; - } - } - - if (!legacyColumns) { - return; - } - - // Convert columns to array format - let columnsArray: string[] = []; - if (Array.isArray(legacyColumns)) { - columnsArray = legacyColumns; - } else if (typeof legacyColumns === 'object' && legacyColumns !== null) { - // Handle object format like {0: 'Time', 1: 'Line'} - columnsArray = Object.values(legacyColumns); - } - - // Map legacy field names to new field names - const mappedColumns = columnsArray.map((column) => { - // Map 'Line' to LOG_LINE_BODY_FIELD_NAME - if (column === TABLE_LINE_FIELD_NAME) { - return LOG_LINE_BODY_FIELD_NAME; - } - // Other fields like 'Time' stay the same - return column; - }); - - // Merge with defaultDisplayedFields: defaults first, then migrated columns (avoid duplicates) - const mergedFields: string[] = [...defaultDisplayedFields]; - mappedColumns.forEach((column) => { - if (!mergedFields.includes(column)) { - mergedFields.push(column); - } - }); - - // Update displayedFields in state - const state: ExploreItemState | undefined = getState().explore.panes[exploreId]; - if (state?.panelsState?.logs) { - // Create new logs state without columns property - // Explicitly exclude 'columns' by copying only the properties we want - const logsState = state.panelsState.logs; - const newLogsState: ExploreLogsPanelState = { - visualisationType: logsState.visualisationType, + // Update displayedFields in Redux state - URL sync will handle URL update + dispatch( + changePanelState(exploreId, 'logs', { + ...panelState?.logs, + columns: undefined, // Remove columns from URL displayedFields: mergedFields, - ...(logsState.labelFieldName && { labelFieldName: logsState.labelFieldName }), - ...(logsState.refId && { refId: logsState.refId }), - ...(logsState.id && { id: logsState.id }), - }; - - dispatch(changePanelState(exploreId, 'logs', newLogsState)); - - // Remove columns from URL by re-serializing all panes - // Use the updated state we just created (without columns) instead of reading from Redux - const allPanes = getState().explore.panes; - const panesObj = Object.entries(allPanes).reduce((acc, [id, paneState]) => { - if (!paneState) { - return acc; - } - // For the current exploreId, use the updated state without columns - let stateToSerialize = paneState; - if (id === exploreId) { - // Create updated pane state with the new logs state (without columns) - stateToSerialize = { - ...paneState, - panelsState: { - ...paneState.panelsState, - logs: newLogsState, - }, - }; - } - return { - ...acc, - [id]: getUrlStateFromPaneState(stateToSerialize), - }; - }, {}); - - // Update URL - columns will be excluded since it's not in the type definition - location.partial({ panes: JSON.stringify(panesObj) }, false); - } - }, [exploreId, displayedFields, defaultDisplayedFields, dispatch, location]); + }) + ); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // Run only on mount // actions const onLogRowHover = useCallback( diff --git a/public/app/features/explore/Logs/utils/columnMigration.test.ts b/public/app/features/explore/Logs/utils/columnMigration.test.ts new file mode 100644 index 00000000000..e65d11a817d --- /dev/null +++ b/public/app/features/explore/Logs/utils/columnMigration.test.ts @@ -0,0 +1,344 @@ +import { LOG_LINE_BODY_FIELD_NAME, TABLE_LINE_FIELD_NAME } from 'app/features/logs/components/LogDetailsBody'; + +import { + parseLegacyColumns, + mapLegacyFieldNames, + mergeWithDefaults, + hasLegacyColumns, + extractColumnsValue, + migrateLegacyColumns, +} from './columnMigration'; + +describe('columnMigration', () => { + describe('parseLegacyColumns', () => { + it('should return null for null input', () => { + expect(parseLegacyColumns(null)).toBeNull(); + }); + + it('should return null for undefined input', () => { + expect(parseLegacyColumns(undefined)).toBeNull(); + }); + + it('should return null for empty array', () => { + expect(parseLegacyColumns([])).toBeNull(); + }); + + it('should return null for empty object', () => { + expect(parseLegacyColumns({})).toBeNull(); + }); + + it('should parse array format correctly', () => { + const input = ['Time', 'Line', 'level']; + expect(parseLegacyColumns(input)).toEqual(['Time', 'Line', 'level']); + }); + + it('should parse object format correctly', () => { + const input = { 0: 'Time', 1: 'Line', 2: 'level' }; + expect(parseLegacyColumns(input)).toEqual(['Time', 'Line', 'level']); + }); + + it('should return null for array with non-string elements', () => { + const input = ['Time', 123, 'level']; + expect(parseLegacyColumns(input)).toBeNull(); + }); + + it('should return null for object with non-string values', () => { + const input = { 0: 'Time', 1: 123, 2: 'level' }; + expect(parseLegacyColumns(input)).toBeNull(); + }); + + it('should return null for primitive types', () => { + expect(parseLegacyColumns('string')).toBeNull(); + expect(parseLegacyColumns(123)).toBeNull(); + expect(parseLegacyColumns(true)).toBeNull(); + }); + + it('should handle single element array', () => { + expect(parseLegacyColumns(['Time'])).toEqual(['Time']); + }); + + it('should handle single property object', () => { + expect(parseLegacyColumns({ 0: 'Time' })).toEqual(['Time']); + }); + + it('should parse real URL format with string numeric keys', () => { + // Real format from URL: columns%22:%7B%220%22:%22cluster%22,%221%22:%22Line%22,%222%22:%22Time%22%7D + // Decoded: {"0":"cluster","1":"Line","2":"Time"} + const input = { '0': 'cluster', '1': 'Line', '2': 'Time' }; + expect(parseLegacyColumns(input)).toEqual(['cluster', 'Line', 'Time']); + }); + }); + + describe('mapLegacyFieldNames', () => { + it('should map Line to LOG_LINE_BODY_FIELD_NAME', () => { + const input = [TABLE_LINE_FIELD_NAME]; + expect(mapLegacyFieldNames(input)).toEqual([LOG_LINE_BODY_FIELD_NAME]); + }); + + it('should preserve other field names', () => { + const input = ['Time', 'level', 'host']; + expect(mapLegacyFieldNames(input)).toEqual(['Time', 'level', 'host']); + }); + + it('should map Line while preserving other fields', () => { + const input = ['Time', TABLE_LINE_FIELD_NAME, 'level']; + expect(mapLegacyFieldNames(input)).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME, 'level']); + }); + + it('should handle empty array', () => { + expect(mapLegacyFieldNames([])).toEqual([]); + }); + + it('should handle multiple Line fields', () => { + const input = [TABLE_LINE_FIELD_NAME, TABLE_LINE_FIELD_NAME]; + expect(mapLegacyFieldNames(input)).toEqual([LOG_LINE_BODY_FIELD_NAME, LOG_LINE_BODY_FIELD_NAME]); + }); + }); + + describe('mergeWithDefaults', () => { + it('should return defaults when migrated columns is empty', () => { + const defaults = ['Time', 'body']; + expect(mergeWithDefaults([], defaults)).toEqual(['Time', 'body']); + }); + + it('should return migrated columns when defaults is empty', () => { + const migrated = ['Time', 'level']; + expect(mergeWithDefaults(migrated, [])).toEqual(['Time', 'level']); + }); + + it('should place defaults first', () => { + const migrated = ['level', 'host']; + const defaults = ['Time', 'body']; + const result = mergeWithDefaults(migrated, defaults); + expect(result).toEqual(['Time', 'body', 'level', 'host']); + }); + + it('should not duplicate fields', () => { + const migrated = ['Time', 'level']; + const defaults = ['Time', 'body']; + const result = mergeWithDefaults(migrated, defaults); + expect(result).toEqual(['Time', 'body', 'level']); + }); + + it('should handle all duplicates', () => { + const migrated = ['Time', 'body']; + const defaults = ['Time', 'body']; + const result = mergeWithDefaults(migrated, defaults); + expect(result).toEqual(['Time', 'body']); + }); + + it('should preserve order of defaults', () => { + const migrated = ['host']; + const defaults = ['body', 'Time', 'level']; + const result = mergeWithDefaults(migrated, defaults); + expect(result[0]).toBe('body'); + expect(result[1]).toBe('Time'); + expect(result[2]).toBe('level'); + expect(result[3]).toBe('host'); + }); + }); + + describe('hasLegacyColumns', () => { + it('should return false for null', () => { + expect(hasLegacyColumns(null)).toBe(false); + }); + + it('should return false for undefined', () => { + expect(hasLegacyColumns(undefined)).toBe(false); + }); + + it('should return false for non-object', () => { + expect(hasLegacyColumns('string')).toBe(false); + expect(hasLegacyColumns(123)).toBe(false); + }); + + it('should return false for object without columns property', () => { + expect(hasLegacyColumns({ displayedFields: ['Time'] })).toBe(false); + }); + + it('should return true for object with columns property', () => { + expect(hasLegacyColumns({ columns: ['Time', 'Line'] })).toBe(true); + }); + + it('should return true even if columns is null', () => { + expect(hasLegacyColumns({ columns: null })).toBe(true); + }); + + it('should return true even if columns is empty', () => { + expect(hasLegacyColumns({ columns: [] })).toBe(true); + }); + }); + + describe('extractColumnsValue', () => { + it('should extract columns array', () => { + const state = { columns: ['Time', 'Line'] }; + expect(extractColumnsValue(state)).toEqual(['Time', 'Line']); + }); + + it('should extract columns object', () => { + const state = { columns: { 0: 'Time', 1: 'Line' } }; + expect(extractColumnsValue(state)).toEqual({ 0: 'Time', 1: 'Line' }); + }); + + it('should return undefined when columns not present', () => { + const state = { displayedFields: ['Time'] }; + expect(extractColumnsValue(state)).toBeUndefined(); + }); + }); + + describe('migrateLegacyColumns', () => { + const defaultDisplayedFields = ['Time', LOG_LINE_BODY_FIELD_NAME]; + + it('should return null when logsState is null', () => { + expect(migrateLegacyColumns(null, defaultDisplayedFields)).toBeNull(); + }); + + it('should return null when logsState is undefined', () => { + expect(migrateLegacyColumns(undefined, defaultDisplayedFields)).toBeNull(); + }); + + it('should return null when no columns property exists', () => { + const logsState = { displayedFields: ['Time'] }; + expect(migrateLegacyColumns(logsState, defaultDisplayedFields)).toBeNull(); + }); + + it('should return null when columns is empty array', () => { + const logsState = { columns: [] }; + expect(migrateLegacyColumns(logsState, defaultDisplayedFields)).toBeNull(); + }); + + it('should return null when columns is invalid', () => { + const logsState = { columns: 'invalid' }; + expect(migrateLegacyColumns(logsState, defaultDisplayedFields)).toBeNull(); + }); + + it('should migrate array format columns', () => { + const logsState = { columns: ['Time', TABLE_LINE_FIELD_NAME, 'level'] }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME, 'level']); + }); + + it('should migrate object format columns', () => { + const logsState = { columns: { 0: 'Time', 1: TABLE_LINE_FIELD_NAME, 2: 'level' } }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME, 'level']); + }); + + it('should merge with defaults and avoid duplicates', () => { + const logsState = { columns: ['level', 'host'] }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME, 'level', 'host']); + }); + + it('should handle columns that are already in defaults', () => { + const logsState = { columns: ['Time', 'level'] }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME, 'level']); + }); + + it('should map Line to body and merge correctly', () => { + const logsState = { columns: [TABLE_LINE_FIELD_NAME] }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + // Line maps to LOG_LINE_BODY_FIELD_NAME which is already in defaults + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME]); + }); + + it('should handle empty defaults', () => { + const logsState = { columns: ['Time', TABLE_LINE_FIELD_NAME] }; + const result = migrateLegacyColumns(logsState, []); + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME]); + }); + + it('should preserve other logsState properties conceptually', () => { + // This test verifies the function only looks at columns + const logsState = { + columns: ['level'], + visualisationType: 'table', + displayedFields: ['existing'], + }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME, 'level']); + }); + + it('should handle real URL format with full logsState structure', () => { + // Real format from URL: columns%22:%7B%220%22:%22cluster%22,%221%22:%22Line%22,%222%22:%22Time%22%7D + // Full structure: {"columns":{"0":"cluster","1":"Line","2":"Time"},"visualisationType":"table","labelFieldName":"labels","refId":"A"} + const logsState = { + columns: { '0': 'cluster', '1': 'Line', '2': 'Time' }, + visualisationType: 'table', + labelFieldName: 'labels', + refId: 'A', + }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + // cluster is new, Line maps to body (already in defaults), Time is already in defaults + expect(result).toEqual(['Time', LOG_LINE_BODY_FIELD_NAME, 'cluster']); + }); + + describe('real URL format from ops.grafana-ops.net', () => { + // URL: https://ops.grafana-ops.net/explore?schemaVersion=1&panes=%7B%2259n%22:... + // Decoded panelsState.logs: + // { + // "sortOrder": "Ascending", + // "columns": {"0": "cluster", "1": "Line", "2": "Time"}, + // "visualisationType": "table", + // "labelFieldName": "labels", + // "refId": "A" + // } + + it('should migrate columns from real Grafana Explore URL', () => { + const logsState = { + sortOrder: 'Ascending', + columns: { '0': 'cluster', '1': 'Line', '2': 'Time' }, + visualisationType: 'table', + labelFieldName: 'labels', + refId: 'A', + }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + expect(result).toContain('cluster'); + expect(result).toContain(LOG_LINE_BODY_FIELD_NAME); + expect(result).toContain('Time'); + }); + + it('should map Line to body field name', () => { + const logsState = { + sortOrder: 'Ascending', + columns: { '0': 'cluster', '1': 'Line', '2': 'Time' }, + visualisationType: 'table', + labelFieldName: 'labels', + refId: 'A', + }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + // Line should be mapped to LOG_LINE_BODY_FIELD_NAME, not remain as 'Line' + expect(result).not.toContain('Line'); + expect(result).toContain(LOG_LINE_BODY_FIELD_NAME); + }); + + it('should place defaults first, then additional columns', () => { + const logsState = { + sortOrder: 'Ascending', + columns: { '0': 'cluster', '1': 'Line', '2': 'Time' }, + visualisationType: 'table', + labelFieldName: 'labels', + refId: 'A', + }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + // Default fields (Time, body) should come first + expect(result![0]).toBe('Time'); + expect(result![1]).toBe(LOG_LINE_BODY_FIELD_NAME); + // Then additional fields from columns (cluster) + expect(result![2]).toBe('cluster'); + }); + + it('should not duplicate fields that exist in both defaults and columns', () => { + const logsState = { + columns: { '0': 'Time', '1': 'Line', '2': 'cluster' }, + visualisationType: 'table', + }; + const result = migrateLegacyColumns(logsState, defaultDisplayedFields); + // Should not have duplicate Time or body entries + expect(result!.filter((f) => f === 'Time').length).toBe(1); + expect(result!.filter((f) => f === LOG_LINE_BODY_FIELD_NAME).length).toBe(1); + }); + }); + }); +}); diff --git a/public/app/features/explore/Logs/utils/columnMigration.ts b/public/app/features/explore/Logs/utils/columnMigration.ts new file mode 100644 index 00000000000..a40a60687f5 --- /dev/null +++ b/public/app/features/explore/Logs/utils/columnMigration.ts @@ -0,0 +1,135 @@ +import { LOG_LINE_BODY_FIELD_NAME, TABLE_LINE_FIELD_NAME } from 'app/features/logs/components/LogDetailsBody'; + +/** + * Migration utility for converting legacy 'columns' URL parameter to 'displayedFields'. + */ + +/** + * Parses legacy columns value from URL. + * Handles both array format and object format (e.g., {0: 'Time', 1: 'Line'}). + * + * @param columnsValue - The raw columns value from URL state + * @returns Array of column names, or null if invalid/empty + */ +export function parseLegacyColumns(columnsValue: unknown): string[] | null { + if (columnsValue === null || columnsValue === undefined) { + return null; + } + + // Handle array format + if (Array.isArray(columnsValue)) { + if (columnsValue.length === 0) { + return null; + } + // Validate all elements are strings + if (columnsValue.every((v) => typeof v === 'string')) { + return columnsValue; + } + return null; + } + + // Handle object format (e.g., {0: 'Time', 1: 'Line'}) + if (typeof columnsValue === 'object') { + const values = Object.values(columnsValue); + if (values.length === 0) { + return null; + } + // Validate all values are strings and filter to string array + if (values.every((v): v is string => typeof v === 'string')) { + return values; + } + } + + return null; +} + +/** + * Maps legacy field names to their new equivalents. + * Currently maps 'Line' to the LOG_LINE_BODY_FIELD_NAME constant. + * + * @param columns - Array of column names + * @returns Array with mapped column names + */ +export function mapLegacyFieldNames(columns: string[]): string[] { + return columns.map((column) => { + // Map 'Line' to LOG_LINE_BODY_FIELD_NAME (typically 'body') + if (column === TABLE_LINE_FIELD_NAME) { + return LOG_LINE_BODY_FIELD_NAME; + } + return column; + }); +} + +/** + * Merges migrated columns with default displayed fields. + * Default fields come first, then migrated columns (avoiding duplicates). + * + * @param migratedColumns - Columns from the legacy format (already mapped) + * @param defaultFields - Default fields to display + * @returns Merged array with defaults first, no duplicates + */ +export function mergeWithDefaults(migratedColumns: string[], defaultFields: string[]): string[] { + const mergedFields = [...defaultFields]; + + migratedColumns.forEach((column) => { + if (!mergedFields.includes(column)) { + mergedFields.push(column); + } + }); + + return mergedFields; +} + +/** + * Checks if a logs state object contains legacy columns that need migration. + * Acts as a type guard to narrow the type to an object with columns property. + * + * @param logsState - The logs panel state from URL + * @returns True if legacy columns exist + */ +export function hasLegacyColumns(logsState: unknown): logsState is object & { columns: unknown } { + if (!logsState || typeof logsState !== 'object') { + return false; + } + return 'columns' in logsState; +} + +/** + * Extracts the columns value from logs state using safe property access. + * + * @param logsState - The logs panel state from URL + * @returns The columns value, or undefined if not present + */ +export function extractColumnsValue(logsState: object): unknown { + const descriptor = Object.getOwnPropertyDescriptor(logsState, 'columns'); + return descriptor?.value; +} + +/** + * Main migration function - orchestrates the full migration process. + * Returns the migrated and merged fields, or null if no migration is needed. + * + * @param logsState - The logs panel state from URL + * @param defaultDisplayedFields - Default fields to merge with + * @returns Merged displayed fields array, or null if no migration needed + */ +export function migrateLegacyColumns(logsState: unknown, defaultDisplayedFields: string[]): string[] | null { + // Check if migration is needed (type guard narrows logsState to object) + if (!hasLegacyColumns(logsState)) { + return null; + } + + // Extract and parse the columns value + const columnsValue = extractColumnsValue(logsState); + const parsedColumns = parseLegacyColumns(columnsValue); + + if (!parsedColumns) { + return null; + } + + // Map legacy field names to new names + const mappedColumns = mapLegacyFieldNames(parsedColumns); + + // Merge with defaults + return mergeWithDefaults(mappedColumns, defaultDisplayedFields); +}