Logs: Derived fields link design update (#23695)

170a0df1 · Andrej Ocenas · GitHub · 376765b3 · 170a0df1 · 170a0df1
Commit 170a0df1 authored Apr 23, 2020 by Andrej Ocenas Committed by GitHub Apr 23, 2020
8 changed files
--- 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<Props> {
      }
      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) => {

--- 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<Props, State> {
          {links &&
            links.map(link => {
              return (
-                <span key={link.href}>
                <>
                  &nbsp;
-                    <LinkButton
+                  <FieldLink link={link} />
-                      variant="link"
-                      size={'sm'}
-                      icon={link.onClick ? 'list-ul' : 'external-link-alt'}
-                      href={link.href}
-                      target={'_blank'}
-                      onClick={
-                        link.onClick &&
-                        ((event: any) => {
-                          if (!(event.ctrlKey || event.metaKey || event.shiftKey) && link.onClick) {
-                            event.preventDefault();
-                            link.onClick(event);
-                          }
-                        })
-                      }
-                    />
                </>
-                </span>
              );
            })}
          {showFieldsStats && (
@@ -154,5 +137,40 @@ class UnThemedLogDetailsRow extends PureComponent<Props, State> {
  }
 }
+const getLinkStyles = stylesFactory(() => {
+  return {
+    tag: css`
+      margin-left: 6px;
+      font-size: 11px;
+      padding: 2px 6px;
+    `,
+  };
+});
+type FieldLinkProps = {
+  link: LinkModel<Field>;
+};
+function FieldLink({ link }: FieldLinkProps) {
+  const styles = getLinkStyles();
+  return (
+    <a
+      href={link.href}
+      target={'_blank'}
+      onClick={
+        link.onClick
+          ? event => {
+              if (!(event.ctrlKey || event.metaKey || event.shiftKey) && link.onClick) {
+                event.preventDefault();
+                link.onClick(event);
+              }
+            }
+          : undefined
+      }
+    >
+      <Tag name={link.title} className={styles.tag} colorIndex={6} />
+    </a>
+  );
+}
 export const LogDetailsRow = withTheme(UnThemedLogDetailsRow);
 LogDetailsRow.displayName = 'LogDetailsRow';
--- 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<HTMLElement>) => any;
 export interface Props extends Omit<HTMLAttributes<HTMLElement>, '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<HTMLElement, Props>(({ name, onClick, className, ...rest }, ref) => {
+export const Tag = forwardRef<HTMLElement, Props>(({ 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<HTMLElement>) => {
    if (onClick) {
@@ -29,20 +31,25 @@ export const Tag = forwardRef<HTMLElement, Props>(({ name, onClick, className, .
  );
 });
-const getTagStyles = (theme: GrafanaTheme, name: string) => {
+const getTagStyles = (theme: GrafanaTheme, name: string, colorIndex?: number) => {
-  const { borderColor, color } = getTagColorsFromName(name);
+  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 {

--- 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 index = Math.abs(hash % TAG_COLORS.length);
-  const borderColor = TAG_BORDER_COLORS[Math.abs(hash % TAG_BORDER_COLORS.length)];
+  return getTagColor(index);
-  return { color, borderColor };
+}
+export function getTagColor(index: number): { color: string; borderColor: string } {
+  return { color: TAG_COLORS[index], borderColor: TAG_BORDER_COLORS[index] };
 }
 function djb2(str: string) {

--- 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' });
  });

--- 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;
  });
 }

--- 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' },
+    });
  });
 });
--- 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 newFields = derivedFields.map(fieldFromDerivedFieldConfig);
-  const fields = derivedFields.reduce((acc, field) => {
+  const newFieldsMap = _.keyBy(newFields, 'name');
-    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<string>([]),
-    };
-    acc[field.name] = dataFrameField;
-    return acc;
-  }, {} as Record<string, any>);
  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<any, ArrayVector> {
+  const config: FieldConfig = {};
+  if (derivedFieldConfig.url || derivedFieldConfig.datasourceUid) {
+    const link: Partial<DataLink> = {
+      // 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<string>([]),
+  };
+}
 export function rangeQueryResponseToTimeSeries(
  response: LokiResponse,
  query: LokiRangeQueryRequest,