Skip to content
Merged
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
6 changes: 4 additions & 2 deletions static/app/views/explore/contexts/logs/logsPageParams.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
} from 'sentry/views/explore/contexts/logs/sortBys';
import {
getModeFromLocation,
updateLocationWithMode,
type Mode,
} from 'sentry/views/explore/contexts/pageParamsContext/mode';
import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types';
Expand Down Expand Up @@ -113,7 +114,7 @@ interface LogsPageParams {
type NullablePartial<T> = {
[P in keyof T]?: T[P] | null;
};
type NonUpdatableParams = 'mode' | 'aggregateFn' | 'aggregateParam' | 'groupBy';
type NonUpdatableParams = 'aggregateFn' | 'aggregateParam' | 'groupBy';
type LogPageParamsUpdate = NullablePartial<Omit<LogsPageParams, NonUpdatableParams>>;

const [_LogsPageParamsProvider, _useLogsPageParams, LogsPageParamsContext] =
Expand Down Expand Up @@ -248,12 +249,13 @@ const decodeLogsQuery = (location: Location): string => {
return decodeScalar(queryParameter, '').trim();
};

function setLogsPageParams(location: Location, pageParams: LogPageParamsUpdate) {
export function setLogsPageParams(location: Location, pageParams: LogPageParamsUpdate) {
const target: Location = {...location, query: {...location.query}};
updateNullableLocation(target, LOGS_QUERY_KEY, pageParams.search?.formatString());
updateNullableLocation(target, LOGS_CURSOR_KEY, pageParams.cursor);
updateNullableLocation(target, LOGS_AGGREGATE_CURSOR_KEY, pageParams.aggregateCursor);
updateNullableLocation(target, LOGS_FIELDS_KEY, pageParams.fields);
updateLocationWithMode(target, pageParams.mode); // Can be swapped with updateNullableLocation if we merge page params.
if (!pageParams.isTableFrozen) {
updateLocationWithLogSortBys(target, pageParams.sortBys);
updateLocationWithAggregateSortBys(target, pageParams.aggregateSortBys);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ describe('LogsAggregateTable', () => {
];
rows.forEach((row, i) => {
const cells = within(row).getAllByTestId('grid-body-cell');
expect(cells[0]).toHaveTextContent(expected[i]![0]!);
expect(cells[1]).toHaveTextContent(expected[i]![1]!);
expect(cells[1]).toHaveTextContent(expected[i]![0]!);
expect(cells[2]).toHaveTextContent(expected[i]![1]!);
});
});
});
76 changes: 70 additions & 6 deletions static/app/views/explore/logs/tables/logsAggregateTable.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,37 @@
import {Fragment} from 'react';
import {useTheme} from '@emotion/react';
import styled from '@emotion/styled';

import {Link} from 'sentry/components/core/link';
import {Tooltip} from 'sentry/components/core/tooltip';
import Pagination from 'sentry/components/pagination';
import GridEditable, {COL_WIDTH_UNDEFINED} from 'sentry/components/tables/gridEditable';
import SortLink from 'sentry/components/tables/gridEditable/sortLink';
import {IconStack} from 'sentry/icons/iconStack';
import {t} from 'sentry/locale';
import {defined} from 'sentry/utils';
import {parseFunction, prettifyParsedFunction} from 'sentry/utils/discover/fields';
import {prettifyTagKey} from 'sentry/utils/fields';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import useProjects from 'sentry/utils/useProjects';
import {
LOGS_AGGREGATE_CURSOR_KEY,
useLogsFields,
useLogsSearch,
useLogsSortBys,
useSetLogsPageParams,
} from 'sentry/views/explore/contexts/logs/logsPageParams';
import {LOGS_AGGREGATE_SORT_BYS_KEY} from 'sentry/views/explore/contexts/logs/sortBys';
import type {RendererExtra} from 'sentry/views/explore/logs/fieldRenderers';
import {LogFieldRenderer} from 'sentry/views/explore/logs/fieldRenderers';
import {getLogColors} from 'sentry/views/explore/logs/styles';
import {useLogsAggregatesQuery} from 'sentry/views/explore/logs/useLogsQuery';
import {SeverityLevel} from 'sentry/views/explore/logs/utils';
import {SeverityLevel, viewLogsSamplesTarget} from 'sentry/views/explore/logs/utils';
import {
useQueryParamsAggregateSortBys,
useQueryParamsGroupBys,
useQueryParamsTopEventsLimit,
useQueryParamsVisualizes,
} from 'sentry/views/explore/queryParams/context';

Expand All @@ -34,13 +44,22 @@ export function LogsAggregateTable() {
const groupBys = useQueryParamsGroupBys();
const visualizes = useQueryParamsVisualizes();
const aggregateSortBys = useQueryParamsAggregateSortBys();
const topEventsLimit = useQueryParamsTopEventsLimit();
const search = useLogsSearch();
const fields = useLogsFields();
const sorts = useLogsSortBys();
const location = useLocation();
const theme = useTheme();
const organization = useOrganization();
const {projects} = useProjects();

const fields: string[] = [];
fields.push(...groupBys.filter(Boolean));
fields.push(...visualizes.map(visualize => visualize.yAxis));
const allFields: string[] = [];
allFields.push(...groupBys.filter(Boolean));
allFields.push(...visualizes.map(visualize => visualize.yAxis));

const numberOfRowsNeedingColor = Math.min(data?.data?.length ?? 0, topEventsLimit ?? 0);

const palette = theme.chart.getColorPalette(numberOfRowsNeedingColor - 1);

return (
<TableContainer>
Expand All @@ -49,14 +68,14 @@ export function LogsAggregateTable() {
isLoading={isLoading}
error={error}
data={data?.data ?? []}
columnOrder={fields.map(field => ({
columnOrder={allFields.map(field => ({
key: field,
name: field,
width: COL_WIDTH_UNDEFINED,
}))}
columnSortBy={[
{
key: fields[0]!,
key: allFields[0]!,
order: 'desc',
},
]}
Expand Down Expand Up @@ -125,6 +144,37 @@ export function LogsAggregateTable() {
/>
);
},
prependColumnWidths: ['40px'],
renderPrependColumns: (isHeader, dataRow, rowIndex) => {
// rowIndex is only defined when `isHeader=false`
if (isHeader || !defined(rowIndex)) {
return [<span key="header-icon" />];
}

const target = viewLogsSamplesTarget({
location,
search,
fields,
groupBys,
visualizes,
sorts,
row: dataRow || {},
projects,
});

return [
<Fragment key={`sample-${rowIndex}`}>
{topEventsLimit && rowIndex < topEventsLimit && (
<TopResultsIndicator color={palette[rowIndex]!} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Palette Initialization Causes Out-of-Bounds Errors

The palette is initialized with numberOfRowsNeedingColor - 1 elements. This can lead to theme.chart.getColorPalette being called with a negative argument (-1) when numberOfRowsNeedingColor is 0. Additionally, when numberOfRowsNeedingColor equals topEventsLimit, the palette is one element too small, causing an out-of-bounds access when palette[rowIndex]! is used for the last rowIndex.

Fix in Cursor Fix in Web

)}
<Tooltip title={t('View Samples')} containerDisplayMode="flex">
<StyledLink to={target}>
<IconStack />
</StyledLink>
</Tooltip>
</Fragment>,
];
},
}}
/>
<Pagination
Expand All @@ -139,3 +189,17 @@ const TableContainer = styled('div')`
display: flex;
flex-direction: column;
`;

const TopResultsIndicator = styled('div')<{color: string}>`
position: absolute;
left: -1px;
width: 9px;
height: 16px;
border-radius: 0 3px 3px 0;

background-color: ${p => p.color};
`;

const StyledLink = styled(Link)`
display: flex;
`;
53 changes: 51 additions & 2 deletions static/app/views/explore/logs/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import type {ReactNode} from 'react';
import * as Sentry from '@sentry/react';
import type {Location} from 'history';
import * as qs from 'query-string';

import type {ApiResult} from 'sentry/api';
import {t} from 'sentry/locale';
import type {PageFilters} from 'sentry/types/core';
import type {TagCollection} from 'sentry/types/group';
import type {Organization} from 'sentry/types/organization';
import type {Project} from 'sentry/types/project';
import {defined} from 'sentry/utils';
import type {EventsMetaType} from 'sentry/utils/discover/eventView';
import {
Expand All @@ -26,6 +28,7 @@ import {
LOGS_FIELDS_KEY,
LOGS_GROUP_BY_KEY,
LOGS_QUERY_KEY,
setLogsPageParams,
} from 'sentry/views/explore/contexts/logs/logsPageParams';
import {LOGS_SORT_BYS_KEY} from 'sentry/views/explore/contexts/logs/sortBys';
import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
Expand All @@ -49,8 +52,11 @@ import {
type OurLogsResponseItem,
} from 'sentry/views/explore/logs/types';
import type {GroupBy} from 'sentry/views/explore/queryParams/groupBy';
import type {BaseVisualize} from 'sentry/views/explore/queryParams/visualize';
import type {PickableDays} from 'sentry/views/explore/utils';
import {
type BaseVisualize,
type Visualize,
} from 'sentry/views/explore/queryParams/visualize';
import {generateTargetQuery, type PickableDays} from 'sentry/views/explore/utils';
import type {useSortedTimeSeries} from 'sentry/views/insights/common/queries/useSortedTimeSeries';

const {warn, fmt} = Sentry.logger;
Expand Down Expand Up @@ -503,6 +509,49 @@ export function ourlogToJson(ourlog: TraceItemDetailsResponse | undefined): stri
return JSON.stringify(copy, null, 2);
}

export function viewLogsSamplesTarget({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe reuse getLogsUrl here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll think about how to consolidate more of these ways to link to logs

location,
search,
fields,
groupBys,
visualizes,
sorts,
row,
projects,
}: {
fields: string[];
groupBys: readonly string[];
location: Location;
// needed to generate targets when `project` is in the group by
projects: Project[];
row: Record<string, any>;
search: MutableSearch;
sorts: Sort[];
visualizes: readonly Visualize[];
}) {
const {
fields: newFields,
search: newSearch,
sortBys: newSortBys,
} = generateTargetQuery({
fields,
groupBys: groupBys.slice(),
location,
projects,
search,
row,
sorts,
yAxes: visualizes.map(visualize => visualize.yAxis),
});

return setLogsPageParams(location, {
mode: Mode.SAMPLES,
fields: newFields,
search: newSearch,
sortBys: newSortBys,
});
}

export const logOnceFactory = (logSeverity: 'info' | 'warn') => {
let fired = false;
return (...args: Parameters<(typeof Sentry.logger)[typeof logSeverity]>) => {
Expand Down
70 changes: 57 additions & 13 deletions static/app/views/explore/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,27 +256,27 @@ export function combineConfidenceForSeries(
return 'high';
}

export function viewSamplesTarget({
location,
query,
export function generateTargetQuery({
fields,
groupBys,
visualizes,
sorts,
row,
location,
projects,
search,
row,
sorts,
yAxes,
}: {
fields: string[];
groupBys: string[];
location: Location;
// needed to generate targets when `project` is in the group by
projects: Project[];
query: string;
row: Record<string, any>;
search: MutableSearch;
sorts: Sort[];
visualizes: Visualize[];
yAxes: string[];
}) {
const search = new MutableSearch(query);
search = search.copy();

// first update the resulting query to filter for the target group
for (const groupBy of groupBys) {
Expand All @@ -300,8 +300,8 @@ export function viewSamplesTarget({
const seenFields = new Set(newFields);

// add all the arguments of the visualizations as columns
for (const visualize of visualizes) {
const parsedFunction = parseFunction(visualize.yAxis);
for (const yAxis of yAxes) {
const parsedFunction = parseFunction(yAxis);
if (!parsedFunction?.arguments[0]) {
continue;
}
Expand Down Expand Up @@ -347,11 +347,55 @@ export function viewSamplesTarget({
break;
}

return {
fields: newFields,
search,
sortBys: [sortBy],
};
}

export function viewSamplesTarget({
location,
query,
fields,
groupBys,
visualizes,
sorts,
row,
projects,
}: {
fields: string[];
groupBys: string[];
location: Location;
// needed to generate targets when `project` is in the group by
projects: Project[];
query: string;
row: Record<string, any>;
sorts: Sort[];
visualizes: Visualize[];
}) {
const search = new MutableSearch(query);

const {
fields: newFields,
search: newSearch,
sortBys: newSortBys,
} = generateTargetQuery({
fields,
groupBys,
location,
projects,
search,
row,
sorts,
yAxes: visualizes.map(visualize => visualize.yAxis),
});

return newExploreTarget(location, {
mode: Mode.SAMPLES,
fields: newFields,
query: search.formatString(),
sampleSortBys: [sortBy],
query: newSearch.formatString(),
sampleSortBys: newSortBys,
});
}

Expand Down
Loading