Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion web/cypress/e2e/integration/logs-dev-page.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ describe('Logs Dev Page', () => {
);
});

cy.byTestID(TestIds.ExecuteQueryButton).click();
cy.byTestID(TestIds.ExecuteQueryButton).click({ force: true });

cy.byTestID(TestIds.LogsMetrics).should('exist');
cy.byTestID(TestIds.ToggleHistogramButton).should('be.disabled');
Expand Down
118 changes: 118 additions & 0 deletions web/cypress/e2e/integration/logs-sorting.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { TestIds } from '../../../src/test-ids';

const LOGS_PAGE_URL = '/monitoring/logs';
const QUERY_RANGE_STREAMS_URL_MATCH =
'/api/proxy/plugin/logging-view-plugin/backend/api/logs/v1/application/loki/api/v1/query_range?query=%7B*';
const QUERY_RANGE_MATRIX_URL_MATCH =
'/api/proxy/plugin/logging-view-plugin/backend/api/logs/v1/application/loki/api/v1/query_range?query=sum*';

const sameTimestampStreamsResponse = () => {
const msTimestamp = BigInt(Date.now());
const nanosTimestamp = msTimestamp * 1000000n;
const nanosString = String(nanosTimestamp);

return {
status: 'success',
data: {
resultType: 'streams',
result: [
{
stream: {
filename: '/var/log/out.log',
job: 'varlogs',
level: 'info',
observedTimestamp: String(nanosTimestamp + 200n),
},
values: [[nanosString, 'log-line-B']],
},
{
stream: {
filename: '/var/log/out.log',
job: 'varlogs',
level: 'info',
observedTimestamp: String(nanosTimestamp + 300n),
},
values: [[nanosString, 'log-line-C']],
},
{
stream: {
filename: '/var/log/out.log',
job: 'varlogs',
level: 'info',
observedTimestamp: String(nanosTimestamp + 100n),
},
values: [[nanosString, 'log-line-A']],
},
],
stats: {
summary: {
bytesProcessedPerSecond: 0,
linesProcessedPerSecond: 0,
totalBytesProcessed: 0,
totalLinesProcessed: 0,
execTime: 0,
queueTime: 0,
subqueries: 0,
},
querier: {
store: {
totalChunksRef: 0,
totalChunksDownloaded: 0,
chunksDownloadTime: 0,
chunk: {
headChunkBytes: 0,
headChunkLines: 0,
decompressedBytes: 0,
decompressedLines: 0,
compressedBytes: 0,
totalDuplicates: 0,
},
},
},
ingester: {
totalReached: 0,
totalChunksMatched: 0,
totalBatches: 0,
totalLinesSent: 0,
store: {
totalChunksRef: 0,
totalChunksDownloaded: 0,
chunksDownloadTime: 0,
chunk: {
headChunkBytes: 0,
headChunkLines: 0,
decompressedBytes: 0,
decompressedLines: 0,
compressedBytes: 0,
totalDuplicates: 0,
},
},
},
},
},
};
};

describe('Logs Table Sorting', () => {
it('sorts same-timestamp logs by observedTimestamp', () => {
cy.intercept(QUERY_RANGE_STREAMS_URL_MATCH, sameTimestampStreamsResponse()).as(
'queryRangeStreams',
);
cy.intercept(QUERY_RANGE_MATRIX_URL_MATCH, { statusCode: 200, body: {} });

cy.visit(LOGS_PAGE_URL);

cy.wait('@queryRangeStreams');

// Default direction is 'backward' (desc) — largest observedTimestamp first
// observedTimestamp order: C(+300) > B(+200) > A(+100)
cy.byTestID(TestIds.LogsTable)
.should('exist')
.within(() => {
cy.get('td[data-label="message"]').should('have.length', 3);
cy.get('td[data-label="message"]').eq(0).should('contain', 'log-line-C');
cy.get('td[data-label="message"]').eq(1).should('contain', 'log-line-B');
cy.get('td[data-label="message"]').eq(2).should('contain', 'log-line-A');
});
});
});
2 changes: 1 addition & 1 deletion web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"lint:tsc": "tsc --noEmit",
"cypress:open": "cypress open",
"cypress:run": "cypress run",
"cypress:run:ci": "NO_COLOR=1 npx cypress run --spec cypress/e2e/integration/*.cy.ts --browser electron",
"cypress:run:ci": "NO_COLOR=1 cypress run --spec \"cypress/e2e/integration/*.cy.ts\" --browser electron",
"test": "concurrently -n \"unit,e2e\" --kill-others-on-fail \"npm run test:unit\" \"npm run test:e2e\"",
"test:unit": "TZ=UTC jest --config jest.config.js",
"test:e2e": "./scripts/run-cypress.sh",
Expand Down
128 changes: 128 additions & 0 deletions web/src/__tests__/numeric-comparator.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import { bigIntDifference, numericComparator } from '../sort-utils';

describe('numericComparator', () => {
it('should return -1 when a < b with positive multiplier', () => {
expect(numericComparator(1, 2, 1)).toBe(-1);
});

it('should return 1 when a > b with positive multiplier', () => {
expect(numericComparator(2, 1, 1)).toBe(1);
});

it('should return 0 when a === b', () => {
expect(numericComparator(1, 1, 1)).toBe(0);
});

it('should invert result with negative multiplier (descending sort)', () => {
expect(numericComparator(1, 2, -1)).toBe(1);
expect(numericComparator(2, 1, -1)).toBe(-1);
});

it('should use fallback comparison when values are equal', () => {
expect(numericComparator(5, 5, 1, 3)).toBe(3);
expect(numericComparator(5, 5, 1, -2)).toBe(-2);
});

it('should apply direction multiplier to fallback comparison', () => {
expect(numericComparator(5, 5, -1, 3)).toBe(-3);
expect(numericComparator(5, 5, -1, -2)).toBe(2);
});
Comment thread
jgbernalp marked this conversation as resolved.

it('should ignore fallback when values are not equal', () => {
expect(numericComparator(1, 2, 1, 100)).toBe(-1);
expect(numericComparator(2, 1, 1, 100)).toBe(1);
});

it('should work with timestamps', () => {
const timestamp1 = 1679000000000;
const timestamp2 = 1679000000001;
expect(numericComparator(timestamp1, timestamp2, 1)).toBe(-1);
expect(numericComparator(timestamp2, timestamp1, 1)).toBe(1);
expect(numericComparator(timestamp1, timestamp1, 1)).toBe(0);
});

it('should use logIndex as tiebreaker for equal timestamps in ascending sort', () => {
const timestamp = 1679000000000;
const logs = [
{ timestamp, logIndex: 0 },
{ timestamp, logIndex: 1 },
{ timestamp, logIndex: 2 },
];

const sortedAsc = [...logs].sort((a, b) =>
numericComparator(a.timestamp, b.timestamp, 1, a.logIndex - b.logIndex),
);
expect(sortedAsc.map((l) => l.logIndex)).toEqual([0, 1, 2]);
});

it('should use logIndex as tiebreaker for equal timestamps in descending sort', () => {
const timestamp = 1679000000000;
const logs = [
{ timestamp, logIndex: 0 },
{ timestamp, logIndex: 1 },
{ timestamp, logIndex: 2 },
];

const sortedDesc = [...logs].sort((a, b) =>
numericComparator(a.timestamp, b.timestamp, -1, a.logIndex - b.logIndex),
);
expect(sortedDesc.map((l) => l.logIndex)).toEqual([2, 1, 0]);
});
});

describe('observedTimestampDifference', () => {
it('should return the numeric difference for small bigints', () => {
expect(bigIntDifference(10n, 3n)).toBe(7);
expect(bigIntDifference(3n, 10n)).toBe(-7);
});

it('should return 0 when both values are equal', () => {
expect(bigIntDifference(42n, 42n)).toBe(0);
});

it('should clamp to MIN_SAFE_INTEGER when difference is too negative', () => {
const a = 0n;
const b = BigInt(Number.MAX_SAFE_INTEGER) + 1n;
expect(bigIntDifference(a, b)).toBe(Number.MIN_SAFE_INTEGER);
});

it('should clamp to MAX_SAFE_INTEGER when difference is too positive', () => {
const a = BigInt(Number.MAX_SAFE_INTEGER) + 1n;
const b = 0n;
expect(bigIntDifference(a, b)).toBe(Number.MAX_SAFE_INTEGER);
});

it('should not clamp when difference is exactly MAX_SAFE_INTEGER', () => {
const maxSafe = BigInt(Number.MAX_SAFE_INTEGER);
expect(bigIntDifference(maxSafe, 0n)).toBe(Number.MAX_SAFE_INTEGER);
});

it('should not clamp when difference is exactly MIN_SAFE_INTEGER', () => {
const minSafe = BigInt(Number.MIN_SAFE_INTEGER);
expect(bigIntDifference(minSafe, 0n)).toBe(Number.MIN_SAFE_INTEGER);
});

it('should clamp when difference is one past MAX_SAFE_INTEGER', () => {
const pastMax = BigInt(Number.MAX_SAFE_INTEGER) + 1n;
expect(bigIntDifference(pastMax, 0n)).toBe(Number.MAX_SAFE_INTEGER);
});

it('should clamp when difference is one past MIN_SAFE_INTEGER', () => {
const pastMin = BigInt(Number.MIN_SAFE_INTEGER) - 1n;
expect(bigIntDifference(pastMin, 0n)).toBe(Number.MIN_SAFE_INTEGER);
});

it('should handle realistic nanosecond timestamp differences', () => {
const ts1 = 1679000000000000000n;
const ts2 = 1679000000000000001n;
expect(bigIntDifference(ts2, ts1)).toBe(1);
expect(bigIntDifference(ts1, ts2)).toBe(-1);
});

it('should clamp when nanosecond timestamps are far apart', () => {
const ts1 = 1679000000000000000n;
const ts2 = 1579000000000000000n;
expect(bigIntDifference(ts1, ts2)).toBe(Number.MAX_SAFE_INTEGER);
expect(bigIntDifference(ts2, ts1)).toBe(Number.MIN_SAFE_INTEGER);
});
});
43 changes: 30 additions & 13 deletions web/src/components/logs-table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
} from '../logs.types';
import { parseName, parseResources, ResourceLabel } from '../parse-resources';
import { severityFromString } from '../severity';
import { numericComparator, bigIntDifference } from '../sort-utils';
import { TestIds } from '../test-ids';
import { LogDetail } from './log-detail';
import './logs-table.css';
Expand All @@ -50,8 +51,6 @@ interface LogsTableProps {
schema: Schema;
}

type TableCellValue = string | number | Resource | Array<Resource>;

const isJSONObject = (value: string): boolean => {
const trimmedValue = value.trim();

Expand All @@ -64,7 +63,14 @@ const streamToTableData = (stream: StreamLogData, timezone?: string): Array<LogT
return values.map((value) => {
const logValue = String(value[1]);
const message = isJSONObject(logValue) ? stream.stream['message'] || logValue : logValue;
const timestamp = parseFloat(String(value[0]));
const rawTimestamp = String(value[0]);
const timestamp = parseFloat(rawTimestamp);
const observedTimestamp = stream.stream.observedTimestamp
? BigInt(stream.stream.observedTimestamp)
: undefined;
const openshiftSequence = stream.stream.openshift_sequence
? BigInt(stream.stream.openshift_sequence)
: undefined;
const time = timestamp / 1e6;
const formattedTime = dateToFormat(time, DateFormat.Full, timezone);
const severity = parseName(stream.stream, ResourceLabel.Severity);
Expand All @@ -81,6 +87,8 @@ const streamToTableData = (stream: StreamLogData, timezone?: string): Array<LogT
namespace,
podName,
type: 'log',
observedTimestamp,
openshiftSequence,
// index is 0 here to match the type, but it will be recalculated when flattening the array
logIndex: 0,
};
Expand Down Expand Up @@ -108,13 +116,6 @@ const getSeverityClass = (severity: string) => {
return severity ? `lv-plugin__table__severity-${severity}` : '';
};

// sort with an appropriate numeric comparator for big floats
const numericComparator = <T extends TableCellValue>(
a: T,
b: T,
directionMultiplier: number,
): number => (a < b ? -1 : a > b ? 1 : 0) * directionMultiplier;

const columns: Array<TableColumn<LogTableData>> = [
{
id: 'expand',
Expand All @@ -131,7 +132,14 @@ const columns: Array<TableColumn<LogTableData>> = [
},
sort: (data, sortDirection) =>
data.sort((a, b) =>
numericComparator(a.timestamp, b.timestamp, sortDirection === 'asc' ? 1 : -1),
numericComparator(
a.timestamp,
b.timestamp,
sortDirection === 'asc' ? 1 : -1,
a.openshiftSequence !== undefined && b.openshiftSequence !== undefined
? bigIntDifference(a.openshiftSequence, b.openshiftSequence)
: bigIntDifference(a.observedTimestamp, b.observedTimestamp),
),
),
},
{
Expand Down Expand Up @@ -338,8 +346,17 @@ export const LogsTable: FC<PropsWithChildren<LogsTableProps>> = ({
}
}

return dataCopy.sort((a, b) => numericComparator(a.timestamp, b.timestamp, -1));
}, [tableData, sortBy]);
return dataCopy.sort((a, b) =>
numericComparator(
a.timestamp,
b.timestamp,
direction === 'backward' ? -1 : 1,
a.openshiftSequence !== undefined && b.openshiftSequence !== undefined
? bigIntDifference(a.openshiftSequence, b.openshiftSequence)
: bigIntDifference(a.observedTimestamp, b.observedTimestamp),
),
);
}, [tableData, sortBy, direction]);

const dataIsEmpty = sortedData.length === 0;

Expand Down
2 changes: 2 additions & 0 deletions web/src/logs.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,6 @@ export type LogTableData = {
data: Record<string, string>;
type: 'log' | 'expand';
logIndex: number;
observedTimestamp?: bigint;
openshiftSequence?: bigint;
};
28 changes: 28 additions & 0 deletions web/src/sort-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
type SortableValue = string | number;

// sort with an appropriate numeric comparator for big floats
export const numericComparator = <T extends SortableValue>(
a: T,
b: T,
directionMultiplier: number,
fallbackComparison?: number,
): number => {
const result = a < b ? -1 : a > b ? 1 : 0;
if (result === 0 && fallbackComparison !== undefined) {
return fallbackComparison * directionMultiplier;
Comment thread
jgbernalp marked this conversation as resolved.
}
return result * directionMultiplier;
};

const MIN_SAFE_BIGINT = BigInt(Number.MIN_SAFE_INTEGER);
const MAX_SAFE_BIGINT = BigInt(Number.MAX_SAFE_INTEGER);

export const bigIntDifference = (a: bigint | undefined, b: bigint | undefined): number => {
if (a === undefined || b === undefined) {
return 0;
}
const difference = a - b;
if (difference < MIN_SAFE_BIGINT) return Number.MIN_SAFE_INTEGER;
if (difference > MAX_SAFE_BIGINT) return Number.MAX_SAFE_INTEGER;
return Number(difference);
};