Skip to content

Commit

Permalink
Traces custom source - Bug Fixes (#2298) (#2307)
Browse files Browse the repository at this point in the history 
* fix custom traces filter bug, add default metric selection



* helper function for start time, handle date and datenanos



* add toast messages for all errors service-traces, add mode to url



* fix url re-direction in non custom state



* address comments, add testing



* update url #, send mode to applications



* update helper function



* add validation to url mode



* fix applications removing traces redirect



---------



(cherry picked from commit a57c10e)

Signed-off-by: Adam Tackett <tackadam@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Adam Tackett <tackadam@amazon.com>
  • Loading branch information
@opensearch-trigger-bot @github-actions
3 people authored Jan 8, 2025
1 parent a254c47 commit a1cc0f0
Show file tree
Hide file tree
Showing 14 changed files with 451 additions and 112 deletions.
54 changes: 54 additions & 0 deletions public/components/trace_analytics/components/common/__tests__/helper_functions.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
TEST_SERVICE_MAP_GRAPH,
} from '../../../../../../test/constants';
import {
appendModeToTraceViewUri,
calculateTicks,
filtersToDsl,
fixedIntervalToMilli,
Expand All @@ -22,6 +23,7 @@ import {
getServiceMapGraph,
getServiceMapScaleColor,
getServiceMapTargetResources,
getTimestampPrecision,
milliToNanoSec,
minFixedInterval,
MissingConfigurationMessage,
Expand Down Expand Up @@ -195,4 +197,56 @@ describe('Helper functions', () => {
expect(result).toEqual([]);
});
});

describe('getTimestampPrecision', () => {
it('returns "millis" for 13-digit timestamps', () => {
expect(getTimestampPrecision(1703599200000)).toEqual('millis');
});

it('returns "micros" for 16-digit timestamps', () => {
expect(getTimestampPrecision(1703599200000000)).toEqual('micros');
});

it('returns "nanos" for 19-digit timestamps', () => {
expect(getTimestampPrecision(1703599200000000000)).toEqual('nanos');
});

it('returns "millis" for invalid or missing timestamps', () => {
expect(getTimestampPrecision((undefined as unknown) as number)).toEqual('millis');
expect(getTimestampPrecision(123)).toEqual('millis');
});
});

describe('appendModeToTraceViewUri', () => {
const mockGetTraceViewUri = (traceId: string) => `#/traces/${traceId}`;

it('appends mode to the URI when mode is provided', () => {
const result = appendModeToTraceViewUri('123', mockGetTraceViewUri, 'data_prepper');
expect(result).toEqual('#/traces/123?mode=data_prepper');
});

it('appends mode correctly for hash router URIs with existing query params', () => {
const result = appendModeToTraceViewUri('123', (id) => `#/traces/${id}?foo=bar`, 'jaeger');
expect(result).toEqual('#/traces/123?foo=bar&mode=jaeger');
});

it('does not append mode if not provided', () => {
const result = appendModeToTraceViewUri('123', mockGetTraceViewUri, null);
expect(result).toEqual('#/traces/123');
});

it('handles URIs without a hash router', () => {
const result = appendModeToTraceViewUri(
'123',
(id) => `/traces/${id}`,
'custom_data_prepper'
);
expect(result).toEqual('/traces/123?mode=custom_data_prepper');
});

it('handles URIs without a hash router and existing query params', () => {
const result = appendModeToTraceViewUri('123', (id) => `/traces/${id}?foo=bar`, 'jaeger');
expect(result).toEqual('/traces/123?foo=bar&mode=jaeger');
});
});
});
35 changes: 35 additions & 0 deletions public/components/trace_analytics/components/common/helper_functions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,41 @@ export function nanoToMilliSec(nano: number) {
return nano / 1000000;
}

export const getTimestampPrecision = (timestamp: number): 'millis' | 'micros' | 'nanos' => {
if (!timestamp) return 'millis';

const digitCount = timestamp.toString().length;

// For Unix timestamps:
// 13 digits = milliseconds (e.g., 1703599200000)
// 16 digits = microseconds (e.g., 1703599200000000)
// 19 digits = nanoseconds (e.g., 1703599200000000000)
switch (digitCount) {
case 13:
return 'millis';
case 16:
return 'micros';
case 19:
return 'nanos';
default:
return 'millis';
}
};

export const appendModeToTraceViewUri = (
traceId: string,
getTraceViewUri?: (traceId: string) => string,
traceMode?: string | null
): string => {
const baseUri = getTraceViewUri ? getTraceViewUri(traceId) : '';
const separator = baseUri.includes('?') ? '&' : '?';

if (traceMode) {
return `${baseUri}${separator}mode=${encodeURIComponent(traceMode)}`;
}
return baseUri;
};

export function microToMilliSec(micro: number) {
if (typeof micro !== 'number') return 0;
return micro / 1000;
Expand Down
29 changes: 17 additions & 12 deletions public/components/trace_analytics/components/common/plots/service_map.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,6 @@ export function ServiceMap({
);
}, [filters, focusedService]);

const onChangeSelectable = (value: React.SetStateAction<Array<EuiSuperSelectOption<any>>>) => {
// if the change is changing for the first time then callback servicemap with metrics
if (selectableValue.length === 0 && value.length !== 0) {
if (includeMetricsCallback) {
includeMetricsCallback();
}
}
setIdSelected(value);
setSelectableValue(value);
};

const metricOptions: Array<EuiSuperSelectOption<any>> = [
{
value: 'latency',
Expand All @@ -149,6 +138,19 @@ export function ServiceMap({
},
];

// For the traces custom page
useEffect(() => {
if (!selectableValue || selectableValue.length === 0) {
// Set to the first option ("latency") and trigger the onChange function
const defaultValue = metricOptions[0].value;
setSelectableValue(defaultValue); // Update the state
setIdSelected(defaultValue); // Propagate the default to parent
if (includeMetricsCallback) {
includeMetricsCallback();
}
}
}, []);

const removeFilter = (field: string, value: string) => {
if (!setFilters) return;
const updatedFilters = filters.filter(
Expand Down Expand Up @@ -504,7 +506,10 @@ export function ServiceMap({
compressed
options={metricOptions}
valueOfSelected={selectableValue}
onChange={(value) => onChangeSelectable(value)}
onChange={(value) => {
setSelectableValue(value);
setIdSelected(value);
}}
/>
</EuiFlexItem>
)}
Expand Down
23 changes: 23 additions & 0 deletions public/components/trace_analytics/components/dashboard/mode_picker.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,28 @@ export function DataSourcePicker(props: {
);
};

const updateUrlWithMode = (key: TraceAnalyticsMode) => {
const currentUrl = window.location.href.split('#')[0];
const hash = window.location.hash;

if (hash) {
const [hashPath, hashQueryString] = hash.substring(1).split('?');
const queryParams = new URLSearchParams(hashQueryString || '');
queryParams.set('mode', key);

const newHash = `${hashPath}?${queryParams.toString()}`;
const newUrl = `${currentUrl}#${newHash}`;
window.history.replaceState(null, '', newUrl);
} else {
// Non-hash-based URL
const queryParams = new URLSearchParams(window.location.search);
queryParams.set('mode', key);

const newUrl = `${currentUrl}?${queryParams.toString()}`;
window.history.replaceState(null, '', newUrl);
}
};

return (
<>
<EuiPopover
Expand Down Expand Up @@ -94,6 +116,7 @@ export function DataSourcePicker(props: {
key: TraceAnalyticsMode;
};
setMode(choice.key);
updateUrlWithMode(choice.key);
setPopoverIsOpen(false);
sessionStorage.setItem('TraceAnalyticsMode', choice.key);
}}
Expand Down
3 changes: 3 additions & 0 deletions ...components/trace_analytics/components/traces/__tests__/__snapshots__/traces.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,7 @@ exports[`Traces component renders empty traces page 1`] = `
items={Array []}
loading={true}
mode="data_prepper"
page="traces"
refresh={[Function]}
>
<EuiPanel>
Expand Down Expand Up @@ -3930,6 +3931,7 @@ exports[`Traces component renders jaeger traces page 1`] = `
jaegerIndicesExist={true}
loading={true}
mode="jaeger"
page="traces"
refresh={[Function]}
>
<EuiPanel>
Expand Down Expand Up @@ -6217,6 +6219,7 @@ exports[`Traces component renders traces page 1`] = `
items={Array []}
loading={true}
mode="data_prepper"
page="traces"
refresh={[Function]}
>
<EuiPanel>
Expand Down
18 changes: 10 additions & 8 deletions ...ents/trace_analytics/components/traces/__tests__/__snapshots__/traces_table.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1121,12 +1121,13 @@ exports[`Traces table component renders jaeger traces table 1`] = `
>
<EuiLink
data-test-subj="trace-link"
href="#/trace_analytics/traces/00079a615e31e61766fcb20b557051c1"
>
<button
<a
className="euiLink euiLink--primary"
data-test-subj="trace-link"
disabled={false}
type="button"
href="#/trace_analytics/traces/00079a615e31e61766fcb20b557051c1"
rel="noreferrer"
>
<EuiText
className="traces-table traces-table-trace-id"
Expand All @@ -1140,7 +1141,7 @@ exports[`Traces table component renders jaeger traces table 1`] = `
00079a615e31e61766fcb20b557051c1
</div>
</EuiText>
</button>
</a>
</EuiLink>
</div>
</EuiFlexItem>
Expand Down Expand Up @@ -2686,12 +2687,13 @@ exports[`Traces table component renders traces table 1`] = `
>
<EuiLink
data-test-subj="trace-link"
href="#/trace_analytics/traces/00079a615e31e61766fcb20b557051c1"
>
<button
<a
className="euiLink euiLink--primary"
data-test-subj="trace-link"
disabled={false}
type="button"
href="#/trace_analytics/traces/00079a615e31e61766fcb20b557051c1"
rel="noreferrer"
>
<EuiText
className="traces-table traces-table-trace-id"
Expand All @@ -2705,7 +2707,7 @@ exports[`Traces table component renders traces table 1`] = `
00079a615e31e61766fcb20b557051c1
</div>
</EuiText>
</button>
</a>
</EuiLink>
</div>
</EuiFlexItem>
Expand Down
9 changes: 3 additions & 6 deletions public/components/trace_analytics/components/traces/__tests__/traces_table.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ describe('Traces table component', () => {

it('renders empty traces table message', () => {
const refresh = jest.fn();
const getTraceViewUri = (item: any) =>
location.assign(`#/trace_analytics/traces/${encodeURIComponent(item)}`);
const getTraceViewUri = (item: any) => `#/trace_analytics/traces/${encodeURIComponent(item)}`;
const noIndicesTable = mount(
<TracesTable
items={[]}
Expand Down Expand Up @@ -55,8 +54,7 @@ describe('Traces table component', () => {
actions: '#',
},
];
const getTraceViewUri = (item: any) =>
location.assign(`#/trace_analytics/traces/${encodeURIComponent(item)}`);
const getTraceViewUri = (item: any) => `#/trace_analytics/traces/${encodeURIComponent(item)}`;
const refresh = jest.fn();
const wrapper = mount(
<TracesTable
Expand Down Expand Up @@ -86,8 +84,7 @@ describe('Traces table component', () => {
actions: '#',
},
];
const getTraceViewUri = (item: any) =>
location.assign(`#/trace_analytics/traces/${encodeURIComponent(item)}`);
const getTraceViewUri = (item: any) => `#/trace_analytics/traces/${encodeURIComponent(item)}`;
const refresh = jest.fn();
const wrapper = mount(
<TracesTable
Expand Down
Loading

0 comments on commit a1cc0f0

Please sign in to comment.