mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
Fix optimize order by shortcomings, including for otel_traces (#2014)
This commit is contained in:
Aaron Knudtson
GitHub
parent
941d045077
commit
05a1b76588
3 changed files with 63 additions and 83 deletions
5
.changeset/lovely-radios-walk.md
Normal file
5
.changeset/lovely-radios-walk.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@hyperdx/app": patch
|
||||
---
|
||||
fix: optimize order by should factor in wider cases, including the
|
||||
default otel_traces
|
||||
|
|
@ -710,74 +710,40 @@ function useSearchedConfigToChartConfig(
|
|||
]);
|
||||
}
|
||||
|
||||
const implicitDateTimePrefixes = [
|
||||
'toStartOf',
|
||||
'toUnixTimestamp',
|
||||
'toDateTime',
|
||||
'Timestamp',
|
||||
] as const;
|
||||
|
||||
function optimizeDefaultOrderBy(
|
||||
timestampExpr: string,
|
||||
displayedTimestampExpr: string | undefined,
|
||||
sortingKey: string | undefined,
|
||||
) {
|
||||
const defaultModifier = 'DESC';
|
||||
const firstTimestampValueExpression =
|
||||
getFirstTimestampValueExpression(timestampExpr ?? '') ?? '';
|
||||
const defaultOrderByItems = [firstTimestampValueExpression];
|
||||
const trimmedDisplayedTimestampExpr = displayedTimestampExpr?.trim();
|
||||
const orderByArr: string[] = [];
|
||||
|
||||
if (
|
||||
trimmedDisplayedTimestampExpr &&
|
||||
trimmedDisplayedTimestampExpr !== firstTimestampValueExpression
|
||||
) {
|
||||
defaultOrderByItems.push(trimmedDisplayedTimestampExpr);
|
||||
const timestampExprParts = splitAndTrimWithBracket(timestampExpr);
|
||||
const keys = splitAndTrimWithBracket(sortingKey ?? '');
|
||||
keys.push(...timestampExprParts);
|
||||
if (displayedTimestampExpr) {
|
||||
keys.push(displayedTimestampExpr.trim());
|
||||
}
|
||||
|
||||
const fallbackOrderBy =
|
||||
defaultOrderByItems.length > 1
|
||||
? `(${defaultOrderByItems.join(', ')}) ${defaultModifier}`
|
||||
: `${defaultOrderByItems[0]} ${defaultModifier}`;
|
||||
|
||||
if (!sortingKey) return fallbackOrderBy;
|
||||
|
||||
const orderByArr = [];
|
||||
const sortKeys = splitAndTrimWithBracket(sortingKey);
|
||||
for (let i = 0; i < sortKeys.length; i++) {
|
||||
const sortKey = sortKeys[i];
|
||||
for (const key of keys) {
|
||||
if (
|
||||
sortKey.includes('toStartOf') &&
|
||||
sortKey.includes(firstTimestampValueExpression)
|
||||
!orderByArr.includes(key) &&
|
||||
(implicitDateTimePrefixes.some(v => key.startsWith(v)) ||
|
||||
timestampExprParts.includes(key) ||
|
||||
displayedTimestampExpr?.trim() === key)
|
||||
) {
|
||||
orderByArr.push(sortKey);
|
||||
} else if (
|
||||
sortKey === firstTimestampValueExpression ||
|
||||
(sortKey.startsWith('toUnixTimestamp') &&
|
||||
sortKey.includes(firstTimestampValueExpression)) ||
|
||||
(sortKey.startsWith('toDateTime') &&
|
||||
sortKey.includes(firstTimestampValueExpression))
|
||||
) {
|
||||
if (orderByArr.length === 0) {
|
||||
// fallback if the first sort key is the timestamp sort key
|
||||
return fallbackOrderBy;
|
||||
} else {
|
||||
orderByArr.push(sortKey);
|
||||
break;
|
||||
}
|
||||
} else if (sortKey === trimmedDisplayedTimestampExpr) {
|
||||
orderByArr.push(sortKey);
|
||||
orderByArr.push(key);
|
||||
}
|
||||
}
|
||||
|
||||
// If we can't find an optimized order by, use the fallback/default
|
||||
if (orderByArr.length === 0) {
|
||||
return fallbackOrderBy;
|
||||
}
|
||||
|
||||
if (
|
||||
trimmedDisplayedTimestampExpr &&
|
||||
!orderByArr.includes(trimmedDisplayedTimestampExpr)
|
||||
) {
|
||||
orderByArr.push(trimmedDisplayedTimestampExpr);
|
||||
}
|
||||
|
||||
return orderByArr.length > 1
|
||||
? `(${orderByArr.join(', ')}) ${defaultModifier}`
|
||||
: `${orderByArr[0]} ${defaultModifier}`;
|
||||
? `(${orderByArr.join(', ')}) DESC`
|
||||
: `${orderByArr[0]} DESC`;
|
||||
}
|
||||
|
||||
export function useDefaultOrderBy(sourceID: string | undefined | null) {
|
||||
|
|
|
|||
|
|
@ -28,32 +28,28 @@ describe('useDefaultOrderBy', () => {
|
|||
expected: 'Timestamp DESC',
|
||||
},
|
||||
{
|
||||
// Traces Table
|
||||
sortingKey: 'ServiceName, SpanName, toDateTime(Timestamp)',
|
||||
expected: 'Timestamp DESC',
|
||||
expected: '(toDateTime(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
// Optimized Traces Table
|
||||
sortingKey:
|
||||
'toStartOfHour(Timestamp), ServiceName, SpanName, toDateTime(Timestamp)',
|
||||
expected: '(toStartOfHour(Timestamp), toDateTime(Timestamp)) DESC',
|
||||
expected:
|
||||
'(toStartOfHour(Timestamp), toDateTime(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
// Unsupported for now as it's not a great sort key, want to just
|
||||
// use default behavior for this
|
||||
sortingKey: 'toDateTime(Timestamp), ServiceName, SpanName, Timestamp',
|
||||
expected: 'Timestamp DESC',
|
||||
},
|
||||
{
|
||||
// Unsupported prefix sort key
|
||||
sortingKey: 'toDateTime(Timestamp), ServiceName, SpanName',
|
||||
expected: 'Timestamp DESC',
|
||||
},
|
||||
{
|
||||
// Inverted sort key order, we should not try to optimize this
|
||||
sortingKey:
|
||||
'ServiceName, toDateTime(Timestamp), SeverityText, toStartOfHour(Timestamp)',
|
||||
expected: 'Timestamp DESC',
|
||||
'toStartOfHour(Timestamp), ServiceName, SpanName, toDateTime(Timestamp)',
|
||||
expected:
|
||||
'(toStartOfHour(Timestamp), toDateTime(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'toDateTime(Timestamp), ServiceName, SpanName, Timestamp',
|
||||
expected: '(toDateTime(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'toDateTime(Timestamp), ServiceName, SpanName',
|
||||
expected: '(toDateTime(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'toStartOfHour(Timestamp), other_column, Timestamp',
|
||||
|
|
@ -71,14 +67,14 @@ describe('useDefaultOrderBy', () => {
|
|||
sortingKey:
|
||||
'toStartOfMinute(Timestamp), user_id, status, toUnixTimestamp(Timestamp)',
|
||||
expected:
|
||||
'(toStartOfMinute(Timestamp), toUnixTimestamp(Timestamp)) DESC',
|
||||
'(toStartOfMinute(Timestamp), toUnixTimestamp(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
// test variation of toUnixTimestamp
|
||||
sortingKey:
|
||||
'toStartOfMinute(Timestamp), user_id, status, toUnixTimestamp64Nano(Timestamp)',
|
||||
expected:
|
||||
'(toStartOfMinute(Timestamp), toUnixTimestamp64Nano(Timestamp)) DESC',
|
||||
'(toStartOfMinute(Timestamp), toUnixTimestamp64Nano(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey:
|
||||
|
|
@ -91,6 +87,24 @@ describe('useDefaultOrderBy', () => {
|
|||
timestampValueExpression: 'Timestamp, toStartOfMinute(Timestamp)',
|
||||
expected: '(toStartOfMinute(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'toStartOfMinute(Timestamp), user_id, status, Timestamp',
|
||||
timestampValueExpression: 'toStartOfMinute(Timestamp), Timestamp',
|
||||
expected: '(toStartOfMinute(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'toStartOfMinute(Timestamp), user_id, status, Timestamp',
|
||||
expected: '(toStartOfMinute(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'toStartOfMinute(Timestamp), user_id, status',
|
||||
expected: '(toStartOfMinute(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'toStartOfMinute(Timestamp), user_id, status',
|
||||
timestampValueExpression: 'toStartOfMinute(Timestamp), Timestamp',
|
||||
expected: '(toStartOfMinute(Timestamp), Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'Timestamp',
|
||||
displayedTimestampValueExpression: 'Timestamp64',
|
||||
|
|
@ -103,7 +117,6 @@ describe('useDefaultOrderBy', () => {
|
|||
},
|
||||
{
|
||||
sortingKey: 'Timestamp',
|
||||
displayedTimestampValueExpression: 'Timestamp',
|
||||
expected: 'Timestamp DESC',
|
||||
},
|
||||
{
|
||||
|
|
@ -141,20 +154,16 @@ describe('useDefaultOrderBy', () => {
|
|||
{
|
||||
sortingKey: 'ServiceName, TimestampTime, Timestamp',
|
||||
timestampValueExpression: 'TimestampTime, Timestamp',
|
||||
displayedTimestampValueExpression: 'Timestamp',
|
||||
expected: '(TimestampTime, Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: 'ServiceName, TimestampTime, Timestamp',
|
||||
timestampValueExpression: 'Timestamp, TimestampTime',
|
||||
displayedTimestampValueExpression: 'Timestamp',
|
||||
expected: 'Timestamp DESC',
|
||||
expected: '(TimestampTime, Timestamp) DESC',
|
||||
},
|
||||
{
|
||||
sortingKey: '',
|
||||
timestampValueExpression: 'Timestamp, TimestampTime',
|
||||
displayedTimestampValueExpression: '',
|
||||
expected: 'Timestamp DESC',
|
||||
sortingKey: 'ServiceName, TimestampTime, Timestamp',
|
||||
expected: '(TimestampTime, Timestamp) DESC',
|
||||
},
|
||||
];
|
||||
for (const testCase of testCases) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue