-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(insights): Database system selector in Queries module (#75942)
Adds a new dropdown selector to the Queries module that is responsible for changing the view depending on the currently selected database system, While the component itself is simple, there is a lot of logic happening behind the scenes that is difficult to follow, here it is broken down; - Queries the current project's spans to finds all database systems being used, and populates the dropdown selector options with compatible systems, in descending order based on number of spans per system - defaults to choosing the first option in the list (the system with the most spans) - Component is disabled when only one database system is being used (consistent with our design philosophy to not hide information / components) - The most recent option that was selected is saved to `localStorage`, so next time you view the page it will maintain your selection - The currently selected system is saved in the URL's query params - Following a URL with a non-default system selected will maintain this selection, but will not override your selection that is saved to your localStorage **TODO in followup PR** - [ ] Update the view according to which system was selected (mongodb terminology and formatting will be different from SQL) ![image](https://github.com/user-attachments/assets/25c1dfe5-b98c-4be1-b04a-e1a4a219cee7)
- Loading branch information
Showing
5 changed files
with
354 additions
and
1 deletion.
There are no files selected for viewing
234 changes: 234 additions & 0 deletions
234
static/app/views/insights/database/components/databaseSystemSelector.spec.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,234 @@ | ||
import {OrganizationFixture} from 'sentry-fixture/organization'; | ||
|
||
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; | ||
|
||
import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; | ||
import {useLocation} from 'sentry/utils/useLocation'; | ||
import {useNavigate} from 'sentry/utils/useNavigate'; | ||
import {useSpanMetrics} from 'sentry/views/insights/common/queries/useDiscover'; | ||
import {DatabaseSystemSelector} from 'sentry/views/insights/database/components/databaseSystemSelector'; | ||
import {SpanMetricsField} from 'sentry/views/insights/types'; | ||
|
||
jest.mock('sentry/views/insights/common/queries/useDiscover', () => ({ | ||
useSpanMetrics: jest.fn(), | ||
})); | ||
|
||
jest.mock('sentry/utils/useLocalStorageState', () => ({ | ||
useLocalStorageState: jest.fn(), | ||
})); | ||
|
||
jest.mock('sentry/utils/useLocation', () => ({ | ||
useLocation: jest.fn(), | ||
})); | ||
|
||
jest.mock('sentry/utils/useNavigate', () => ({ | ||
useNavigate: jest.fn(), | ||
})); | ||
|
||
const mockUseLocalStorageState = jest.mocked(useLocalStorageState); | ||
const mockUseSpanMetrics = jest.mocked(useSpanMetrics); | ||
const mockUseLocation = jest.mocked(useLocation); | ||
const mockUseNavigate = jest.mocked(useNavigate); | ||
|
||
describe('DatabaseSystemSelector', function () { | ||
const organization = OrganizationFixture(); | ||
|
||
afterAll(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
beforeEach(() => { | ||
mockUseLocation.mockReturnValue({ | ||
query: {project: ['1']}, | ||
pathname: '', | ||
search: '', | ||
hash: '', | ||
state: undefined, | ||
action: 'POP', | ||
key: '', | ||
}); | ||
}); | ||
|
||
it('is disabled and does not select a system if there are none available', async function () { | ||
const mockSetState = jest.fn(); | ||
mockUseLocalStorageState.mockReturnValue(['', mockSetState]); | ||
mockUseSpanMetrics.mockReturnValue({ | ||
data: [], | ||
isLoading: false, | ||
isError: false, | ||
} as any); | ||
|
||
render(<DatabaseSystemSelector />, {organization}); | ||
|
||
expect(mockSetState).not.toHaveBeenCalled(); | ||
const dropdownButton = await screen.findByRole('button'); | ||
expect(dropdownButton).toBeInTheDocument(); | ||
expect(dropdownButton).toHaveTextContent('DB SystemNone'); | ||
}); | ||
|
||
it('is disabled when only one database system is present and shows that system as selected', async function () { | ||
const mockSetState = jest.fn(); | ||
mockUseLocalStorageState.mockReturnValue(['', mockSetState]); | ||
mockUseSpanMetrics.mockReturnValue({ | ||
data: [ | ||
{ | ||
'span.system': 'postgresql', | ||
'count()': 1000, | ||
}, | ||
], | ||
isLoading: false, | ||
isError: false, | ||
} as any); | ||
|
||
render(<DatabaseSystemSelector />, {organization}); | ||
|
||
const dropdownSelector = await screen.findByRole('button'); | ||
expect(dropdownSelector).toBeDisabled(); | ||
expect(mockSetState).toHaveBeenCalledWith('postgresql'); | ||
}); | ||
|
||
it('renders all database system options correctly', async function () { | ||
mockUseSpanMetrics.mockReturnValue({ | ||
data: [ | ||
{ | ||
'span.system': 'postgresql', | ||
'count()': 1000, | ||
}, | ||
{ | ||
'span.system': 'mongodb', | ||
'count()': 500, | ||
}, | ||
{ | ||
'span.system': 'chungusdb', | ||
'count()': 200, | ||
}, | ||
], | ||
isLoading: false, | ||
isError: false, | ||
} as any); | ||
|
||
render(<DatabaseSystemSelector />, {organization}); | ||
|
||
const dropdownSelector = await screen.findByRole('button'); | ||
expect(dropdownSelector).toBeEnabled(); | ||
expect(mockUseSpanMetrics).toHaveBeenCalled(); | ||
|
||
const dropdownButton = await screen.findByRole('button'); | ||
expect(dropdownButton).toBeInTheDocument(); | ||
|
||
await userEvent.click(dropdownButton); | ||
|
||
const dropdownOptionLabels = await screen.findAllByTestId('menu-list-item-label'); | ||
expect(dropdownOptionLabels[0]).toHaveTextContent('PostgreSQL'); | ||
expect(dropdownOptionLabels[1]).toHaveTextContent('MongoDB'); | ||
// chungusdb does not exist, so we do not expect this option to have casing | ||
expect(dropdownOptionLabels[2]).toHaveTextContent('chungusdb'); | ||
}); | ||
|
||
it('chooses the currently selected system from localStorage', async function () { | ||
mockUseLocalStorageState.mockReturnValue(['mongodb', () => {}]); | ||
mockUseSpanMetrics.mockReturnValue({ | ||
data: [ | ||
{ | ||
'span.system': 'postgresql', | ||
'count()': 1000, | ||
}, | ||
{ | ||
'span.system': 'mongodb', | ||
'count()': 500, | ||
}, | ||
{ | ||
'span.system': 'chungusdb', | ||
'count()': 200, | ||
}, | ||
], | ||
isLoading: false, | ||
isError: false, | ||
} as any); | ||
|
||
render(<DatabaseSystemSelector />, {organization}); | ||
|
||
expect(await screen.findByText('MongoDB')).toBeInTheDocument(); | ||
}); | ||
|
||
it('does not set the value from localStorage if the value is invalid', async function () { | ||
const mockSetState = jest.fn(); | ||
mockUseLocalStorageState.mockReturnValue(['chungusdb', mockSetState]); | ||
mockUseSpanMetrics.mockReturnValue({ | ||
data: [ | ||
{ | ||
'span.system': 'postgresql', | ||
'count()': 1000, | ||
}, | ||
], | ||
isLoading: false, | ||
isError: false, | ||
} as any); | ||
|
||
render(<DatabaseSystemSelector />, {organization}); | ||
|
||
const dropdownSelector = await screen.findByRole('button'); | ||
expect(dropdownSelector).toBeInTheDocument(); | ||
expect(mockSetState).not.toHaveBeenCalledWith('chungusdb'); | ||
}); | ||
|
||
it('prioritizes the system set in query parameters but does not replace localStorage value until an option is clicked', async function () { | ||
const {SPAN_SYSTEM} = SpanMetricsField; | ||
const mockNavigate = jest.fn(); | ||
mockUseNavigate.mockReturnValue(mockNavigate); | ||
|
||
mockUseLocation.mockReturnValue({ | ||
query: {project: ['1'], [SPAN_SYSTEM]: 'mongodb'}, | ||
pathname: '', | ||
search: '', | ||
hash: '', | ||
state: undefined, | ||
action: 'POP', | ||
key: '', | ||
}); | ||
|
||
mockUseSpanMetrics.mockReturnValue({ | ||
data: [ | ||
{ | ||
'span.system': 'postgresql', | ||
'count()': 1000, | ||
}, | ||
{ | ||
'span.system': 'mongodb', | ||
'count()': 500, | ||
}, | ||
], | ||
isLoading: false, | ||
isError: false, | ||
} as any); | ||
|
||
const mockSetState = jest.fn(); | ||
mockUseLocalStorageState.mockReturnValue(['postgresql', mockSetState]); | ||
|
||
render(<DatabaseSystemSelector />, {organization}); | ||
|
||
const dropdownSelector = await screen.findByRole('button'); | ||
expect(dropdownSelector).toHaveTextContent('DB SystemMongoDB'); | ||
expect(mockSetState).not.toHaveBeenCalledWith('mongodb'); | ||
|
||
// Now that it has been confirmed that following a URL does not reset localStorage state, confirm that | ||
// clicking a different option will update both the state and the URL | ||
await userEvent.click(dropdownSelector); | ||
const dropdownOptionLabels = await screen.findAllByTestId('menu-list-item-label'); | ||
expect(dropdownOptionLabels[0]).toHaveTextContent('PostgreSQL'); | ||
expect(dropdownOptionLabels[1]).toHaveTextContent('MongoDB'); | ||
|
||
await userEvent.click(dropdownOptionLabels[0]); | ||
expect(dropdownSelector).toHaveTextContent('DB SystemPostgreSQL'); | ||
expect(mockSetState).toHaveBeenCalledWith('postgresql'); | ||
expect(mockNavigate).toHaveBeenCalledWith({ | ||
action: 'POP', | ||
hash: '', | ||
key: '', | ||
pathname: '', | ||
query: {project: ['1'], 'span.system': 'postgresql'}, | ||
search: '', | ||
state: undefined, | ||
}); | ||
}); | ||
}); |
33 changes: 33 additions & 0 deletions
33
static/app/views/insights/database/components/databaseSystemSelector.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import {CompactSelect} from 'sentry/components/compactSelect'; | ||
import {t} from 'sentry/locale'; | ||
import {decodeScalar} from 'sentry/utils/queryString'; | ||
import {useLocation} from 'sentry/utils/useLocation'; | ||
import {useNavigate} from 'sentry/utils/useNavigate'; | ||
import {useSystemSelectorOptions} from 'sentry/views/insights/database/components/useSystemSelectorOptions'; | ||
import {SpanMetricsField} from 'sentry/views/insights/types'; | ||
|
||
const {SPAN_SYSTEM} = SpanMetricsField; | ||
|
||
export function DatabaseSystemSelector() { | ||
const location = useLocation(); | ||
const navigate = useNavigate(); | ||
|
||
// If there is no query parameter for the system, retrieve the current value from the hook instead | ||
const systemQueryParam = decodeScalar(location.query?.[SPAN_SYSTEM]); | ||
const {selectedSystem, setSelectedSystem, options, isLoading, isError} = | ||
useSystemSelectorOptions(); | ||
|
||
return ( | ||
<CompactSelect | ||
onChange={option => { | ||
setSelectedSystem(option.value); | ||
navigate({...location, query: {...location.query, [SPAN_SYSTEM]: option.value}}); | ||
}} | ||
options={options} | ||
triggerProps={{prefix: t('DB System')}} | ||
loading={isLoading} | ||
disabled={isError || isLoading || options.length <= 1} | ||
value={systemQueryParam ?? selectedSystem} | ||
/> | ||
); | ||
} |
71 changes: 71 additions & 0 deletions
71
static/app/views/insights/database/components/useSystemSelectorOptions.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
import type {SelectOption} from 'sentry/components/compactSelect'; | ||
import {MutableSearch} from 'sentry/utils/tokenizeSearch'; | ||
import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; | ||
import {useSpanMetrics} from 'sentry/views/insights/common/queries/useDiscover'; | ||
import {SpanMetricsField} from 'sentry/views/insights/types'; | ||
|
||
/** | ||
* The supported relational database system values are based on what is | ||
* set in the Sentry Python SDK. The only currently supported NoSQL DBMS is MongoDB. | ||
* | ||
* https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/integrations/sqlalchemy.py#L125 | ||
*/ | ||
enum SupportedDatabaseSystems { | ||
// SQL | ||
SQLITE = 'sqlite', | ||
POSTGRESQL = 'postgresql', | ||
MARIADB = 'mariadb', | ||
MYSQL = 'mysql', | ||
ORACLE = 'oracle', | ||
// NoSQL | ||
MONGODB = 'mongodb', | ||
} | ||
|
||
const DATABASE_SYSTEM_TO_LABEL: Record<SupportedDatabaseSystems, string> = { | ||
[SupportedDatabaseSystems.SQLITE]: 'SQLite', | ||
[SupportedDatabaseSystems.POSTGRESQL]: 'PostgreSQL', | ||
[SupportedDatabaseSystems.MARIADB]: 'MariaDB', | ||
[SupportedDatabaseSystems.MYSQL]: 'MySQL', | ||
[SupportedDatabaseSystems.ORACLE]: 'Oracle', | ||
[SupportedDatabaseSystems.MONGODB]: 'MongoDB', | ||
}; | ||
|
||
export function useSystemSelectorOptions() { | ||
const [selectedSystem, setSelectedSystem] = useLocalStorageState<string>( | ||
'insights-db-system-selector', | ||
'' | ||
); | ||
|
||
const {data, isLoading, isError} = useSpanMetrics( | ||
{ | ||
search: MutableSearch.fromQueryObject({'span.op': 'db'}), | ||
|
||
fields: [SpanMetricsField.SPAN_SYSTEM, 'count()'], | ||
sorts: [{field: 'count()', kind: 'desc'}], | ||
}, | ||
'api.starfish.database-system-selector' | ||
); | ||
|
||
const options: SelectOption<string>[] = []; | ||
data.forEach(entry => { | ||
const system = entry['span.system']; | ||
if (system) { | ||
const label: string = | ||
system in DATABASE_SYSTEM_TO_LABEL ? DATABASE_SYSTEM_TO_LABEL[system] : system; | ||
|
||
options.push({value: system, label, textValue: label}); | ||
} | ||
}); | ||
|
||
// Edge case: Invalid DB system was retrieved from localStorage | ||
if (!options.find(option => selectedSystem === option.value) && options.length > 0) { | ||
setSelectedSystem(options[0].value); | ||
} | ||
|
||
// Edge case: No current system is saved in localStorage | ||
if (!selectedSystem && options.length > 0) { | ||
setSelectedSystem(options[0].value); | ||
} | ||
|
||
return {selectedSystem, setSelectedSystem, options, isLoading, isError}; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.