Skip to content

Commit 863cbe8

Browse files
authored
fix: Normalize uppercase trace ids to lowercase in URL (#3485)
## Which problem is this PR solving? - Resolves #3477 ## Description of the changes - Replaced broken class component normalization with useEffect hook - Uses useNavigate with replace option for URL normalization - Preserves query parameters during normalization - Updated test to match new implementation ## How was this change tested? - All existing unit tests pass (87/87 tests in TracePage suite) - Updated the "forces lowercase id" test to verify new implementation behavior - Manually tested with uppercase trace ID: `http://localhost:5173/trace/B36DE06C5972AB071AC119C504CA07DC` successfully auto-normalizes to lowercase - Verified already-lowercase URLs don't trigger unnecessary redirects - TypeScript compilation successful ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Samar <hello.samar7@gmail.com>
1 parent eba19f5 commit 863cbe8

File tree

4 files changed

+102
-18
lines changed

4 files changed

+102
-18
lines changed

packages/jaeger-ui/src/components/TracePage/index.test.js

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -347,21 +347,15 @@ describe('<TracePage>', () => {
347347
expect(screen.getByTestId('loading-indicator')).toBeInTheDocument();
348348
});
349349

350-
it('forces lowercase id', () => {
351-
const replaceMock = jest.fn();
350+
it('renders without error when given uppercase id', () => {
351+
// URL normalization is handled by the useNormalizeTraceId hook in the wrapper.
352+
// This test verifies TracePageImpl renders successfully with uppercase IDs.
352353
const props = {
353354
...defaultProps,
354355
id: trace.traceID.toUpperCase(),
355-
history: {
356-
replace: replaceMock,
357-
},
358356
};
359-
render(<TracePage {...props} />);
360-
expect(replaceMock).toHaveBeenCalledWith(
361-
expect.objectContaining({
362-
pathname: expect.stringContaining(trace.traceID),
363-
})
364-
);
357+
expect(() => render(<TracePage {...props} />)).not.toThrow();
358+
expect(document.querySelector('.Tracepage--headerSection')).toBeInTheDocument();
365359
});
366360

367361
it('focuses on search bar when there is a search bar and focusOnSearchBar is called', () => {

packages/jaeger-ui/src/components/TracePage/index.tsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import * as React from 'react';
55
import { InputRef } from 'antd';
6+
import { useNormalizeTraceId } from './useNormalizeTraceId';
67
import { Location, History as RouterHistory } from 'history';
78
import _clamp from 'lodash/clamp';
89
import _get from 'lodash/get';
@@ -31,7 +32,7 @@ import TracePageHeader from './TracePageHeader';
3132
import TraceTimelineViewer from './TraceTimelineViewer';
3233
import { actions as timelineActions } from './TraceTimelineViewer/duck';
3334
import { TUpdateViewRangeTimeFunction, IViewRange, ViewRangeTimeUpdate, ETraceViewType } from './types';
34-
import { getLocation, getUrl } from './url';
35+
import { getUrl } from './url';
3536
import ErrorMessage from '../common/ErrorMessage';
3637
import LoadingIndicator from '../common/LoadingIndicator';
3738
import { extractUiFindFromState } from '../common/UiFindInput';
@@ -291,14 +292,9 @@ export class TracePageImpl extends React.PureComponent<TProps, TState> {
291292
};
292293

293294
ensureTraceFetched() {
294-
const { fetchTrace, location, trace, id } = this.props;
295+
const { fetchTrace, trace, id } = this.props;
295296
if (!trace) {
296297
fetchTrace(id);
297-
return;
298-
}
299-
const { history } = this.props;
300-
if (id && id !== id.toLowerCase()) {
301-
history.replace(getLocation(id.toLowerCase(), location.state));
302298
}
303299
}
304300

@@ -524,9 +520,13 @@ type TracePageProps = {
524520

525521
const TracePage = (props: TracePageProps) => {
526522
const config = useConfig();
523+
const traceID = props.params.id;
524+
const normalizedTraceID = useNormalizeTraceId(traceID);
525+
527526
return (
528527
<ConnectedTracePage
529528
{...props}
529+
params={{ ...props.params, id: normalizedTraceID }}
530530
archiveEnabled={Boolean(config.archiveEnabled)}
531531
enableSidePanel={Boolean(config.traceTimeline?.enableSidePanel)}
532532
storageCapabilities={config.storageCapabilities}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright (c) 2026 The Jaeger Authors.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
const mockNavigate = jest.fn();
5+
let mockLocation: { search: string; state: unknown } = { search: '', state: null };
6+
7+
jest.mock('react-router-dom-v5-compat', () => ({
8+
...jest.requireActual('react-router-dom-v5-compat'),
9+
useNavigate: () => mockNavigate,
10+
useLocation: () => mockLocation,
11+
}));
12+
13+
// Simplified mock sufficient for testing normalization logic;
14+
// URL prefix handling is tested separately in url/index.test.js
15+
jest.mock('./url', () => ({
16+
getUrl: (id: string) => `/trace/${id}`,
17+
}));
18+
19+
import { renderHook } from '@testing-library/react';
20+
21+
import { useNormalizeTraceId } from './useNormalizeTraceId';
22+
23+
describe('useNormalizeTraceId', () => {
24+
beforeEach(() => {
25+
mockNavigate.mockClear();
26+
mockLocation = { search: '', state: null };
27+
});
28+
29+
it('normalizes uppercase trace IDs to lowercase in the URL', () => {
30+
const uppercaseTraceId = 'ABC123DEF456';
31+
const lowercaseTraceId = uppercaseTraceId.toLowerCase();
32+
33+
const { result } = renderHook(() => useNormalizeTraceId(uppercaseTraceId));
34+
35+
expect(result.current).toBe(lowercaseTraceId);
36+
expect(mockNavigate).toHaveBeenCalledWith(`/trace/${lowercaseTraceId}`, {
37+
replace: true,
38+
state: null,
39+
});
40+
});
41+
42+
it('preserves query parameters during trace ID normalization', () => {
43+
const uppercaseTraceId = 'ABC123DEF456';
44+
const lowercaseTraceId = uppercaseTraceId.toLowerCase();
45+
const searchParams = '?uiFind=foo&x=1';
46+
47+
mockLocation = { search: searchParams, state: null };
48+
49+
const { result } = renderHook(() => useNormalizeTraceId(uppercaseTraceId));
50+
51+
expect(result.current).toBe(lowercaseTraceId);
52+
expect(mockNavigate).toHaveBeenCalledWith(`/trace/${lowercaseTraceId}${searchParams}`, {
53+
replace: true,
54+
state: null,
55+
});
56+
});
57+
58+
it('does not redirect when trace ID is already lowercase', () => {
59+
const lowercaseTraceId = 'abc123def456';
60+
61+
const { result } = renderHook(() => useNormalizeTraceId(lowercaseTraceId));
62+
63+
expect(result.current).toBe(lowercaseTraceId);
64+
expect(mockNavigate).not.toHaveBeenCalled();
65+
});
66+
});
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright (c) 2026 The Jaeger Authors.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import * as React from 'react';
5+
import { useNavigate, useLocation } from 'react-router-dom-v5-compat';
6+
7+
import { getUrl } from './url';
8+
9+
// Custom hook that normalizes a trace ID to lowercase in the URL
10+
export function useNormalizeTraceId(traceID: string): string {
11+
const normalizedTraceID = traceID.toLowerCase();
12+
const navigate = useNavigate();
13+
const location = useLocation();
14+
15+
React.useEffect(() => {
16+
if (traceID && traceID !== normalizedTraceID) {
17+
const url = getUrl(normalizedTraceID);
18+
navigate(`${url}${location.search}`, { replace: true, state: location.state });
19+
}
20+
// eslint-disable-next-line react-hooks/exhaustive-deps
21+
}, [traceID, normalizedTraceID]);
22+
23+
return normalizedTraceID;
24+
}

0 commit comments

Comments
 (0)