From 170a0df194e8c28bd986a09b979d969af55ba354 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Thu, 23 Apr 2020 20:47:54 +0200 Subject: [PATCH] Logs: Derived fields link design update (#23695) --- .../src/components/Logs/LogDetails.tsx | 14 +++- .../src/components/Logs/LogDetailsRow.tsx | 62 +++++++++++------- .../grafana-ui/src/components/Tags/Tag.tsx | 21 ++++-- packages/grafana-ui/src/utils/tags.ts | 9 ++- .../app/features/explore/utils/links.test.ts | 16 ++++- public/app/features/explore/utils/links.ts | 26 ++++++++ .../loki/result_transformer.test.ts | 13 +++- .../datasource/loki/result_transformer.ts | 65 +++++++++++-------- 8 files changed, 159 insertions(+), 67 deletions(-) diff --git a/packages/grafana-ui/src/components/Logs/LogDetails.tsx b/packages/grafana-ui/src/components/Logs/LogDetails.tsx index ac875a71ba0..56632c54746 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetails.tsx +++ b/packages/grafana-ui/src/components/Logs/LogDetails.tsx @@ -121,7 +121,19 @@ class UnThemedLogDetails extends PureComponent { } return acc; }, {} as { [key: string]: FieldDef }); - return Object.values(fieldsMap); + const allFields = Object.values(fieldsMap); + allFields.sort((fieldA, fieldB) => { + if (fieldA.links?.length && !fieldB.links?.length) { + return -1; + } + + if (!fieldA.links?.length && fieldB.links?.length) { + return 1; + } + + return fieldA.key > fieldB.key ? 1 : fieldA.key < fieldB.key ? -1 : 0; + }); + return allFields; }); getStatsForParsedField = (key: string) => { diff --git a/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx b/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx index babaf781eed..bcc5836019b 100644 --- a/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx +++ b/packages/grafana-ui/src/components/Logs/LogDetailsRow.tsx @@ -9,8 +9,8 @@ import { stylesFactory } from '../../themes/stylesFactory'; //Components import { LogLabelStats } from './LogLabelStats'; -import { LinkButton } from '../Button/Button'; import { IconButton } from '../IconButton/IconButton'; +import { Tag } from '..'; export interface Props extends Themeable { parsedValue: string; @@ -116,27 +116,10 @@ class UnThemedLogDetailsRow extends PureComponent { {links && links.map(link => { return ( - - <> -   - { - if (!(event.ctrlKey || event.metaKey || event.shiftKey) && link.onClick) { - event.preventDefault(); - link.onClick(event); - } - }) - } - /> - - + <> +   + + ); })} {showFieldsStats && ( @@ -154,5 +137,40 @@ class UnThemedLogDetailsRow extends PureComponent { } } +const getLinkStyles = stylesFactory(() => { + return { + tag: css` + margin-left: 6px; + font-size: 11px; + padding: 2px 6px; + `, + }; +}); + +type FieldLinkProps = { + link: LinkModel; +}; +function FieldLink({ link }: FieldLinkProps) { + const styles = getLinkStyles(); + return ( + { + if (!(event.ctrlKey || event.metaKey || event.shiftKey) && link.onClick) { + event.preventDefault(); + link.onClick(event); + } + } + : undefined + } + > + + + ); +} + export const LogDetailsRow = withTheme(UnThemedLogDetailsRow); LogDetailsRow.displayName = 'LogDetailsRow'; diff --git a/packages/grafana-ui/src/components/Tags/Tag.tsx b/packages/grafana-ui/src/components/Tags/Tag.tsx index dc8fa636565..e2f3581d1cc 100644 --- a/packages/grafana-ui/src/components/Tags/Tag.tsx +++ b/packages/grafana-ui/src/components/Tags/Tag.tsx @@ -2,19 +2,21 @@ import React, { forwardRef, HTMLAttributes } from 'react'; import { cx, css } from 'emotion'; import { GrafanaTheme } from '@grafana/data'; import { useTheme } from '../../themes'; -import { getTagColorsFromName } from '../../utils'; +import { getTagColor, getTagColorsFromName } from '../../utils'; export type OnTagClick = (name: string, event: React.MouseEvent) => any; export interface Props extends Omit, 'onClick'> { /** Name of the tag to display */ name: string; + /** Use constant color from TAG_COLORS. Using index instead of color directly so we can match other styling. */ + colorIndex?: number; onClick?: OnTagClick; } -export const Tag = forwardRef(({ name, onClick, className, ...rest }, ref) => { +export const Tag = forwardRef(({ name, onClick, className, colorIndex, ...rest }, ref) => { const theme = useTheme(); - const styles = getTagStyles(theme, name); + const styles = getTagStyles(theme, name, colorIndex); const onTagClick = (event: React.MouseEvent) => { if (onClick) { @@ -29,20 +31,25 @@ export const Tag = forwardRef(({ name, onClick, className, . ); }); -const getTagStyles = (theme: GrafanaTheme, name: string) => { - const { borderColor, color } = getTagColorsFromName(name); +const getTagStyles = (theme: GrafanaTheme, name: string, colorIndex?: number) => { + let colors; + if (colorIndex === undefined) { + colors = getTagColorsFromName(name); + } else { + colors = getTagColor(colorIndex); + } return { wrapper: css` font-weight: ${theme.typography.weight.semibold}; font-size: ${theme.typography.size.sm}; line-height: ${theme.typography.lineHeight.xs}; vertical-align: baseline; - background-color: ${color}; + background-color: ${colors.color}; color: ${theme.palette.white}; white-space: nowrap; text-shadow: none; padding: 3px 6px; - border: 1px solid ${borderColor}; + border: 1px solid ${colors.borderColor}; border-radius: ${theme.border.radius.md}; :hover { diff --git a/packages/grafana-ui/src/utils/tags.ts b/packages/grafana-ui/src/utils/tags.ts index a75ac5f7f09..ab845c6178f 100644 --- a/packages/grafana-ui/src/utils/tags.ts +++ b/packages/grafana-ui/src/utils/tags.ts @@ -68,9 +68,12 @@ const TAG_BORDER_COLORS = [ */ export function getTagColorsFromName(name = ''): { color: string; borderColor: string } { const hash = djb2(name.toLowerCase()); - const color = TAG_COLORS[Math.abs(hash % TAG_COLORS.length)]; - const borderColor = TAG_BORDER_COLORS[Math.abs(hash % TAG_BORDER_COLORS.length)]; - return { color, borderColor }; + const index = Math.abs(hash % TAG_COLORS.length); + return getTagColor(index); +} + +export function getTagColor(index: number): { color: string; borderColor: string } { + return { color: TAG_COLORS[index], borderColor: TAG_BORDER_COLORS[index] }; } function djb2(str: string) { diff --git a/public/app/features/explore/utils/links.test.ts b/public/app/features/explore/utils/links.test.ts index 7a221e61ef9..580c3e5d0a6 100644 --- a/public/app/features/explore/utils/links.test.ts +++ b/public/app/features/explore/utils/links.test.ts @@ -25,22 +25,32 @@ describe('getFieldLinksForExplore', () => { expect(links[0].title).toBe('external'); }); + it('returns generates title for external link', () => { + const { field, range } = setup({ + title: '', + url: 'http://regionalhost', + }); + const links = getFieldLinksForExplore(field, 0, jest.fn(), range); + + expect(links[0].href).toBe('http://regionalhost'); + expect(links[0].title).toBe('regionalhost'); + }); + it('returns correct link model for internal link', () => { const { field, range } = setup({ - title: 'test', + title: '', url: 'query_1', meta: { datasourceUid: 'uid_1', }, }); const splitfn = jest.fn(); - const links = getFieldLinksForExplore(field, 0, splitfn, range); expect(links[0].href).toBe( '/explore?left={"range":{"from":"now-1h","to":"now"},"datasource":"test_ds","queries":[{"query":"query_1"}],"mode":"Metrics","ui":{"showingGraph":true,"showingTable":true,"showingLogs":true}}' ); - expect(links[0].title).toBe('test'); + expect(links[0].title).toBe('test_ds'); links[0].onClick({}); expect(splitfn).toBeCalledWith({ datasourceUid: 'uid_1', query: 'query_1' }); }); diff --git a/public/app/features/explore/utils/links.ts b/public/app/features/explore/utils/links.ts index 651f267bc41..5be70302455 100644 --- a/public/app/features/explore/utils/links.ts +++ b/public/app/features/explore/utils/links.ts @@ -22,6 +22,10 @@ export function getFieldLinksForExplore( if (d.link.meta?.datasourceUid) { return { ...d.linkModel, + title: + d.linkModel.title || + getDataSourceSrv().getDataSourceSettingsByUid(d.link.meta.datasourceUid)?.name || + 'Unknown datasource', onClick: () => { splitOpenFn({ datasourceUid: d.link.meta.datasourceUid, @@ -37,6 +41,28 @@ export function getFieldLinksForExplore( href: generateInternalHref(d.link.meta.datasourceUid, d.linkModel.href, range), }; } + + if (!d.linkModel.title) { + let href = d.linkModel.href; + // The URL constructor needs the url to have protocol + if (href.indexOf('://') < 0) { + // Doesn't really matter what protocol we use. + href = `http://${href}`; + } + let title; + try { + const parsedUrl = new URL(href); + title = parsedUrl.hostname; + } catch (_e) { + // Should be good enough fallback, user probably did not input valid url. + title = href; + } + + return { + ...d.linkModel, + title, + }; + } return d.linkModel; }); } diff --git a/public/app/plugins/datasource/loki/result_transformer.test.ts b/public/app/plugins/datasource/loki/result_transformer.test.ts index eb8c4e39558..ec0f1fa86f1 100644 --- a/public/app/plugins/datasource/loki/result_transformer.test.ts +++ b/public/app/plugins/datasource/loki/result_transformer.test.ts @@ -104,7 +104,7 @@ describe('loki result transformer', () => { }); describe('enhanceDataFrame', () => { - it('', () => { + it('adds links to fields', () => { const df = new MutableDataFrame({ fields: [{ name: 'line', values: ['nothing', 'trace1=1234', 'trace2=foo'] }] }); enhanceDataFrame(df, { derivedFields: [ @@ -123,8 +123,15 @@ describe('enhanceDataFrame', () => { expect(df.fields.length).toBe(3); const fc = new FieldCache(df); expect(fc.getFieldByName('trace1').values.toArray()).toEqual([null, '1234', null]); - expect(fc.getFieldByName('trace1').config.links[0]).toEqual({ url: 'http://localhost/${__value.raw}', title: '' }); + expect(fc.getFieldByName('trace1').config.links[0]).toEqual({ + url: 'http://localhost/${__value.raw}', + title: '', + }); + expect(fc.getFieldByName('trace2').values.toArray()).toEqual([null, null, 'foo']); - expect(fc.getFieldByName('trace2').config.links[0]).toEqual({ title: '', meta: { datasourceUid: 'uid' } }); + expect(fc.getFieldByName('trace2').config.links[0]).toEqual({ + title: '', + meta: { datasourceUid: 'uid' }, + }); }); }); diff --git a/public/app/plugins/datasource/loki/result_transformer.ts b/public/app/plugins/datasource/loki/result_transformer.ts index 4f4bf528c44..fe45629725d 100644 --- a/public/app/plugins/datasource/loki/result_transformer.ts +++ b/public/app/plugins/datasource/loki/result_transformer.ts @@ -12,6 +12,8 @@ import { findUniqueLabels, FieldConfig, DataFrameView, + DataLink, + Field, } from '@grafana/data'; import templateSrv from 'app/features/templating/template_srv'; @@ -28,6 +30,7 @@ import { LokiTailResponse, LokiQuery, LokiOptions, + DerivedFieldConfig, } from './types'; /** @@ -289,44 +292,50 @@ export const enhanceDataFrame = (dataFrame: DataFrame, config: LokiOptions | nul if (!derivedFields.length) { return; } - - const fields = derivedFields.reduce((acc, field) => { - const config: FieldConfig = {}; - if (field.url || field.datasourceUid) { - config.links = [ - { - url: field.url, - title: '', - meta: field.datasourceUid - ? { - datasourceUid: field.datasourceUid, - } - : undefined, - }, - ]; - } - const dataFrameField = { - name: field.name, - type: FieldType.string, - config, - values: new ArrayVector([]), - }; - - acc[field.name] = dataFrameField; - return acc; - }, {} as Record); + const newFields = derivedFields.map(fieldFromDerivedFieldConfig); + const newFieldsMap = _.keyBy(newFields, 'name'); const view = new DataFrameView(dataFrame); view.forEach((row: { line: string }) => { for (const field of derivedFields) { const logMatch = row.line.match(field.matcherRegex); - fields[field.name].values.add(logMatch && logMatch[1]); + newFieldsMap[field.name].values.add(logMatch && logMatch[1]); } }); - dataFrame.fields = [...dataFrame.fields, ...Object.values(fields)]; + dataFrame.fields = [...dataFrame.fields, ...newFields]; }; +/** + * Transform derivedField config into dataframe field with config that contains link. + */ +function fieldFromDerivedFieldConfig(derivedFieldConfig: DerivedFieldConfig): Field { + const config: FieldConfig = {}; + if (derivedFieldConfig.url || derivedFieldConfig.datasourceUid) { + const link: Partial = { + // We do not know what title to give here so we count on presentation layer to create a title from metadata. + title: '', + url: derivedFieldConfig.url, + }; + + // Having field.datasourceUid means it is an internal link. + if (derivedFieldConfig.datasourceUid) { + link.meta = { + datasourceUid: derivedFieldConfig.datasourceUid, + }; + } + + config.links = [link as DataLink]; + } + return { + name: derivedFieldConfig.name, + type: FieldType.string, + config, + // We are adding values later on + values: new ArrayVector([]), + }; +} + export function rangeQueryResponseToTimeSeries( response: LokiResponse, query: LokiRangeQueryRequest,