Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loki: Filter by labels based on the type of label (structured, indexed, parsed) #78595

Merged
merged 20 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from 10 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
21 changes: 20 additions & 1 deletion packages/grafana-data/src/types/datasource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,10 +582,29 @@ export interface QueryFix {

export type QueryFixType = 'ADD_FILTER' | 'ADD_FILTER_OUT' | 'ADD_STRING_FILTER' | 'ADD_STRING_FILTER_OUT';
export interface QueryFixAction {
type: QueryFixType | string;
query?: string;
preventSubmit?: boolean;
/**
* The type of action to perform. Will be passed to the data source to handle.
*
* @type {(QueryFixType | string)}
* @memberof QueryFixAction
*/
svennergr marked this conversation as resolved.
Show resolved Hide resolved
type: QueryFixType | string;
/**
* A key value map of options that will be passed. Usually used to pass e.g. the label and value.
*
* @type {KeyValue<string>}
* @memberof QueryFixAction
*/
options?: KeyValue<string>;
/**
* An optional single row data frame containing the row that triggered the the QueryFixAction.
*
* @type {DataFrame}
* @memberof QueryFixAction
*/
frame?: DataFrame;
svennergr marked this conversation as resolved.
Show resolved Hide resolved
}

export interface QueryHint {
Expand Down
1 change: 1 addition & 0 deletions packages/grafana-data/src/types/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ export interface QueryFilterOptions extends KeyValue<string> {}
export interface ToggleFilterAction {
type: 'FILTER_FOR' | 'FILTER_OUT';
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR but just a note - would be nice if we could type type: 'FILTER_FOR' | 'FILTER_OUT'; using QueryFixType somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, not sure about a cleaner way here. @matyax maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe for 11 we can clean up all these types, and even rename some to reflect, for example, if you're adding a "field" (or label) filter or a string filter.

options: QueryFilterOptions;
frame?: DataFrame;
}
/**
* Data sources that support toggleable filters through `toggleQueryFilter`, and displaying the active
Expand Down
24 changes: 20 additions & 4 deletions public/app/features/explore/Explore.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import AutoSizer from 'react-virtualized-auto-sizer';

import {
AbsoluteTimeRange,
DataFrame,
EventBus,
GrafanaTheme2,
hasToggleableQueryFiltersSupport,
Expand Down Expand Up @@ -219,15 +220,29 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
/**
* Used by Logs details.
*/
onClickFilterLabel = (key: string, value: string, refId?: string) => {
this.onModifyQueries({ type: 'ADD_FILTER', options: { key, value } }, refId);
onClickFilterLabel = (key: string, value: string, refId?: string, frame?: DataFrame) => {
svennergr marked this conversation as resolved.
Show resolved Hide resolved
this.onModifyQueries(
{
type: 'ADD_FILTER',
options: { key, value },
frame,
},
refId
);
};

/**
* Used by Logs details.
*/
onClickFilterOutLabel = (key: string, value: string, refId?: string) => {
this.onModifyQueries({ type: 'ADD_FILTER_OUT', options: { key, value } }, refId);
onClickFilterOutLabel = (key: string, value: string, refId?: string, frame?: DataFrame) => {
this.onModifyQueries(
{
type: 'ADD_FILTER_OUT',
options: { key, value },
frame,
},
refId
);
};

/**
Expand Down Expand Up @@ -269,6 +284,7 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
return ds.toggleQueryFilter(query, {
type: modification.type === 'ADD_FILTER' ? 'FILTER_FOR' : 'FILTER_OUT',
options: modification.options ?? {},
frame: modification.frame,
});
}
if (ds.modifyQuery) {
Expand Down
28 changes: 26 additions & 2 deletions public/app/features/logs/components/LogDetails.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,35 @@ describe('LogDetails', () => {

await userEvent.click(screen.getByLabelText('Filter for value in query A'));
expect(onClickFilterLabelMock).toHaveBeenCalledTimes(1);
expect(onClickFilterLabelMock).toHaveBeenCalledWith('key1', 'label1', mockRow.dataFrame.refId);
expect(onClickFilterLabelMock).toHaveBeenCalledWith(
'key1',
'label1',
mockRow.dataFrame.refId,
expect.objectContaining({
fields: [
expect.objectContaining({ values: [0] }),
expect.objectContaining({ values: ['line1'] }),
expect.objectContaining({ values: [{ app: 'app01' }] }),
],
length: 1,
})
);

await userEvent.click(screen.getByLabelText('Filter out value in query A'));
expect(onClickFilterOutLabelMock).toHaveBeenCalledTimes(1);
expect(onClickFilterOutLabelMock).toHaveBeenCalledWith('key1', 'label1', mockRow.dataFrame.refId);
expect(onClickFilterOutLabelMock).toHaveBeenCalledWith(
'key1',
'label1',
mockRow.dataFrame.refId,
expect.objectContaining({
fields: [
expect.objectContaining({ values: [0] }),
expect.objectContaining({ values: ['line1'] }),
expect.objectContaining({ values: [{ app: 'app01' }] }),
],
length: 1,
})
);
});
});
it('should not render filter controls when the callbacks are not provided', () => {
Expand Down
40 changes: 38 additions & 2 deletions public/app/features/logs/components/LogDetailsRow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ type Props = ComponentProps<typeof LogDetailsRow>;

const setup = (propOverrides?: Partial<Props>) => {
const props: Props = {
parsedValues: [''],
parsedKeys: [''],
parsedValues: ['value'],
parsedKeys: ['key'],
isLabel: true,
wrapLogMessage: false,
getStats: () => null,
Expand Down Expand Up @@ -66,6 +66,42 @@ describe('LogDetailsRow', () => {
});
expect(await screen.findByRole('button', { name: 'Remove filter in query A' })).toBeInTheDocument();
});
it('should trigger a call to `onClickFilterOutLabel` when the filter out button is clicked', () => {
const onClickFilterOutLabel = jest.fn();
setup({ onClickFilterOutLabel });
fireEvent.click(screen.getByRole('button', { name: 'Filter out value in query A' }));
expect(onClickFilterOutLabel).toHaveBeenCalledWith(
'key',
'value',
'A',
expect.objectContaining({
fields: [
expect.objectContaining({ values: [0] }),
expect.objectContaining({ values: ['line1'] }),
expect.objectContaining({ values: [{ app: 'app01' }] }),
],
length: 1,
})
);
});
it('should trigger a call to `onClickFilterLabel` when the filter button is clicked', () => {
const onClickFilterLabel = jest.fn();
setup({ onClickFilterLabel });
fireEvent.click(screen.getByRole('button', { name: 'Filter for value in query A' }));
expect(onClickFilterLabel).toHaveBeenCalledWith(
'key',
'value',
'A',
expect.objectContaining({
fields: [
expect.objectContaining({ values: [0] }),
expect.objectContaining({ values: ['line1'] }),
expect.objectContaining({ values: [{ app: 'app01' }] }),
],
length: 1,
})
);
});
});

describe('if props is not a label', () => {
Expand Down
21 changes: 16 additions & 5 deletions public/app/features/logs/components/LogDetailsRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ import { isEqual } from 'lodash';
import memoizeOne from 'memoize-one';
import React, { PureComponent, useState } from 'react';

import { CoreApp, Field, GrafanaTheme2, IconName, LinkModel, LogLabelStatsModel, LogRowModel } from '@grafana/data';
import {
CoreApp,
DataFrame,
Field,
GrafanaTheme2,
IconName,
LinkModel,
LogLabelStatsModel,
LogRowModel,
} from '@grafana/data';
import { reportInteraction } from '@grafana/runtime';
import { ClipboardButton, DataLinkButton, IconButton, Themeable2, withTheme2 } from '@grafana/ui';

import { logRowToDataFrame } from '../logsModel';

import { LogLabelStats } from './LogLabelStats';
import { getLogRowStyles } from './getLogRowStyles';

Expand All @@ -16,8 +27,8 @@ export interface Props extends Themeable2 {
disableActions: boolean;
wrapLogMessage?: boolean;
isLabel?: boolean;
onClickFilterLabel?: (key: string, value: string, refId?: string) => void;
onClickFilterOutLabel?: (key: string, value: string, refId?: string) => void;
onClickFilterLabel?: (key: string, value: string, refId?: string, frame?: DataFrame) => void;
onClickFilterOutLabel?: (key: string, value: string, refId?: string, frame?: DataFrame) => void;
links?: Array<LinkModel<Field>>;
getStats: () => LogLabelStatsModel[] | null;
displayedFields?: string[];
Expand Down Expand Up @@ -143,7 +154,7 @@ class UnThemedLogDetailsRow extends PureComponent<Props, State> {
filterLabel = () => {
const { onClickFilterLabel, parsedKeys, parsedValues, row } = this.props;
if (onClickFilterLabel) {
onClickFilterLabel(parsedKeys[0], parsedValues[0], row.dataFrame?.refId);
onClickFilterLabel(parsedKeys[0], parsedValues[0], row.dataFrame?.refId, logRowToDataFrame(row) || undefined);
}

reportInteraction('grafana_explore_logs_log_details_filter_clicked', {
Expand All @@ -156,7 +167,7 @@ class UnThemedLogDetailsRow extends PureComponent<Props, State> {
filterOutLabel = () => {
const { onClickFilterOutLabel, parsedKeys, parsedValues, row } = this.props;
if (onClickFilterOutLabel) {
onClickFilterOutLabel(parsedKeys[0], parsedValues[0], row.dataFrame?.refId);
onClickFilterOutLabel(parsedKeys[0], parsedValues[0], row.dataFrame?.refId, logRowToDataFrame(row) || undefined);
}

reportInteraction('grafana_explore_logs_log_details_filter_clicked', {
Expand Down
15 changes: 13 additions & 2 deletions public/app/features/logs/components/__mocks__/logRow.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LogLevel, LogRowModel, MutableDataFrame } from '@grafana/data';
import { FieldType, LogLevel, LogRowModel, toDataFrame } from '@grafana/data';

export const createLogRow = (overrides?: Partial<LogRowModel>): LogRowModel => {
const uid = overrides?.uid || '1';
Expand All @@ -8,7 +8,18 @@ export const createLogRow = (overrides?: Partial<LogRowModel>): LogRowModel => {
return {
entryFieldIndex: 0,
rowIndex: 0,
dataFrame: new MutableDataFrame({ refId: 'A', fields: [] }),
dataFrame: toDataFrame({
refId: 'A',
svennergr marked this conversation as resolved.
Show resolved Hide resolved
fields: [
{ name: 'Time', type: FieldType.time, values: [0, 1] },
{
name: 'Line',
type: FieldType.string,
values: ['line1', 'line2'],
},
{ name: 'labels', type: FieldType.other, values: [{ app: 'app01' }, { app: 'app02' }] },
],
}),
uid,
logLevel: LogLevel.info,
entry,
Expand Down
52 changes: 52 additions & 0 deletions public/app/features/logs/logsModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
filterLogLevels,
getSeriesProperties,
LIMIT_LABEL,
logRowToDataFrame,
logSeriesToLogsModel,
queryLogsSample,
queryLogsVolume,
Expand Down Expand Up @@ -1458,3 +1459,54 @@ describe('logs sample', () => {
});
});
});

const mockLogRow = {
dataFrame: toDataFrame({
fields: [
{ name: 'Time', type: FieldType.time, values: [0, 1] },
{
name: 'Line',
type: FieldType.string,
values: ['line1', 'line2'],
},
{ name: 'labels', type: FieldType.other, values: [{ app: 'app01' }, { app: 'app02' }] },
],
}),
rowIndex: 0,
} as unknown as LogRowModel;

describe('logRowToDataFrame', () => {
it('should return a DataFrame with the values from the specified row', () => {
const result = logRowToDataFrame(mockLogRow);

expect(result?.length).toBe(1);

expect(result?.fields[0].values[0]).toEqual(0);
expect(result?.fields[1].values[0]).toEqual('line1');
expect(result?.fields[2].values[0]).toEqual({ app: 'app01' });
});

it('should return a DataFrame with the values from the specified different row', () => {
const result = logRowToDataFrame({ ...mockLogRow, rowIndex: 1 });

expect(result?.length).toBe(1);

expect(result?.fields[0].values[0]).toEqual(1);
expect(result?.fields[1].values[0]).toEqual('line2');
expect(result?.fields[2].values[0]).toEqual({ app: 'app02' });
});

it('should handle an empty DataFrame', () => {
const emptyLogRow = { dataFrame: { fields: [] }, rowIndex: 0 } as unknown as LogRowModel;
const result = logRowToDataFrame(emptyLogRow);

expect(result?.length).toBe(0);
});

it('should handle rowIndex exceeding array bounds', () => {
const invalidRowIndex = 10;
const result = logRowToDataFrame({ ...mockLogRow, rowIndex: invalidRowIndex });

expect(result).toBe(null);
});
});
22 changes: 22 additions & 0 deletions public/app/features/logs/logsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { from, isObservable, Observable } from 'rxjs';

import {
AbsoluteTimeRange,
createDataFrame,
DataFrame,
DataQuery,
DataQueryRequest,
Expand Down Expand Up @@ -789,3 +790,24 @@ function getIntervalInfo(scopedVars: ScopedVars, timespanMs: number): { interval
return { interval: '$__interval' };
}
}

/**
* Converts log row into data frame with a single row.
*
* @export
* @param {LogRowModel} logRow
* @return {DataFrame}
svennergr marked this conversation as resolved.
Show resolved Hide resolved
*/
export function logRowToDataFrame(logRow: LogRowModel): DataFrame | null {
svennergr marked this conversation as resolved.
Show resolved Hide resolved
const originFrame = logRow.dataFrame;

if (originFrame.length === 0 || originFrame.length <= logRow.rowIndex) {
return null;
}

const frame = createDataFrame({
svennergr marked this conversation as resolved.
Show resolved Hide resolved
fields: originFrame.fields.map((field) => ({ ...field, values: [field.values[logRow.rowIndex]] })),
});

return frame;
}
Loading
Loading