From 33fcae92eb4715a65f03b370b0e8e33b8aaab24f Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Wed, 4 Jul 2018 19:00:47 +0200 Subject: [PATCH 01/18] Link patterns to make values in tags, processes and logs clickable Signed-off-by: David-Emmanuel Divernois --- .../SpanDetail/AccordianKeyValues.js | 5 +- .../SpanDetail/AccordianLogs.js | 4 +- .../SpanDetail/KeyValuesTable.css | 10 + .../SpanDetail/KeyValuesTable.js | 47 ++++- .../TraceTimelineViewer/SpanDetail/index.js | 15 +- .../TraceTimelineViewer/SpanDetailRow.js | 6 + .../VirtualizedTraceView.js | 10 +- .../jaeger-ui/src/constants/default-config.js | 1 + packages/jaeger-ui/src/model/link-patterns.js | 183 ++++++++++++++++++ 9 files changed, 273 insertions(+), 8 deletions(-) create mode 100644 packages/jaeger-ui/src/model/link-patterns.js diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js index d0eb11d797..ec2c6e4845 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js @@ -30,6 +30,7 @@ type AccordianKeyValuesProps = { highContrast?: boolean, isOpen: boolean, label: string, + linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], onToggle: () => void, }; @@ -59,7 +60,7 @@ KeyValuesSummary.defaultProps = { }; export default function AccordianKeyValues(props: AccordianKeyValuesProps) { - const { className, data, highContrast, isOpen, label, onToggle } = props; + const { className, data, highContrast, isOpen, label, linksGetter, onToggle } = props; const isEmpty = !Array.isArray(data) || !data.length; const iconCls = cx('u-align-icon', { 'AccordianKeyValues--emptyIcon': isEmpty }); return ( @@ -80,7 +81,7 @@ export default function AccordianKeyValues(props: AccordianKeyValuesProps) { {!isOpen && } - {isOpen && } + {isOpen && } ); } diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js index a3df52a8bc..3c037e4034 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js @@ -27,6 +27,7 @@ import './AccordianLogs.css'; type AccordianLogsProps = { isOpen: boolean, + linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], logs: Log[], onItemToggle: Log => void, onToggle: () => void, @@ -35,7 +36,7 @@ type AccordianLogsProps = { }; export default function AccordianLogs(props: AccordianLogsProps) { - const { isOpen, logs, openedItems, onItemToggle, onToggle, timestamp } = props; + const { isOpen, linksGetter, logs, openedItems, onItemToggle, onToggle, timestamp } = props; return (
@@ -59,6 +60,7 @@ export default function AccordianLogs(props: AccordianLogsProps) { // compact highContrast isOpen={openedItems.has(log)} + linksGetter={linksGetter} data={log.fields || []} label={`${formatDuration(log.timestamp - timestamp)}`} onToggle={() => onItemToggle(log)} diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css index 1add328273..eeb7e8b774 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css @@ -39,3 +39,13 @@ limitations under the License. white-space: pre; width: 125px; } + +.KeyValueTable--link { + display: block; + position: relative; +} + +.KeyValueTable--linkIcon { + position: absolute; + right: 0px; +} diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index 8abffdeb78..e461125879 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -16,6 +16,7 @@ import React from 'react'; import jsonMarkup from 'json-markup'; +import { Dropdown, Icon, Menu } from 'antd'; import './KeyValuesTable.css'; @@ -32,10 +33,11 @@ function parseIfJson(value) { type KeyValuesTableProps = { data: { key: string, value: any }[], + linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], }; export default function KeyValuesTable(props: KeyValuesTableProps) { - const { data } = props; + const { data, linksGetter } = props; return (
@@ -45,12 +47,53 @@ export default function KeyValuesTable(props: KeyValuesTableProps) { // eslint-disable-next-line react/no-danger
); + let valueMarkup = jsonTable; + const links = linksGetter ? linksGetter(data, i) : null; + if (links && links.length === 1) { + valueMarkup = ( + + + {jsonTable} + + ); + } else if (links && links.length > 1) { + const menuItems = ( + + {links.map((link, index) => { + const { text, url } = link; + return ( + // `index` is necessary in the key because url can repeat + // eslint-disable-next-line react/no-array-index-key + + + {text} + + + ); + })} + + ); + valueMarkup = ( + + + + {jsonTable} + + + ); + } return ( // `i` is necessary in the key because row.key can repeat // eslint-disable-next-line react/no-array-index-key
- + ); })} diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js index cec43234ad..27df34cbb4 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js @@ -28,6 +28,7 @@ import './index.css'; type SpanDetailProps = { detailState: DetailState, + linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, @@ -37,7 +38,16 @@ type SpanDetailProps = { }; export default function SpanDetail(props: SpanDetailProps) { - const { detailState, logItemToggle, logsToggle, processToggle, span, tagsToggle, traceStartTime } = props; + const { + detailState, + linksGetter, + logItemToggle, + logsToggle, + processToggle, + span, + tagsToggle, + traceStartTime, + } = props; const { isTagsOpen, isProcessOpen, logs: logsState } = detailState; const { operationName, process, duration, relativeStartTime, spanID, logs, tags } = span; const overviewItems = [ @@ -73,6 +83,7 @@ export default function SpanDetail(props: SpanDetailProps) { tagsToggle(spanID)} /> @@ -81,6 +92,7 @@ export default function SpanDetail(props: SpanDetailProps) { className="ub-mb1" data={process.tags} label="Process" + linksGetter={linksGetter} isOpen={isProcessOpen} onToggle={() => processToggle(spanID)} /> @@ -89,6 +101,7 @@ export default function SpanDetail(props: SpanDetailProps) { {logs && logs.length > 0 && ( void, isFilteredOut: boolean, + linksGetter: (number, { key: string, value: any }[], number) => { url: string, text: string }[], logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, span: Span, + spanIndex: number, tagsToggle: string => void, traceStartTime: number, }; @@ -45,6 +47,9 @@ export default class SpanDetailRow extends React.PureComponent + this.props.linksGetter(this.props.spanIndex, items, itemIndex); + render() { const { color, @@ -76,6 +81,7 @@ export default class SpanDetailRow extends React.PureComponent + getLinks(this.props.trace, spanIndex, items, itemIndex); + renderRow = (key: string, style: Style, index: number, attrs: {}) => { const { isDetail, span, spanIndex } = this.rowStates[index]; return isDetail - ? this.renderSpanDetailRow(span, key, style, attrs) + ? this.renderSpanDetailRow(span, spanIndex, key, style, attrs) : this.renderSpanBarRow(span, spanIndex, key, style, attrs); }; @@ -352,7 +356,7 @@ export class VirtualizedTraceViewImpl extends React.PureComponent diff --git a/packages/jaeger-ui/src/constants/default-config.js b/packages/jaeger-ui/src/constants/default-config.js index d526193fca..250d133761 100644 --- a/packages/jaeger-ui/src/constants/default-config.js +++ b/packages/jaeger-ui/src/constants/default-config.js @@ -24,6 +24,7 @@ export default deepFreeze( dagMaxNumServices: FALLBACK_DAG_MAX_NUM_SERVICES, menuEnabled: true, }, + linkPatterns: [], tracking: { gaID: null, trackErrors: true, diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js new file mode 100644 index 0000000000..cf7eeabec3 --- /dev/null +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -0,0 +1,183 @@ +// Copyright (c) 2017 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import _uniq from 'lodash/uniq'; +import { getConfigValue } from '../utils/config/get-config'; + +const parameterRegExp = /\$\{([^{}]*)\}/; + +export function processTemplate(template, encodeFn) { + if (typeof template !== 'string') { + if (!template || !Array.isArray(template.parameters) || !(template.template instanceof Function)) { + throw new Error('Invalid template'); + } + return template; + } + const templateSplit = template.split(parameterRegExp); + const templateSplitLength = templateSplit.length; + const parameters = []; + // odd indexes contain variable names + for (let i = 1; i < templateSplitLength; i += 2) { + const param = templateSplit[i]; + let paramIndex = parameters.indexOf(param); + if (paramIndex === -1) { + paramIndex = parameters.length; + parameters.push(param); + } + templateSplit[i] = paramIndex; + } + return { + parameters, + template: (...args) => { + let text = ''; + for (let i = 0; i < templateSplitLength; i++) { + if (i % 2 === 0) { + text += templateSplit[i]; + } else { + text += encodeFn(args[templateSplit[i]]); + } + } + return text; + }, + }; +} + +export function createTestFunction(entry) { + if (typeof entry === 'string') { + return arg => arg === entry; + } + if (Array.isArray(entry)) { + return arg => entry.indexOf(arg) > -1; + } + if (entry instanceof RegExp) { + return arg => entry.test(arg); + } + if (entry instanceof Function) { + return entry; + } + if (!entry) { + return () => true; + } + throw new Error(`Invalid value: ${entry}`); +} + +const identity = a => a; + +export function processLinkPattern(pattern) { + try { + const url = processTemplate(pattern.url, encodeURIComponent); + const text = processTemplate(pattern.text, identity); + return { + object: pattern, + type: createTestFunction(pattern.type), + key: createTestFunction(pattern.key), + value: createTestFunction(pattern.value), + url, + text, + parameters: _uniq(url.parameters.concat(text.parameters)), + }; + } catch (error) { + // eslint-disable-next-line no-console + console.error(`Ignoring invalid link pattern: ${error}`, pattern); + return null; + } +} + +export function getParameterInArray(name, array) { + if (array) { + return array.find(entry => entry.key === name); + } + return null; +} + +export function getParameterInAncestor(name, spans, startSpanIndex) { + let currentSpan = { depth: spans[startSpanIndex].depth + 1 }; + for (let spanIndex = startSpanIndex; spanIndex >= 0; spanIndex--) { + const nextSpan = spans[spanIndex]; + if (nextSpan.depth < currentSpan.depth) { + currentSpan = nextSpan; + const result = + getParameterInArray(name, currentSpan.tags) || getParameterInArray(name, currentSpan.process.tags); + if (result) { + return result; + } + } + } + return null; +} + +export function callTemplate(template, data) { + return template.template(...template.parameters.map(param => data[param])); +} + +export function computeLinks(linkPatterns, trace, spanIndex, items, itemIndex) { + const item = items[itemIndex]; + const span = trace.spans[spanIndex]; + let type = 'logs'; + const processTags = span.process.tags === items; + if (processTags) { + type = 'process'; + } + const spanTags = span.tags === items; + if (spanTags) { + type = 'tags'; + } + const result = []; + linkPatterns.forEach(pattern => { + if (pattern.type(type) && pattern.key(item.key, item.value, type) && pattern.value(item.value)) { + let parameterValues = {}; + pattern.parameters.every(parameter => { + let entry = getParameterInArray(parameter, items); + if (!entry && !processTags) { + // do not look in ancestors for process tags because the same object may appear in different places in the hierarchy + entry = getParameterInAncestor(parameter, trace.spans, spanIndex); + } + if (entry) { + parameterValues[parameter] = entry.value; + return true; + } + // eslint-disable-next-line no-console + console.warn( + `Skipping link pattern, missing parameter ${parameter} for key ${item.key} in ${type}.`, + pattern.object + ); + parameterValues = null; + return false; + }); + if (parameterValues) { + result.push({ + url: callTemplate(pattern.url, parameterValues), + text: callTemplate(pattern.text, parameterValues), + }); + } + } + }); + return result; +} + +const linkPatterns = (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(value => !!value); +const alreadyComputed = new WeakMap(); + +export default function getLinks(trace, spanIndex, items, itemIndex) { + if (linkPatterns.length === 0) { + return []; + } + const item = items[itemIndex]; + let result = alreadyComputed.get(item); + if (!result) { + result = computeLinks(linkPatterns, trace, spanIndex, items, itemIndex); + alreadyComputed.set(item, result); + } + return result; +} From 83c257e7686535cbe1441e5ceecc37712d28d6b5 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Thu, 5 Jul 2018 11:29:43 +0200 Subject: [PATCH 02/18] Fixing failing test Signed-off-by: David-Emmanuel Divernois --- .../SpanDetail/AccordianKeyValues.js | 2 +- .../TraceTimelineViewer/SpanDetail/AccordianLogs.js | 2 +- .../TraceTimelineViewer/SpanDetail/KeyValuesTable.js | 2 +- .../TracePage/TraceTimelineViewer/SpanDetail/index.js | 2 +- .../TracePage/TraceTimelineViewer/SpanDetailRow.js | 11 +++++++---- .../TraceTimelineViewer/SpanDetailRow.test.js | 1 + 6 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js index ec2c6e4845..aac3bbf240 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js @@ -30,7 +30,7 @@ type AccordianKeyValuesProps = { highContrast?: boolean, isOpen: boolean, label: string, - linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], onToggle: () => void, }; diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js index 3c037e4034..9b27d42fba 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js @@ -27,7 +27,7 @@ import './AccordianLogs.css'; type AccordianLogsProps = { isOpen: boolean, - linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], logs: Log[], onItemToggle: Log => void, onToggle: () => void, diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index e461125879..5790ff028e 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -33,7 +33,7 @@ function parseIfJson(value) { type KeyValuesTableProps = { data: { key: string, value: any }[], - linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], }; export default function KeyValuesTable(props: KeyValuesTableProps) { diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js index 27df34cbb4..1c443d56f0 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js @@ -28,7 +28,7 @@ import './index.css'; type SpanDetailProps = { detailState: DetailState, - linksGetter: ({ key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js index ab550b8029..7bba54a2fc 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js @@ -30,7 +30,7 @@ type SpanDetailRowProps = { detailState: DetailState, onDetailToggled: string => void, isFilteredOut: boolean, - linksGetter: (number, { key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?(number, { key: string, value: any }[], number) => { url: string, text: string }[], logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, @@ -47,8 +47,10 @@ export default class SpanDetailRow extends React.PureComponent - this.props.linksGetter(this.props.spanIndex, items, itemIndex); + _linksGetter = (items: { key: string, value: any }[], itemIndex: number) => { + const { linksGetter, spanIndex } = this.props; + return linksGetter ? linksGetter(spanIndex, items, itemIndex) : []; + }; render() { const { @@ -56,6 +58,7 @@ export default class SpanDetailRow extends React.PureComponent ', () => { const spanDetail = ( Date: Thu, 5 Jul 2018 15:35:25 +0200 Subject: [PATCH 03/18] Moving icons closer to the value to make them more visible Signed-off-by: David-Emmanuel Divernois --- .../SpanDetail/KeyValuesTable.css | 9 +--- .../SpanDetail/KeyValuesTable.js | 53 +++++++++++-------- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css index eeb7e8b774..dd26aab9b1 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css @@ -40,12 +40,7 @@ limitations under the License. width: 125px; } -.KeyValueTable--link { - display: block; - position: relative; -} - .KeyValueTable--linkIcon { - position: absolute; - right: 0px; + vertical-align: middle; + font-weight: bold; } diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index 5790ff028e..7f47afbfb1 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -31,6 +31,22 @@ function parseIfJson(value) { return value; } +function markupAsDiv(markup) { + // eslint-disable-next-line react/no-danger + return
; +} + +function markupAsSpan(markup) { + // eslint-disable-next-line react/no-danger + return ( + $/i, ''), + }} + /> + ); +} + type KeyValuesTableProps = { data: { key: string, value: any }[], linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], @@ -43,24 +59,16 @@ export default function KeyValuesTable(props: KeyValuesTableProps) {
{row.key}{jsonTable}{valueMarkup}
{data.map((row, i) => { - const jsonTable = ( - // eslint-disable-next-line react/no-danger -
- ); - let valueMarkup = jsonTable; + const markup = jsonMarkup(parseIfJson(row.value)); const links = linksGetter ? linksGetter(data, i) : null; + let valueMarkup; if (links && links.length === 1) { valueMarkup = ( - - - {jsonTable} - + ); } else if (links && links.length > 1) { const menuItems = ( @@ -80,13 +88,16 @@ export default function KeyValuesTable(props: KeyValuesTableProps) { ); valueMarkup = ( - - - - {jsonTable} - - + ); + } else { + valueMarkup = markupAsDiv(markup); } return ( // `i` is necessary in the key because row.key can repeat From f305938c4c901410af1f6cbfeed3b2a860797030 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Thu, 5 Jul 2018 18:49:20 +0200 Subject: [PATCH 04/18] Adding some tests Signed-off-by: David-Emmanuel Divernois --- .../SpanDetail/KeyValuesTable.test.js | 57 +++++++++++++ .../jaeger-ui/src/model/link-patterns.test.js | 85 +++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 packages/jaeger-ui/src/model/link-patterns.test.js diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js index f82e062d1f..b059ab4b07 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js @@ -14,6 +14,7 @@ import React from 'react'; import { shallow } from 'enzyme'; +import { Dropdown, Icon } from 'antd'; import KeyValuesTable from './KeyValuesTable'; @@ -38,4 +39,60 @@ describe('', () => { expect(tr.find('.KeyValueTable--keyColumn').text()).toMatch(data[i].key); }); }); + + it('renders a single link correctly', () => { + wrapper.setProps({ + linksGetter: (array, i) => + array[i].key === 'span.kind' + ? [ + { + url: `http://example.com/?kind=${encodeURIComponent(array[i].value)}`, + text: `More info about ${array[i].value}`, + }, + ] + : [], + }); + + const anchor = wrapper.find('a'); + expect(anchor).toHaveLength(1); + expect(anchor.prop('href')).toBe('http://example.com/?kind=client'); + expect(anchor.prop('title')).toBe('More info about client'); + expect(anchor.find(Icon)).toHaveLength(1); + expect( + anchor + .closest('tr') + .find('td') + .first() + .text() + ).toBe('span.kind'); + }); + + it('renders multiple links correctly', () => { + wrapper.setProps({ + linksGetter: (array, i) => + array[i].key === 'span.kind' + ? [ + { url: `http://example.com/1?kind=${encodeURIComponent(array[i].value)}`, text: 'Example 1' }, + { url: `http://example.com/2?kind=${encodeURIComponent(array[i].value)}`, text: 'Example 2' }, + ] + : [], + }); + const dropdown = wrapper.find(Dropdown); + const menu = shallow(dropdown.prop('overlay')); + const anchors = menu.find('a'); + expect(anchors).toHaveLength(2); + const firstAnchor = anchors.first(); + expect(firstAnchor.prop('href')).toBe('http://example.com/1?kind=client'); + expect(firstAnchor.text()).toBe('Example 1'); + const secondAnchor = anchors.last(); + expect(secondAnchor.prop('href')).toBe('http://example.com/2?kind=client'); + expect(secondAnchor.text()).toBe('Example 2'); + expect( + dropdown + .closest('tr') + .find('td') + .first() + .text() + ).toBe('span.kind'); + }); }); diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js new file mode 100644 index 0000000000..531c812d01 --- /dev/null +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -0,0 +1,85 @@ +// Copyright (c) 2017 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { processTemplate } from './link-patterns'; + +describe('processTemplate()', () => { + it('correctly replaces variables', () => { + const processedTemplate = processTemplate( + // eslint-disable-next-line no-template-curly-in-string + 'this is a test with ${oneVariable}${anotherVariable} and the same ${oneVariable}', + a => a + ); + expect(processedTemplate.parameters).toEqual(['oneVariable', 'anotherVariable']); + expect(processedTemplate.template('MYFIRSTVAR', 'SECOND')).toBe( + 'this is a test with MYFIRSTVARSECOND and the same MYFIRSTVAR' + ); + }); + + it('correctly uses the encoding function', () => { + const processedTemplate = processTemplate( + // eslint-disable-next-line no-template-curly-in-string + 'this is a test with ${oneVariable}${anotherVariable} and the same ${oneVariable}', + e => `/${e}\\` + ); + expect(processedTemplate.parameters).toEqual(['oneVariable', 'anotherVariable']); + expect(processedTemplate.template('MYFIRSTVAR', 'SECOND')).toBe( + 'this is a test with /MYFIRSTVAR\\/SECOND\\ and the same /MYFIRSTVAR\\' + ); + }); + + it('correctly returns the same object when passing an already processed template', () => { + const alreadyProcessed = { + parameters: ['b'], + template: b => `a${b}c`, + }; + const processedTemplate = processTemplate(alreadyProcessed, a => a); + expect(processedTemplate).toBe(alreadyProcessed); + }); + + it('reports an error when passing an object that does not look like an already processed template', () => { + expect(() => + processTemplate( + { + template: b => `a${b}c`, + }, + a => a + ) + ).toThrow(); + expect(() => + processTemplate( + { + parameters: ['b'], + }, + a => a + ) + ).toThrow(); + expect(() => processTemplate({}, a => a)).toThrow(); + }); +}); + +// TODO: +/* +describe('createTestFunction()', () => {}); + +describe('processLinkPattern()', () => {}); + +describe('getParameterInArray()', () => {}); + +describe('getParameterInAncestor()', () => {}); + +describe('callTemplate()', () => {}); + +describe('computeLinks()', () => {}); +*/ From 7849922a16a0879cc565bb20762c1928ecb1b978 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Thu, 5 Jul 2018 18:50:56 +0200 Subject: [PATCH 05/18] Fixing misplaced "// eslint-disable-next-line react/no-danger" comment Signed-off-by: David-Emmanuel Divernois --- .../TraceTimelineViewer/SpanDetail/KeyValuesTable.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index 7f47afbfb1..b2354c57c0 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -37,14 +37,9 @@ function markupAsDiv(markup) { } function markupAsSpan(markup) { + const spanMarkup = markup.replace(/^
$/i, ''); // eslint-disable-next-line react/no-danger - return ( - $/i, ''), - }} - /> - ); + return ; } type KeyValuesTableProps = { From efd44fdecd0dd9c7314f94bfb8aba9eceea79a1a Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Fri, 6 Jul 2018 11:47:33 +0200 Subject: [PATCH 06/18] Using ub-inline-block instead of manipulating HTML in KeyValuesTable (as recommended by Joe Farro) Signed-off-by: David-Emmanuel Divernois --- .../SpanDetail/KeyValuesTable.css | 5 ++++ .../SpanDetail/KeyValuesTable.js | 23 +++++++------------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css index dd26aab9b1..826cd97a05 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.css @@ -40,6 +40,11 @@ limitations under the License. width: 125px; } +.KeyValueTable--body > tr > td { + padding: 0.25rem 0.5rem; + vertical-align: top; +} + .KeyValueTable--linkIcon { vertical-align: middle; font-weight: bold; diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index b2354c57c0..d2454978a6 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -31,17 +31,6 @@ function parseIfJson(value) { return value; } -function markupAsDiv(markup) { - // eslint-disable-next-line react/no-danger - return
; -} - -function markupAsSpan(markup) { - const spanMarkup = markup.replace(/^
$/i, ''); - // eslint-disable-next-line react/no-danger - return ; -} - type KeyValuesTableProps = { data: { key: string, value: any }[], linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], @@ -54,14 +43,18 @@ export default function KeyValuesTable(props: KeyValuesTableProps) {
{data.map((row, i) => { - const markup = jsonMarkup(parseIfJson(row.value)); + const markup = { + __html: jsonMarkup(parseIfJson(row.value)), + }; + // eslint-disable-next-line react/no-danger + const jsonTable =
; const links = linksGetter ? linksGetter(data, i) : null; let valueMarkup; if (links && links.length === 1) { valueMarkup = ( ); @@ -86,13 +79,13 @@ export default function KeyValuesTable(props: KeyValuesTableProps) { ); } else { - valueMarkup = markupAsDiv(markup); + valueMarkup = jsonTable; } return ( // `i` is necessary in the key because row.key can repeat From 0901e533e62ad404d849e8598379eb2586bc3fc6 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Fri, 6 Jul 2018 14:41:09 +0200 Subject: [PATCH 07/18] Adding tests for createTestFunction Signed-off-by: David-Emmanuel Divernois --- packages/jaeger-ui/src/model/link-patterns.js | 4 +- .../jaeger-ui/src/model/link-patterns.test.js | 70 ++++++++++++++++++- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js index cf7eeabec3..5dcada0c1b 100644 --- a/packages/jaeger-ui/src/model/link-patterns.js +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -63,10 +63,10 @@ export function createTestFunction(entry) { if (entry instanceof RegExp) { return arg => entry.test(arg); } - if (entry instanceof Function) { + if (typeof entry === 'function') { return entry; } - if (!entry) { + if (entry == null) { return () => true; } throw new Error(`Invalid value: ${entry}`); diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 531c812d01..472e0f3525 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { processTemplate } from './link-patterns'; +import { processTemplate, createTestFunction } from './link-patterns'; describe('processTemplate()', () => { it('correctly replaces variables', () => { @@ -69,10 +69,72 @@ describe('processTemplate()', () => { }); }); +describe('createTestFunction()', () => { + it('accepts a string', () => { + const testFn = createTestFunction('myValue'); + expect(testFn('myValue')).toBe(true); + expect(testFn('myFirstValue')).toBe(false); + expect(testFn('mySecondValue')).toBe(false); + expect(testFn('otherValue')).toBe(false); + }); + + it('accepts an array', () => { + const testFn = createTestFunction(['myFirstValue', 'mySecondValue']); + expect(testFn('myValue')).toBe(false); + expect(testFn('myFirstValue')).toBe(true); + expect(testFn('mySecondValue')).toBe(true); + expect(testFn('otherValue')).toBe(false); + }); + + it('accepts a regular expression', () => { + const testFn = createTestFunction(/^my.*Value$/); + expect(testFn('myValue')).toBe(true); + expect(testFn('myFirstValue')).toBe(true); + expect(testFn('mySecondValue')).toBe(true); + expect(testFn('otherValue')).toBe(false); + }); + + it('accepts a function', () => { + const mockCallback = jest.fn(); + mockCallback + .mockReturnValueOnce(true) + .mockReturnValueOnce(false) + .mockReturnValueOnce(true) + .mockReturnValue(false); + const testFn = createTestFunction(mockCallback); + expect(testFn('myValue')).toBe(true); + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenCalledWith('myValue'); + expect(testFn('myFirstValue')).toBe(false); + expect(mockCallback).toHaveBeenCalledTimes(2); + expect(mockCallback).toHaveBeenCalledWith('myFirstValue'); + expect(testFn('mySecondValue')).toBe(true); + expect(mockCallback).toHaveBeenCalledTimes(3); + expect(mockCallback).toHaveBeenCalledWith('mySecondValue'); + expect(testFn('otherValue')).toBe(false); + expect(mockCallback).toHaveBeenCalledTimes(4); + expect(mockCallback).toHaveBeenCalledWith('otherValue'); + }); + + it('accepts undefined', () => { + const testFn = createTestFunction(); + expect(testFn('myValue')).toBe(true); + expect(testFn('myFirstValue')).toBe(true); + expect(testFn('mySecondValue')).toBe(true); + expect(testFn('otherValue')).toBe(true); + }); + + it('rejects unknown values', () => { + expect(() => createTestFunction({})).toThrow(); + expect(() => createTestFunction(true)).toThrow(); + expect(() => createTestFunction(false)).toThrow(); + expect(() => createTestFunction(0)).toThrow(); + expect(() => createTestFunction(5)).toThrow(); + }); +}); + // TODO: /* -describe('createTestFunction()', () => {}); - describe('processLinkPattern()', () => {}); describe('getParameterInArray()', () => {}); @@ -82,4 +144,6 @@ describe('getParameterInAncestor()', () => {}); describe('callTemplate()', () => {}); describe('computeLinks()', () => {}); + +describe('getLinks()', () => {}); */ From 0544a6257f17936e409484455a464f85e5a5c519 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Fri, 6 Jul 2018 15:10:11 +0200 Subject: [PATCH 08/18] Adding tests for getParameterInArray Signed-off-by: David-Emmanuel Divernois --- packages/jaeger-ui/src/model/link-patterns.js | 4 ++-- .../jaeger-ui/src/model/link-patterns.test.js | 22 ++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js index 5dcada0c1b..5ac2bd6ede 100644 --- a/packages/jaeger-ui/src/model/link-patterns.js +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -98,7 +98,7 @@ export function getParameterInArray(name, array) { if (array) { return array.find(entry => entry.key === name); } - return null; + return undefined; } export function getParameterInAncestor(name, spans, startSpanIndex) { @@ -114,7 +114,7 @@ export function getParameterInAncestor(name, spans, startSpanIndex) { } } } - return null; + return undefined; } export function callTemplate(template, data) { diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 472e0f3525..27568863dd 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { processTemplate, createTestFunction } from './link-patterns'; +import { processTemplate, createTestFunction, getParameterInArray } from './link-patterns'; describe('processTemplate()', () => { it('correctly replaces variables', () => { @@ -133,12 +133,28 @@ describe('createTestFunction()', () => { }); }); +describe('getParameterInArray()', () => { + const data = [{ key: 'mykey', value: 'ok' }, { key: 'otherkey', value: 'v' }]; + + it('returns an entry that is present', () => { + expect(getParameterInArray('mykey', data)).toBe(data[0]); + expect(getParameterInArray('otherkey', data)).toBe(data[1]); + }); + + it('returns undefined when the entry cannot be found', () => { + expect(getParameterInArray('myotherkey', data)).toBeUndefined(); + }); + + it('returns undefined when there is no array', () => { + expect(getParameterInArray('otherkey')).toBeUndefined(); + expect(getParameterInArray('otherkey', null)).toBeUndefined(); + }); +}); + // TODO: /* describe('processLinkPattern()', () => {}); -describe('getParameterInArray()', () => {}); - describe('getParameterInAncestor()', () => {}); describe('callTemplate()', () => {}); From 858bb1087cb58b0ee7b2e7818624e6e6c1ff8612 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Fri, 6 Jul 2018 16:00:23 +0200 Subject: [PATCH 09/18] Adding tests for getParameterInAncestor Signed-off-by: David-Emmanuel Divernois --- .../jaeger-ui/src/model/link-patterns.test.js | 129 +++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 27568863dd..64f686ab54 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -12,7 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { processTemplate, createTestFunction, getParameterInArray } from './link-patterns'; +import { + processTemplate, + createTestFunction, + getParameterInArray, + getParameterInAncestor, +} from './link-patterns'; describe('processTemplate()', () => { it('correctly replaces variables', () => { @@ -151,12 +156,130 @@ describe('getParameterInArray()', () => { }); }); +describe('getParameterInAncestor()', () => { + const spans = [ + { + depth: 0, + process: { + tags: [ + { key: 'a', value: 'a7' }, + { key: 'b', value: 'b7' }, + { key: 'c', value: 'c7' }, + { key: 'd', value: 'd7' }, + { key: 'e', value: 'e7' }, + { key: 'f', value: 'f7' }, + { key: 'g', value: 'g7' }, + { key: 'h', value: 'h7' }, + ], + }, + tags: [ + { key: 'a', value: 'a6' }, + { key: 'b', value: 'b6' }, + { key: 'c', value: 'c6' }, + { key: 'd', value: 'd6' }, + { key: 'e', value: 'e6' }, + { key: 'f', value: 'f6' }, + { key: 'g', value: 'g6' }, + ], + }, + { + depth: 1, + process: { + tags: [ + { key: 'a', value: 'a5' }, + { key: 'b', value: 'b5' }, + { key: 'c', value: 'c5' }, + { key: 'd', value: 'd5' }, + { key: 'e', value: 'e5' }, + { key: 'f', value: 'f5' }, + ], + }, + tags: [ + { key: 'a', value: 'a4' }, + { key: 'b', value: 'b4' }, + { key: 'c', value: 'c4' }, + { key: 'd', value: 'd4' }, + { key: 'e', value: 'e4' }, + ], + }, + { + depth: 1, + process: { + tags: [ + { key: 'a', value: 'a3' }, + { key: 'b', value: 'b3' }, + { key: 'c', value: 'c3' }, + { key: 'd', value: 'd3' }, + ], + }, + tags: [{ key: 'a', value: 'a2' }, { key: 'b', value: 'b2' }, { key: 'c', value: 'c2' }], + }, + { + depth: 2, + process: { + tags: [{ key: 'a', value: 'a1' }, { key: 'b', value: 'b1' }], + }, + tags: [{ key: 'a', value: 'a0' }], + }, + ]; + + it('uses current span tags', () => { + expect(getParameterInAncestor('a', spans, 3)).toEqual({ key: 'a', value: 'a0' }); + expect(getParameterInAncestor('a', spans, 2)).toEqual({ key: 'a', value: 'a2' }); + expect(getParameterInAncestor('a', spans, 1)).toEqual({ key: 'a', value: 'a4' }); + expect(getParameterInAncestor('a', spans, 0)).toEqual({ key: 'a', value: 'a6' }); + }); + + it('uses current span process tags', () => { + expect(getParameterInAncestor('b', spans, 3)).toEqual({ key: 'b', value: 'b1' }); + expect(getParameterInAncestor('d', spans, 2)).toEqual({ key: 'd', value: 'd3' }); + expect(getParameterInAncestor('f', spans, 1)).toEqual({ key: 'f', value: 'f5' }); + expect(getParameterInAncestor('h', spans, 0)).toEqual({ key: 'h', value: 'h7' }); + }); + + it('uses parent span tags', () => { + expect(getParameterInAncestor('c', spans, 3)).toEqual({ key: 'c', value: 'c2' }); + expect(getParameterInAncestor('e', spans, 2)).toEqual({ key: 'e', value: 'e6' }); + expect(getParameterInAncestor('f', spans, 2)).toEqual({ key: 'f', value: 'f6' }); + expect(getParameterInAncestor('g', spans, 2)).toEqual({ key: 'g', value: 'g6' }); + expect(getParameterInAncestor('g', spans, 1)).toEqual({ key: 'g', value: 'g6' }); + }); + + it('uses parent span process tags', () => { + expect(getParameterInAncestor('d', spans, 3)).toEqual({ key: 'd', value: 'd3' }); + expect(getParameterInAncestor('h', spans, 2)).toEqual({ key: 'h', value: 'h7' }); + expect(getParameterInAncestor('h', spans, 1)).toEqual({ key: 'h', value: 'h7' }); + }); + + it('uses grand-parent span tags', () => { + expect(getParameterInAncestor('e', spans, 3)).toEqual({ key: 'e', value: 'e6' }); + expect(getParameterInAncestor('f', spans, 3)).toEqual({ key: 'f', value: 'f6' }); + expect(getParameterInAncestor('g', spans, 3)).toEqual({ key: 'g', value: 'g6' }); + }); + + it('uses grand-parent process tags', () => { + expect(getParameterInAncestor('h', spans, 3)).toEqual({ key: 'h', value: 'h7' }); + }); + + it('returns undefined when the entry cannot be found', () => { + expect(getParameterInAncestor('i', spans, 3)).toBeUndefined(); + }); + + it('does not break if some tags are not defined', () => { + const spansWithUndefinedTags = [ + { + depth: 0, + process: {}, + }, + ]; + expect(getParameterInAncestor('a', spansWithUndefinedTags, 0)).toBeUndefined(); + }); +}); + // TODO: /* describe('processLinkPattern()', () => {}); -describe('getParameterInAncestor()', () => {}); - describe('callTemplate()', () => {}); describe('computeLinks()', () => {}); From 136c149060637b33342cd8aa04baea9774709508 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Fri, 6 Jul 2018 16:54:22 +0200 Subject: [PATCH 10/18] Adding test for SpanDetailRow Signed-off-by: David-Emmanuel Divernois --- .../TraceTimelineViewer/SpanDetailRow.js | 3 +-- .../TraceTimelineViewer/SpanDetailRow.test.js | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js index 7bba54a2fc..303a3b5f67 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js @@ -58,7 +58,6 @@ export default class SpanDetailRow extends React.PureComponent ', () => { columnDivision: 0.5, detailState: new DetailState(), onDetailToggled: jest.fn(), + linksGetter: jest.fn(), isFilteredOut: false, logItemToggle: jest.fn(), logsToggle: jest.fn(), processToggle: jest.fn(), span: { spanID, depth: 3 }, + spanIndex: 4, tagsToggle: jest.fn(), traceStartTime: 1000, }; @@ -40,6 +42,7 @@ describe('', () => { beforeEach(() => { props.onDetailToggled.mockReset(); + props.linksGetter.mockReset(); props.logItemToggle.mockReset(); props.logsToggle.mockReset(); props.processToggle.mockReset(); @@ -72,7 +75,7 @@ describe('', () => { const spanDetail = ( ', () => { ); expect(wrapper.contains(spanDetail)).toBe(true); }); + + it('adds spanIndex when calling linksGetter', () => { + const spanDetail = wrapper.find(SpanDetail); + const linksGetter = spanDetail.prop('linksGetter'); + const tags = [{ key: 'myKey', value: 'myValue' }]; + const linksGetterResponse = {}; + props.linksGetter.mockReturnValueOnce(linksGetterResponse); + const result = linksGetter(tags, 0); + expect(result).toBe(linksGetterResponse); + expect(props.linksGetter).toHaveBeenCalledTimes(1); + expect(props.linksGetter).toHaveBeenCalledWith(props.spanIndex, tags, 0); + }); }); From 73db8e90395fd432a5e6f74834eb5c2d0723ce4a Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Fri, 6 Jul 2018 17:07:38 +0200 Subject: [PATCH 11/18] Adding test for callTemplate Signed-off-by: David-Emmanuel Divernois --- .../jaeger-ui/src/model/link-patterns.test.js | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 64f686ab54..57afb3e194 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -17,6 +17,7 @@ import { createTestFunction, getParameterInArray, getParameterInAncestor, + callTemplate, } from './link-patterns'; describe('processTemplate()', () => { @@ -276,12 +277,28 @@ describe('getParameterInAncestor()', () => { }); }); +describe('callTemplate()', () => { + it('correctly calls the template', () => { + const template = { + parameters: ['myKey', 'otherKey'], + template: jest.fn(), + }; + template.template.mockReturnValue('ok'); + expect( + callTemplate(template, { + otherKey: 'valueForOtherKey', + myKey: 'forMyKey', + }) + ).toBe('ok'); + expect(template.template).toHaveBeenCalledTimes(1); + expect(template.template).toHaveBeenCalledWith('forMyKey', 'valueForOtherKey'); + }); +}); + // TODO: /* describe('processLinkPattern()', () => {}); -describe('callTemplate()', () => {}); - describe('computeLinks()', () => {}); describe('getLinks()', () => {}); From a159459c548770100f8b7e4970845017f19f69b9 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Fri, 6 Jul 2018 17:28:08 +0200 Subject: [PATCH 12/18] Adding test for computeLinks Signed-off-by: David-Emmanuel Divernois --- .../jaeger-ui/src/model/link-patterns.test.js | 47 ++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 57afb3e194..213d787738 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -18,6 +18,8 @@ import { getParameterInArray, getParameterInAncestor, callTemplate, + processLinkPattern, + computeLinks, } from './link-patterns'; describe('processTemplate()', () => { @@ -295,11 +297,44 @@ describe('callTemplate()', () => { }); }); -// TODO: -/* -describe('processLinkPattern()', () => {}); +describe('computeLinks()', () => { + const linkPatterns = [ + { + type: 'tags', + key: 'myKey', + // eslint-disable-next-line no-template-curly-in-string + url: 'http://example.com/?myKey=${myKey}', + // eslint-disable-next-line no-template-curly-in-string + text: 'first link (${myKey})', + }, + { + key: 'myOtherKey', + // eslint-disable-next-line no-template-curly-in-string + url: 'http://example.com/?myKey=${myOtherKey}&myKey=${myKey}', + // eslint-disable-next-line no-template-curly-in-string + text: 'second link (${myOtherKey})', + }, + ].map(processLinkPattern); -describe('computeLinks()', () => {}); + const trace = { + spans: [ + { depth: 0, process: {}, tags: [{ key: 'myKey', value: 'valueOfMyKey' }] }, + { depth: 1, process: {}, logs: [{ fields: [{ key: 'myOtherKey', value: 'valueOfMy+Other+Key' }] }] }, + ], + }; -describe('getLinks()', () => {}); -*/ + it('correctly computes links', () => { + expect(computeLinks(linkPatterns, trace, 0, trace.spans[0].tags, 0)).toEqual([ + { + url: 'http://example.com/?myKey=valueOfMyKey', + text: 'first link (valueOfMyKey)', + }, + ]); + expect(computeLinks(linkPatterns, trace, 1, trace.spans[1].logs[0].fields, 0)).toEqual([ + { + url: 'http://example.com/?myKey=valueOfMy%2BOther%2BKey&myKey=valueOfMyKey', + text: 'second link (valueOfMy+Other+Key)', + }, + ]); + }); +}); From dd795d24a3e10573946652d3be485bf403a74469 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Wed, 25 Jul 2018 17:52:32 +0200 Subject: [PATCH 13/18] Changes following code review Signed-off-by: David-Emmanuel Divernois --- .../SpanDetail/KeyValuesTable.js | 46 ++++--- .../SpanDetail/KeyValuesTable.test.js | 11 +- packages/jaeger-ui/src/model/link-patterns.js | 123 +++++++++++------- .../jaeger-ui/src/model/link-patterns.test.js | 37 ++---- 4 files changed, 122 insertions(+), 95 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index d2454978a6..402ebf4da3 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -14,7 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import React from 'react'; +import * as React from 'react'; import jsonMarkup from 'json-markup'; import { Dropdown, Icon, Menu } from 'antd'; @@ -31,6 +31,24 @@ function parseIfJson(value) { return value; } +const LinkValue = (props: { href: string, title?: string, children: React.Node }) => ( + + {props.children} + +); + +const linkValueList = (links: { url: string, text: string }[]) => ( + + {links.map(({ text, url }, index) => ( + // `index` is necessary in the key because url can repeat + // eslint-disable-next-line react/no-array-index-key + + {text} + + ))} + +); + type KeyValuesTableProps = { data: { key: string, value: any }[], linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], @@ -53,31 +71,15 @@ export default function KeyValuesTable(props: KeyValuesTableProps) { if (links && links.length === 1) { valueMarkup = (
- - {jsonTable} - + + {jsonTable} +
); } else if (links && links.length > 1) { - const menuItems = ( - - {links.map((link, index) => { - const { text, url } = link; - return ( - // `index` is necessary in the key because url can repeat - // eslint-disable-next-line react/no-array-index-key - - - {text} - - - ); - })} - - ); valueMarkup = (
- + {jsonTable} @@ -101,3 +103,5 @@ export default function KeyValuesTable(props: KeyValuesTableProps) {
); } + +KeyValuesTable.LinkValue = LinkValue; diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js index b059ab4b07..653c8104af 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js @@ -14,7 +14,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { Dropdown, Icon } from 'antd'; +import { Dropdown } from 'antd'; import KeyValuesTable from './KeyValuesTable'; @@ -53,11 +53,10 @@ describe('', () => { : [], }); - const anchor = wrapper.find('a'); + const anchor = wrapper.find(KeyValuesTable.LinkValue); expect(anchor).toHaveLength(1); expect(anchor.prop('href')).toBe('http://example.com/?kind=client'); expect(anchor.prop('title')).toBe('More info about client'); - expect(anchor.find(Icon)).toHaveLength(1); expect( anchor .closest('tr') @@ -79,14 +78,14 @@ describe('', () => { }); const dropdown = wrapper.find(Dropdown); const menu = shallow(dropdown.prop('overlay')); - const anchors = menu.find('a'); + const anchors = menu.find(KeyValuesTable.LinkValue); expect(anchors).toHaveLength(2); const firstAnchor = anchors.first(); expect(firstAnchor.prop('href')).toBe('http://example.com/1?kind=client'); - expect(firstAnchor.text()).toBe('Example 1'); + expect(firstAnchor.children().text()).toBe('Example 1'); const secondAnchor = anchors.last(); expect(secondAnchor.prop('href')).toBe('http://example.com/2?kind=client'); - expect(secondAnchor.text()).toBe('Example 2'); + expect(secondAnchor.children().text()).toBe('Example 2'); expect( dropdown .closest('tr') diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js index 5ac2bd6ede..0d5b8a6eac 100644 --- a/packages/jaeger-ui/src/model/link-patterns.js +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -1,3 +1,5 @@ +// @flow + // Copyright (c) 2017 The Jaeger Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,58 +16,67 @@ import _uniq from 'lodash/uniq'; import { getConfigValue } from '../utils/config/get-config'; +import type { Span, Trace } from '../types'; + +const parameterRegExp = /\$\{([^{}]*)\}/g; + +type ProcessedTemplate = { + parameters: string[], + template: (data: { [parameter: string]: any }) => string, +}; + +function getParamNames(str) { + const names = []; + str.replace(parameterRegExp, (match, name) => { + names.push(name); + return match; + }); + return _uniq(names); +} -const parameterRegExp = /\$\{([^{}]*)\}/; +function stringSupplant(str, encodeFn: any => string, map) { + return str.replace(parameterRegExp, (_, name) => encodeFn(map[name])); +} -export function processTemplate(template, encodeFn) { +export function processTemplate( + template: any, + encodeFn: any => string +): ProcessedTemplate { if (typeof template !== 'string') { - if (!template || !Array.isArray(template.parameters) || !(template.template instanceof Function)) { - throw new Error('Invalid template'); - } - return template; - } - const templateSplit = template.split(parameterRegExp); - const templateSplitLength = templateSplit.length; - const parameters = []; - // odd indexes contain variable names - for (let i = 1; i < templateSplitLength; i += 2) { - const param = templateSplit[i]; - let paramIndex = parameters.indexOf(param); - if (paramIndex === -1) { - paramIndex = parameters.length; - parameters.push(param); + /* + + // kept on ice until #123 is implemented: + if (template && Array.isArray(template.parameters) && (typeof template.template === 'function')) { + return template; } - templateSplit[i] = paramIndex; + + */ + throw new Error('Invalid template'); } return { - parameters, - template: (...args) => { - let text = ''; - for (let i = 0; i < templateSplitLength; i++) { - if (i % 2 === 0) { - text += templateSplit[i]; - } else { - text += encodeFn(args[templateSplit[i]]); - } - } - return text; - }, + parameters: getParamNames(template), + template: stringSupplant.bind(null, template, encodeFn), }; } -export function createTestFunction(entry) { +export function createTestFunction(entry: any) { if (typeof entry === 'string') { - return arg => arg === entry; + return (arg: any) => arg === entry; } if (Array.isArray(entry)) { - return arg => entry.indexOf(arg) > -1; + return (arg: any) => entry.indexOf(arg) > -1; } + /* + + // kept on ice until #123 is implemented: if (entry instanceof RegExp) { - return arg => entry.test(arg); + return (arg: any) => entry.test(arg); } if (typeof entry === 'function') { return entry; } + + */ if (entry == null) { return () => true; } @@ -74,7 +85,17 @@ export function createTestFunction(entry) { const identity = a => a; -export function processLinkPattern(pattern) { +type ProcessedLinkPattern = { + object: any, + type: string => boolean, + key: string => boolean, + value: any => boolean, + url: ProcessedTemplate, + text: ProcessedTemplate, + parameters: string[], +}; + +export function processLinkPattern(pattern: any): ?ProcessedLinkPattern { try { const url = processTemplate(pattern.url, encodeURIComponent); const text = processTemplate(pattern.text, identity); @@ -94,16 +115,16 @@ export function processLinkPattern(pattern) { } } -export function getParameterInArray(name, array) { +export function getParameterInArray(name: string, array: { key: string, value: any }[]) { if (array) { return array.find(entry => entry.key === name); } return undefined; } -export function getParameterInAncestor(name, spans, startSpanIndex) { +export function getParameterInAncestor(name: string, spans: Span[], startSpanIndex: number) { let currentSpan = { depth: spans[startSpanIndex].depth + 1 }; - for (let spanIndex = startSpanIndex; spanIndex >= 0; spanIndex--) { + for (let spanIndex = startSpanIndex; spanIndex >= 0 && currentSpan.depth > 0; spanIndex--) { const nextSpan = spans[spanIndex]; if (nextSpan.depth < currentSpan.depth) { currentSpan = nextSpan; @@ -117,11 +138,17 @@ export function getParameterInAncestor(name, spans, startSpanIndex) { return undefined; } -export function callTemplate(template, data) { - return template.template(...template.parameters.map(param => data[param])); +function callTemplate(template, data) { + return template.template(data); } -export function computeLinks(linkPatterns, trace, spanIndex, items, itemIndex) { +export function computeLinks( + linkPatterns: ProcessedLinkPattern[], + trace: Trace, + spanIndex: number, + items: { key: string, value: any }[], + itemIndex: number +) { const item = items[itemIndex]; const span = trace.spans[spanIndex]; let type = 'logs'; @@ -135,15 +162,16 @@ export function computeLinks(linkPatterns, trace, spanIndex, items, itemIndex) { } const result = []; linkPatterns.forEach(pattern => { - if (pattern.type(type) && pattern.key(item.key, item.value, type) && pattern.value(item.value)) { + if (pattern.type(type) && pattern.key(item.key) && pattern.value(item.value)) { let parameterValues = {}; pattern.parameters.every(parameter => { let entry = getParameterInArray(parameter, items); if (!entry && !processTags) { // do not look in ancestors for process tags because the same object may appear in different places in the hierarchy + // and the cache in getLinks uses that object as a key entry = getParameterInAncestor(parameter, trace.spans, spanIndex); } - if (entry) { + if (entry && parameterValues) { parameterValues[parameter] = entry.value; return true; } @@ -166,10 +194,15 @@ export function computeLinks(linkPatterns, trace, spanIndex, items, itemIndex) { return result; } -const linkPatterns = (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(value => !!value); +const linkPatterns = (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(Boolean); const alreadyComputed = new WeakMap(); -export default function getLinks(trace, spanIndex, items, itemIndex) { +export default function getLinks( + trace: Trace, + spanIndex: number, + items: { key: string, value: any }[], + itemIndex: number +) { if (linkPatterns.length === 0) { return []; } diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 213d787738..65e430325d 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -17,7 +17,6 @@ import { createTestFunction, getParameterInArray, getParameterInAncestor, - callTemplate, processLinkPattern, computeLinks, } from './link-patterns'; @@ -30,7 +29,7 @@ describe('processTemplate()', () => { a => a ); expect(processedTemplate.parameters).toEqual(['oneVariable', 'anotherVariable']); - expect(processedTemplate.template('MYFIRSTVAR', 'SECOND')).toBe( + expect(processedTemplate.template({ oneVariable: 'MYFIRSTVAR', anotherVariable: 'SECOND' })).toBe( 'this is a test with MYFIRSTVARSECOND and the same MYFIRSTVAR' ); }); @@ -42,25 +41,30 @@ describe('processTemplate()', () => { e => `/${e}\\` ); expect(processedTemplate.parameters).toEqual(['oneVariable', 'anotherVariable']); - expect(processedTemplate.template('MYFIRSTVAR', 'SECOND')).toBe( + expect(processedTemplate.template({ oneVariable: 'MYFIRSTVAR', anotherVariable: 'SECOND' })).toBe( 'this is a test with /MYFIRSTVAR\\/SECOND\\ and the same /MYFIRSTVAR\\' ); }); + /* + // kept on ice until #123 is implemented: + it('correctly returns the same object when passing an already processed template', () => { const alreadyProcessed = { parameters: ['b'], - template: b => `a${b}c`, + template: data => `a${data.b}c`, }; const processedTemplate = processTemplate(alreadyProcessed, a => a); expect(processedTemplate).toBe(alreadyProcessed); }); + */ + it('reports an error when passing an object that does not look like an already processed template', () => { expect(() => processTemplate( { - template: b => `a${b}c`, + template: data => `a${data.b}c`, }, a => a ) @@ -94,6 +98,9 @@ describe('createTestFunction()', () => { expect(testFn('otherValue')).toBe(false); }); + /* + // kept on ice until #123 is implemented: + it('accepts a regular expression', () => { const testFn = createTestFunction(/^my.*Value$/); expect(testFn('myValue')).toBe(true); @@ -124,6 +131,8 @@ describe('createTestFunction()', () => { expect(mockCallback).toHaveBeenCalledWith('otherValue'); }); + */ + it('accepts undefined', () => { const testFn = createTestFunction(); expect(testFn('myValue')).toBe(true); @@ -279,24 +288,6 @@ describe('getParameterInAncestor()', () => { }); }); -describe('callTemplate()', () => { - it('correctly calls the template', () => { - const template = { - parameters: ['myKey', 'otherKey'], - template: jest.fn(), - }; - template.template.mockReturnValue('ok'); - expect( - callTemplate(template, { - otherKey: 'valueForOtherKey', - myKey: 'forMyKey', - }) - ).toBe('ok'); - expect(template.template).toHaveBeenCalledTimes(1); - expect(template.template).toHaveBeenCalledWith('forMyKey', 'valueForOtherKey'); - }); -}); - describe('computeLinks()', () => { const linkPatterns = [ { From 9acf208ae54d9437daad8f0e81fdf989a73abcbd Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Thu, 26 Jul 2018 17:10:14 +0200 Subject: [PATCH 14/18] Adding tests for getLinks Signed-off-by: David-Emmanuel Divernois --- packages/jaeger-ui/src/model/link-patterns.js | 43 ++++++++------- .../jaeger-ui/src/model/link-patterns.test.js | 53 +++++++++++++++++++ 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js index 0d5b8a6eac..102a86dfaf 100644 --- a/packages/jaeger-ui/src/model/link-patterns.js +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -38,10 +38,7 @@ function stringSupplant(str, encodeFn: any => string, map) { return str.replace(parameterRegExp, (_, name) => encodeFn(map[name])); } -export function processTemplate( - template: any, - encodeFn: any => string -): ProcessedTemplate { +export function processTemplate(template: any, encodeFn: any => string): ProcessedTemplate { if (typeof template !== 'string') { /* @@ -194,23 +191,25 @@ export function computeLinks( return result; } -const linkPatterns = (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(Boolean); -const alreadyComputed = new WeakMap(); - -export default function getLinks( - trace: Trace, - spanIndex: number, - items: { key: string, value: any }[], - itemIndex: number +export function createGetLinks( + linkPatterns: ProcessedLinkPattern[], + cache: WeakMap<{ key: string, value: any }, { url: string, text: string }[]> ) { - if (linkPatterns.length === 0) { - return []; - } - const item = items[itemIndex]; - let result = alreadyComputed.get(item); - if (!result) { - result = computeLinks(linkPatterns, trace, spanIndex, items, itemIndex); - alreadyComputed.set(item, result); - } - return result; + return (trace: Trace, spanIndex: number, items: { key: string, value: any }[], itemIndex: number) => { + if (linkPatterns.length === 0) { + return []; + } + const item = items[itemIndex]; + let result = cache.get(item); + if (!result) { + result = computeLinks(linkPatterns, trace, spanIndex, items, itemIndex); + cache.set(item, result); + } + return result; + }; } + +export default createGetLinks( + (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(Boolean), + new WeakMap() +); diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 65e430325d..15f7f7f77e 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -19,6 +19,7 @@ import { getParameterInAncestor, processLinkPattern, computeLinks, + createGetLinks, } from './link-patterns'; describe('processTemplate()', () => { @@ -329,3 +330,55 @@ describe('computeLinks()', () => { ]); }); }); + +describe('getLinks()', () => { + const linkPatterns = [ + { + key: 'mySpecialKey', + // eslint-disable-next-line no-template-curly-in-string + url: 'http://example.com/?mySpecialKey=${mySpecialKey}', + // eslint-disable-next-line no-template-curly-in-string + text: 'special key link (${mySpecialKey})', + }, + ].map(processLinkPattern); + const template = jest.spyOn(linkPatterns[0].url, 'template'); + + const trace = { + spans: [{ depth: 0, process: {}, tags: [{ key: 'mySpecialKey', value: 'valueOfMyKey' }] }], + }; + + let cache; + + beforeEach(() => { + cache = new WeakMap(); + template.mockClear(); + }); + + it('does not access the cache if there is no link pattern', () => { + cache.get = jest.fn(); + const getLinks = createGetLinks([], cache); + expect(getLinks(trace, 0, trace.spans[0].tags, 0)).toEqual([]); + expect(cache.get).not.toHaveBeenCalled(); + }); + + it('returns the result from the cache', () => { + const result = []; + cache.set(trace.spans[0].tags[0], result); + const getLinks = createGetLinks(linkPatterns, cache); + expect(getLinks(trace, 0, trace.spans[0].tags, 0)).toBe(result); + expect(template).not.toHaveBeenCalled(); + }); + + it('adds the result to the cache', () => { + const getLinks = createGetLinks(linkPatterns, cache); + const result = getLinks(trace, 0, trace.spans[0].tags, 0); + expect(template).toHaveBeenCalledTimes(1); + expect(result).toEqual([ + { + url: 'http://example.com/?mySpecialKey=valueOfMyKey', + text: 'special key link (valueOfMyKey)', + }, + ]); + expect(cache.get(trace.spans[0].tags[0])).toBe(result); + }); +}); From 68c5d0502494c8dd21d3e5d35b7ac62de2045ec5 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Wed, 1 Aug 2018 11:50:08 +0200 Subject: [PATCH 15/18] Changes following code review Signed-off-by: David-Emmanuel Divernois --- .../SpanDetail/AccordianKeyValues.js | 7 ++- .../SpanDetail/AccordianLogs.js | 4 +- .../SpanDetail/KeyValuesTable.js | 7 ++- .../TraceTimelineViewer/SpanDetail/index.js | 4 +- .../TraceTimelineViewer/SpanDetailRow.js | 6 +- .../VirtualizedTraceView.js | 4 +- packages/jaeger-ui/src/model/link-patterns.js | 55 +++++++++---------- packages/jaeger-ui/src/types/index.js | 7 ++- 8 files changed, 50 insertions(+), 44 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js index aac3bbf240..94c46fe8d2 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js @@ -21,21 +21,22 @@ import IoIosArrowRight from 'react-icons/lib/io/ios-arrow-right'; import * as markers from './AccordianKeyValues.markers'; import KeyValuesTable from './KeyValuesTable'; +import type { KeyValuePair, Link } from '../../../../types'; import './AccordianKeyValues.css'; type AccordianKeyValuesProps = { className?: ?string, - data: { key: string, value: any }[], + data: KeyValuePair[], highContrast?: boolean, isOpen: boolean, label: string, - linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?(KeyValuePair[], number) => Link[], onToggle: () => void, }; // export for tests -export function KeyValuesSummary(props: { data?: { key: string, value: any }[] }) { +export function KeyValuesSummary(props: { data?: KeyValuePair[] }) { const { data } = props; if (!Array.isArray(data) || !data.length) { return null; diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js index 9b27d42fba..5f103977a8 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianLogs.js @@ -21,13 +21,13 @@ import IoIosArrowRight from 'react-icons/lib/io/ios-arrow-right'; import AccordianKeyValues from './AccordianKeyValues'; import { formatDuration } from '../utils'; -import type { Log } from '../../../../types'; +import type { Log, KeyValuePair, Link } from '../../../../types'; import './AccordianLogs.css'; type AccordianLogsProps = { isOpen: boolean, - linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?(KeyValuePair[], number) => Link[], logs: Log[], onItemToggle: Log => void, onToggle: () => void, diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js index 402ebf4da3..42728dc685 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.js @@ -17,6 +17,7 @@ import * as React from 'react'; import jsonMarkup from 'json-markup'; import { Dropdown, Icon, Menu } from 'antd'; +import type { KeyValuePair, Link } from '../../../../types'; import './KeyValuesTable.css'; @@ -37,7 +38,7 @@ const LinkValue = (props: { href: string, title?: string, children: React.Node } ); -const linkValueList = (links: { url: string, text: string }[]) => ( +const linkValueList = (links: Link[]) => ( {links.map(({ text, url }, index) => ( // `index` is necessary in the key because url can repeat @@ -50,8 +51,8 @@ const linkValueList = (links: { url: string, text: string }[]) => ( ); type KeyValuesTableProps = { - data: { key: string, value: any }[], - linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], + data: KeyValuePair[], + linksGetter: ?(KeyValuePair[], number) => Link[], }; export default function KeyValuesTable(props: KeyValuesTableProps) { diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js index 1c443d56f0..f97995783b 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js @@ -22,13 +22,13 @@ import AccordianLogs from './AccordianLogs'; import DetailState from './DetailState'; import { formatDuration } from '../utils'; import LabeledList from '../../../common/LabeledList'; -import type { Log, Span } from '../../../../types'; +import type { Log, Span, KeyValuePair, Link } from '../../../../types'; import './index.css'; type SpanDetailProps = { detailState: DetailState, - linksGetter: ?({ key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?(KeyValuePair[], number) => Link[], logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js index 303a3b5f67..7fca005eab 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js @@ -20,7 +20,7 @@ import SpanDetail from './SpanDetail'; import DetailState from './SpanDetail/DetailState'; import SpanTreeOffset from './SpanTreeOffset'; import TimelineRow from './TimelineRow'; -import type { Log, Span } from '../../../types'; +import type { Log, Span, KeyValuePair, Link } from '../../../types'; import './SpanDetailRow.css'; @@ -30,7 +30,7 @@ type SpanDetailRowProps = { detailState: DetailState, onDetailToggled: string => void, isFilteredOut: boolean, - linksGetter: ?(number, { key: string, value: any }[], number) => { url: string, text: string }[], + linksGetter: ?(number, KeyValuePair[], number) => Link[], logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, @@ -47,7 +47,7 @@ export default class SpanDetailRow extends React.PureComponent { + _linksGetter = (items: KeyValuePair[], itemIndex: number) => { const { linksGetter, spanIndex } = this.props; return linksGetter ? linksGetter(spanIndex, items, itemIndex) : []; }; diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js index 43b23a62d5..6c3481fb8d 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js @@ -33,7 +33,7 @@ import { } from './utils'; import getLinks from '../../../model/link-patterns'; import type { Accessors } from '../ScrollManager'; -import type { Log, Span, Trace } from '../../../types'; +import type { Log, Span, Trace, KeyValuePair } from '../../../types'; import colorGenerator from '../../../utils/color-generator'; import './VirtualizedTraceView.css'; @@ -265,7 +265,7 @@ export class VirtualizedTraceViewImpl extends React.PureComponent + linksGetter = (spanIndex: number, items: KeyValuePair[], itemIndex: number) => getLinks(this.props.trace, spanIndex, items, itemIndex); renderRow = (key: string, style: Style, index: number, attrs: {}) => { diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js index 102a86dfaf..ab4e651d18 100644 --- a/packages/jaeger-ui/src/model/link-patterns.js +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -16,26 +16,39 @@ import _uniq from 'lodash/uniq'; import { getConfigValue } from '../utils/config/get-config'; -import type { Span, Trace } from '../types'; +import type { Span, Trace, Link, KeyValuePair } from '../types'; const parameterRegExp = /\$\{([^{}]*)\}/g; type ProcessedTemplate = { parameters: string[], - template: (data: { [parameter: string]: any }) => string, + template: ({ [string]: mixed }) => string, +}; + +type ProcessedLinkPattern = { + object: any, + type: string => boolean, + key: string => boolean, + value: any => boolean, + url: ProcessedTemplate, + text: ProcessedTemplate, + parameters: string[], }; function getParamNames(str) { - const names = []; + const names = new Set(); str.replace(parameterRegExp, (match, name) => { - names.push(name); + names.add(name); return match; }); - return _uniq(names); + return Array.from(names); } function stringSupplant(str, encodeFn: any => string, map) { - return str.replace(parameterRegExp, (_, name) => encodeFn(map[name])); + return str.replace(parameterRegExp, (_, name) => { + const value = map[name]; + return value == null ? '' : encodeFn(value); + }); } export function processTemplate(template: any, encodeFn: any => string): ProcessedTemplate { @@ -82,16 +95,6 @@ export function createTestFunction(entry: any) { const identity = a => a; -type ProcessedLinkPattern = { - object: any, - type: string => boolean, - key: string => boolean, - value: any => boolean, - url: ProcessedTemplate, - text: ProcessedTemplate, - parameters: string[], -}; - export function processLinkPattern(pattern: any): ?ProcessedLinkPattern { try { const url = processTemplate(pattern.url, encodeURIComponent); @@ -112,7 +115,7 @@ export function processLinkPattern(pattern: any): ?ProcessedLinkPattern { } } -export function getParameterInArray(name: string, array: { key: string, value: any }[]) { +export function getParameterInArray(name: string, array: KeyValuePair[]) { if (array) { return array.find(entry => entry.key === name); } @@ -143,7 +146,7 @@ export function computeLinks( linkPatterns: ProcessedLinkPattern[], trace: Trace, spanIndex: number, - items: { key: string, value: any }[], + items: KeyValuePair[], itemIndex: number ) { const item = items[itemIndex]; @@ -160,15 +163,15 @@ export function computeLinks( const result = []; linkPatterns.forEach(pattern => { if (pattern.type(type) && pattern.key(item.key) && pattern.value(item.value)) { - let parameterValues = {}; - pattern.parameters.every(parameter => { + const parameterValues = {}; + const allParameters = pattern.parameters.every(parameter => { let entry = getParameterInArray(parameter, items); if (!entry && !processTags) { // do not look in ancestors for process tags because the same object may appear in different places in the hierarchy // and the cache in getLinks uses that object as a key entry = getParameterInAncestor(parameter, trace.spans, spanIndex); } - if (entry && parameterValues) { + if (entry) { parameterValues[parameter] = entry.value; return true; } @@ -177,10 +180,9 @@ export function computeLinks( `Skipping link pattern, missing parameter ${parameter} for key ${item.key} in ${type}.`, pattern.object ); - parameterValues = null; return false; }); - if (parameterValues) { + if (allParameters) { result.push({ url: callTemplate(pattern.url, parameterValues), text: callTemplate(pattern.text, parameterValues), @@ -191,11 +193,8 @@ export function computeLinks( return result; } -export function createGetLinks( - linkPatterns: ProcessedLinkPattern[], - cache: WeakMap<{ key: string, value: any }, { url: string, text: string }[]> -) { - return (trace: Trace, spanIndex: number, items: { key: string, value: any }[], itemIndex: number) => { +export function createGetLinks(linkPatterns: ProcessedLinkPattern[], cache: WeakMap) { + return (trace: Trace, spanIndex: number, items: KeyValuePair[], itemIndex: number) => { if (linkPatterns.length === 0) { return []; } diff --git a/packages/jaeger-ui/src/types/index.js b/packages/jaeger-ui/src/types/index.js index 55ca7d2028..33006c2049 100644 --- a/packages/jaeger-ui/src/types/index.js +++ b/packages/jaeger-ui/src/types/index.js @@ -18,11 +18,16 @@ * All timestamps are in microseconds */ -type KeyValuePair = { +export type KeyValuePair = { key: string, value: any, }; +export type Link = { + url: string, + text: string, +}; + export type Log = { timestamp: number, fields: Array, From 13d66cca58ba851fd6043d37c1bf2948c6911db5 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Wed, 1 Aug 2018 13:41:50 +0200 Subject: [PATCH 16/18] Using # instead of $ in link templates Signed-off-by: David-Emmanuel Divernois --- packages/jaeger-ui/src/model/link-patterns.js | 2 +- .../jaeger-ui/src/model/link-patterns.test.js | 24 +++++++------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js index ab4e651d18..ac8d7ce57d 100644 --- a/packages/jaeger-ui/src/model/link-patterns.js +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -18,7 +18,7 @@ import _uniq from 'lodash/uniq'; import { getConfigValue } from '../utils/config/get-config'; import type { Span, Trace, Link, KeyValuePair } from '../types'; -const parameterRegExp = /\$\{([^{}]*)\}/g; +const parameterRegExp = /#\{([^{}]*)\}/g; type ProcessedTemplate = { parameters: string[], diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 15f7f7f77e..22efb1b2bd 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -25,8 +25,7 @@ import { describe('processTemplate()', () => { it('correctly replaces variables', () => { const processedTemplate = processTemplate( - // eslint-disable-next-line no-template-curly-in-string - 'this is a test with ${oneVariable}${anotherVariable} and the same ${oneVariable}', + 'this is a test with #{oneVariable}#{anotherVariable} and the same #{oneVariable}', a => a ); expect(processedTemplate.parameters).toEqual(['oneVariable', 'anotherVariable']); @@ -37,8 +36,7 @@ describe('processTemplate()', () => { it('correctly uses the encoding function', () => { const processedTemplate = processTemplate( - // eslint-disable-next-line no-template-curly-in-string - 'this is a test with ${oneVariable}${anotherVariable} and the same ${oneVariable}', + 'this is a test with #{oneVariable}#{anotherVariable} and the same #{oneVariable}', e => `/${e}\\` ); expect(processedTemplate.parameters).toEqual(['oneVariable', 'anotherVariable']); @@ -294,17 +292,13 @@ describe('computeLinks()', () => { { type: 'tags', key: 'myKey', - // eslint-disable-next-line no-template-curly-in-string - url: 'http://example.com/?myKey=${myKey}', - // eslint-disable-next-line no-template-curly-in-string - text: 'first link (${myKey})', + url: 'http://example.com/?myKey=#{myKey}', + text: 'first link (#{myKey})', }, { key: 'myOtherKey', - // eslint-disable-next-line no-template-curly-in-string - url: 'http://example.com/?myKey=${myOtherKey}&myKey=${myKey}', - // eslint-disable-next-line no-template-curly-in-string - text: 'second link (${myOtherKey})', + url: 'http://example.com/?myKey=#{myOtherKey}&myKey=#{myKey}', + text: 'second link (#{myOtherKey})', }, ].map(processLinkPattern); @@ -335,10 +329,8 @@ describe('getLinks()', () => { const linkPatterns = [ { key: 'mySpecialKey', - // eslint-disable-next-line no-template-curly-in-string - url: 'http://example.com/?mySpecialKey=${mySpecialKey}', - // eslint-disable-next-line no-template-curly-in-string - text: 'special key link (${mySpecialKey})', + url: 'http://example.com/?mySpecialKey=#{mySpecialKey}', + text: 'special key link (#{mySpecialKey})', }, ].map(processLinkPattern); const template = jest.spyOn(linkPatterns[0].url, 'template'); From 6048d0aa8a5745f3aea3a1fc9c58fc2e09a9eb92 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Wed, 1 Aug 2018 16:05:53 +0200 Subject: [PATCH 17/18] Adding a reference to the parent span in the child spans Signed-off-by: David-Emmanuel Divernois --- .../TraceTimelineViewer/SpanDetailRow.js | 7 +- .../TraceTimelineViewer/SpanDetailRow.test.js | 5 +- .../VirtualizedTraceView.js | 8 +- packages/jaeger-ui/src/model/link-patterns.js | 32 +++---- .../jaeger-ui/src/model/link-patterns.test.js | 96 +++++++++++-------- packages/jaeger-ui/src/model/span.js | 27 ++++++ .../src/model/transform-trace-data.js | 18 ++++ packages/jaeger-ui/src/types/index.js | 2 + 8 files changed, 127 insertions(+), 68 deletions(-) create mode 100644 packages/jaeger-ui/src/model/span.js diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js index 7fca005eab..928c11a1ac 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js @@ -30,12 +30,11 @@ type SpanDetailRowProps = { detailState: DetailState, onDetailToggled: string => void, isFilteredOut: boolean, - linksGetter: ?(number, KeyValuePair[], number) => Link[], + linksGetter: ?(Span, KeyValuePair[], number) => Link[], logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, span: Span, - spanIndex: number, tagsToggle: string => void, traceStartTime: number, }; @@ -48,8 +47,8 @@ export default class SpanDetailRow extends React.PureComponent { - const { linksGetter, spanIndex } = this.props; - return linksGetter ? linksGetter(spanIndex, items, itemIndex) : []; + const { linksGetter, span } = this.props; + return linksGetter ? linksGetter(span, items, itemIndex) : []; }; render() { diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.test.js index d290784153..a5b36ffa1c 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.test.js @@ -33,7 +33,6 @@ describe('', () => { logsToggle: jest.fn(), processToggle: jest.fn(), span: { spanID, depth: 3 }, - spanIndex: 4, tagsToggle: jest.fn(), traceStartTime: 1000, }; @@ -87,7 +86,7 @@ describe('', () => { expect(wrapper.contains(spanDetail)).toBe(true); }); - it('adds spanIndex when calling linksGetter', () => { + it('adds span when calling linksGetter', () => { const spanDetail = wrapper.find(SpanDetail); const linksGetter = spanDetail.prop('linksGetter'); const tags = [{ key: 'myKey', value: 'myValue' }]; @@ -96,6 +95,6 @@ describe('', () => { const result = linksGetter(tags, 0); expect(result).toBe(linksGetterResponse); expect(props.linksGetter).toHaveBeenCalledTimes(1); - expect(props.linksGetter).toHaveBeenCalledWith(props.spanIndex, tags, 0); + expect(props.linksGetter).toHaveBeenCalledWith(props.span, tags, 0); }); }); diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js index 6c3481fb8d..cd3046df10 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js @@ -265,13 +265,12 @@ export class VirtualizedTraceViewImpl extends React.PureComponent - getLinks(this.props.trace, spanIndex, items, itemIndex); + linksGetter = (span: Span, items: KeyValuePair[], itemIndex: number) => getLinks(span, items, itemIndex); renderRow = (key: string, style: Style, index: number, attrs: {}) => { const { isDetail, span, spanIndex } = this.rowStates[index]; return isDetail - ? this.renderSpanDetailRow(span, spanIndex, key, style, attrs) + ? this.renderSpanDetailRow(span, key, style, attrs) : this.renderSpanBarRow(span, spanIndex, key, style, attrs); }; @@ -356,7 +355,7 @@ export class VirtualizedTraceViewImpl extends React.PureComponent diff --git a/packages/jaeger-ui/src/model/link-patterns.js b/packages/jaeger-ui/src/model/link-patterns.js index ac8d7ce57d..098806bf4f 100644 --- a/packages/jaeger-ui/src/model/link-patterns.js +++ b/packages/jaeger-ui/src/model/link-patterns.js @@ -16,7 +16,8 @@ import _uniq from 'lodash/uniq'; import { getConfigValue } from '../utils/config/get-config'; -import type { Span, Trace, Link, KeyValuePair } from '../types'; +import { getParent } from './span'; +import type { Span, Link, KeyValuePair } from '../types'; const parameterRegExp = /#\{([^{}]*)\}/g; @@ -122,18 +123,15 @@ export function getParameterInArray(name: string, array: KeyValuePair[]) { return undefined; } -export function getParameterInAncestor(name: string, spans: Span[], startSpanIndex: number) { - let currentSpan = { depth: spans[startSpanIndex].depth + 1 }; - for (let spanIndex = startSpanIndex; spanIndex >= 0 && currentSpan.depth > 0; spanIndex--) { - const nextSpan = spans[spanIndex]; - if (nextSpan.depth < currentSpan.depth) { - currentSpan = nextSpan; - const result = - getParameterInArray(name, currentSpan.tags) || getParameterInArray(name, currentSpan.process.tags); - if (result) { - return result; - } +export function getParameterInAncestor(name: string, span: Span) { + let currentSpan = span; + while (currentSpan) { + const result = + getParameterInArray(name, currentSpan.tags) || getParameterInArray(name, currentSpan.process.tags); + if (result) { + return result; } + currentSpan = getParent(currentSpan); } return undefined; } @@ -144,13 +142,11 @@ function callTemplate(template, data) { export function computeLinks( linkPatterns: ProcessedLinkPattern[], - trace: Trace, - spanIndex: number, + span: Span, items: KeyValuePair[], itemIndex: number ) { const item = items[itemIndex]; - const span = trace.spans[spanIndex]; let type = 'logs'; const processTags = span.process.tags === items; if (processTags) { @@ -169,7 +165,7 @@ export function computeLinks( if (!entry && !processTags) { // do not look in ancestors for process tags because the same object may appear in different places in the hierarchy // and the cache in getLinks uses that object as a key - entry = getParameterInAncestor(parameter, trace.spans, spanIndex); + entry = getParameterInAncestor(parameter, span); } if (entry) { parameterValues[parameter] = entry.value; @@ -194,14 +190,14 @@ export function computeLinks( } export function createGetLinks(linkPatterns: ProcessedLinkPattern[], cache: WeakMap) { - return (trace: Trace, spanIndex: number, items: KeyValuePair[], itemIndex: number) => { + return (span: Span, items: KeyValuePair[], itemIndex: number) => { if (linkPatterns.length === 0) { return []; } const item = items[itemIndex]; let result = cache.get(item); if (!result) { - result = computeLinks(linkPatterns, trace, spanIndex, items, itemIndex); + result = computeLinks(linkPatterns, span, items, itemIndex); cache.set(item, result); } return result; diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 22efb1b2bd..8694c8f0ff 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -233,47 +233,65 @@ describe('getParameterInAncestor()', () => { tags: [{ key: 'a', value: 'a0' }], }, ]; + spans[1].references = [ + { + refType: 'CHILD_OF', + span: spans[0], + }, + ]; + spans[2].references = [ + { + refType: 'CHILD_OF', + span: spans[0], + }, + ]; + spans[3].references = [ + { + refType: 'CHILD_OF', + span: spans[2], + }, + ]; it('uses current span tags', () => { - expect(getParameterInAncestor('a', spans, 3)).toEqual({ key: 'a', value: 'a0' }); - expect(getParameterInAncestor('a', spans, 2)).toEqual({ key: 'a', value: 'a2' }); - expect(getParameterInAncestor('a', spans, 1)).toEqual({ key: 'a', value: 'a4' }); - expect(getParameterInAncestor('a', spans, 0)).toEqual({ key: 'a', value: 'a6' }); + expect(getParameterInAncestor('a', spans[3])).toEqual({ key: 'a', value: 'a0' }); + expect(getParameterInAncestor('a', spans[2])).toEqual({ key: 'a', value: 'a2' }); + expect(getParameterInAncestor('a', spans[1])).toEqual({ key: 'a', value: 'a4' }); + expect(getParameterInAncestor('a', spans[0])).toEqual({ key: 'a', value: 'a6' }); }); it('uses current span process tags', () => { - expect(getParameterInAncestor('b', spans, 3)).toEqual({ key: 'b', value: 'b1' }); - expect(getParameterInAncestor('d', spans, 2)).toEqual({ key: 'd', value: 'd3' }); - expect(getParameterInAncestor('f', spans, 1)).toEqual({ key: 'f', value: 'f5' }); - expect(getParameterInAncestor('h', spans, 0)).toEqual({ key: 'h', value: 'h7' }); + expect(getParameterInAncestor('b', spans[3])).toEqual({ key: 'b', value: 'b1' }); + expect(getParameterInAncestor('d', spans[2])).toEqual({ key: 'd', value: 'd3' }); + expect(getParameterInAncestor('f', spans[1])).toEqual({ key: 'f', value: 'f5' }); + expect(getParameterInAncestor('h', spans[0])).toEqual({ key: 'h', value: 'h7' }); }); it('uses parent span tags', () => { - expect(getParameterInAncestor('c', spans, 3)).toEqual({ key: 'c', value: 'c2' }); - expect(getParameterInAncestor('e', spans, 2)).toEqual({ key: 'e', value: 'e6' }); - expect(getParameterInAncestor('f', spans, 2)).toEqual({ key: 'f', value: 'f6' }); - expect(getParameterInAncestor('g', spans, 2)).toEqual({ key: 'g', value: 'g6' }); - expect(getParameterInAncestor('g', spans, 1)).toEqual({ key: 'g', value: 'g6' }); + expect(getParameterInAncestor('c', spans[3])).toEqual({ key: 'c', value: 'c2' }); + expect(getParameterInAncestor('e', spans[2])).toEqual({ key: 'e', value: 'e6' }); + expect(getParameterInAncestor('f', spans[2])).toEqual({ key: 'f', value: 'f6' }); + expect(getParameterInAncestor('g', spans[2])).toEqual({ key: 'g', value: 'g6' }); + expect(getParameterInAncestor('g', spans[1])).toEqual({ key: 'g', value: 'g6' }); }); it('uses parent span process tags', () => { - expect(getParameterInAncestor('d', spans, 3)).toEqual({ key: 'd', value: 'd3' }); - expect(getParameterInAncestor('h', spans, 2)).toEqual({ key: 'h', value: 'h7' }); - expect(getParameterInAncestor('h', spans, 1)).toEqual({ key: 'h', value: 'h7' }); + expect(getParameterInAncestor('d', spans[3])).toEqual({ key: 'd', value: 'd3' }); + expect(getParameterInAncestor('h', spans[2])).toEqual({ key: 'h', value: 'h7' }); + expect(getParameterInAncestor('h', spans[1])).toEqual({ key: 'h', value: 'h7' }); }); it('uses grand-parent span tags', () => { - expect(getParameterInAncestor('e', spans, 3)).toEqual({ key: 'e', value: 'e6' }); - expect(getParameterInAncestor('f', spans, 3)).toEqual({ key: 'f', value: 'f6' }); - expect(getParameterInAncestor('g', spans, 3)).toEqual({ key: 'g', value: 'g6' }); + expect(getParameterInAncestor('e', spans[3])).toEqual({ key: 'e', value: 'e6' }); + expect(getParameterInAncestor('f', spans[3])).toEqual({ key: 'f', value: 'f6' }); + expect(getParameterInAncestor('g', spans[3])).toEqual({ key: 'g', value: 'g6' }); }); it('uses grand-parent process tags', () => { - expect(getParameterInAncestor('h', spans, 3)).toEqual({ key: 'h', value: 'h7' }); + expect(getParameterInAncestor('h', spans[3])).toEqual({ key: 'h', value: 'h7' }); }); it('returns undefined when the entry cannot be found', () => { - expect(getParameterInAncestor('i', spans, 3)).toBeUndefined(); + expect(getParameterInAncestor('i', spans[3])).toBeUndefined(); }); it('does not break if some tags are not defined', () => { @@ -283,7 +301,7 @@ describe('getParameterInAncestor()', () => { process: {}, }, ]; - expect(getParameterInAncestor('a', spansWithUndefinedTags, 0)).toBeUndefined(); + expect(getParameterInAncestor('a', spansWithUndefinedTags[0])).toBeUndefined(); }); }); @@ -302,21 +320,25 @@ describe('computeLinks()', () => { }, ].map(processLinkPattern); - const trace = { - spans: [ - { depth: 0, process: {}, tags: [{ key: 'myKey', value: 'valueOfMyKey' }] }, - { depth: 1, process: {}, logs: [{ fields: [{ key: 'myOtherKey', value: 'valueOfMy+Other+Key' }] }] }, - ], - }; + const spans = [ + { depth: 0, process: {}, tags: [{ key: 'myKey', value: 'valueOfMyKey' }] }, + { depth: 1, process: {}, logs: [{ fields: [{ key: 'myOtherKey', value: 'valueOfMy+Other+Key' }] }] }, + ]; + spans[1].references = [ + { + refType: 'CHILD_OF', + span: spans[0], + }, + ]; it('correctly computes links', () => { - expect(computeLinks(linkPatterns, trace, 0, trace.spans[0].tags, 0)).toEqual([ + expect(computeLinks(linkPatterns, spans[0], spans[0].tags, 0)).toEqual([ { url: 'http://example.com/?myKey=valueOfMyKey', text: 'first link (valueOfMyKey)', }, ]); - expect(computeLinks(linkPatterns, trace, 1, trace.spans[1].logs[0].fields, 0)).toEqual([ + expect(computeLinks(linkPatterns, spans[1], spans[1].logs[0].fields, 0)).toEqual([ { url: 'http://example.com/?myKey=valueOfMy%2BOther%2BKey&myKey=valueOfMyKey', text: 'second link (valueOfMy+Other+Key)', @@ -335,9 +357,7 @@ describe('getLinks()', () => { ].map(processLinkPattern); const template = jest.spyOn(linkPatterns[0].url, 'template'); - const trace = { - spans: [{ depth: 0, process: {}, tags: [{ key: 'mySpecialKey', value: 'valueOfMyKey' }] }], - }; + const span = { depth: 0, process: {}, tags: [{ key: 'mySpecialKey', value: 'valueOfMyKey' }] }; let cache; @@ -349,21 +369,21 @@ describe('getLinks()', () => { it('does not access the cache if there is no link pattern', () => { cache.get = jest.fn(); const getLinks = createGetLinks([], cache); - expect(getLinks(trace, 0, trace.spans[0].tags, 0)).toEqual([]); + expect(getLinks(span, span.tags, 0)).toEqual([]); expect(cache.get).not.toHaveBeenCalled(); }); it('returns the result from the cache', () => { const result = []; - cache.set(trace.spans[0].tags[0], result); + cache.set(span.tags[0], result); const getLinks = createGetLinks(linkPatterns, cache); - expect(getLinks(trace, 0, trace.spans[0].tags, 0)).toBe(result); + expect(getLinks(span, span.tags, 0)).toBe(result); expect(template).not.toHaveBeenCalled(); }); it('adds the result to the cache', () => { const getLinks = createGetLinks(linkPatterns, cache); - const result = getLinks(trace, 0, trace.spans[0].tags, 0); + const result = getLinks(span, span.tags, 0); expect(template).toHaveBeenCalledTimes(1); expect(result).toEqual([ { @@ -371,6 +391,6 @@ describe('getLinks()', () => { text: 'special key link (valueOfMyKey)', }, ]); - expect(cache.get(trace.spans[0].tags[0])).toBe(result); + expect(cache.get(span.tags[0])).toBe(result); }); }); diff --git a/packages/jaeger-ui/src/model/span.js b/packages/jaeger-ui/src/model/span.js new file mode 100644 index 0000000000..370d5945cd --- /dev/null +++ b/packages/jaeger-ui/src/model/span.js @@ -0,0 +1,27 @@ +// @flow + +// Copyright (c) 2017 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import type { Span } from '../types'; + +/** + * Searches the span.references to find 'CHILD_OF' reference type or returns null. + * @param {Span} span The span whose parent is to be returned. + * @return {Span|null} The parent span if there is one, null otherwise. + */ +export function getParent(span: Span) { + const parentRef = span.references ? span.references.find(ref => ref.refType === 'CHILD_OF') : null; + return parentRef ? parentRef.span : null; +} diff --git a/packages/jaeger-ui/src/model/transform-trace-data.js b/packages/jaeger-ui/src/model/transform-trace-data.js index 4b1a5fe1ed..314d43ea15 100644 --- a/packages/jaeger-ui/src/model/transform-trace-data.js +++ b/packages/jaeger-ui/src/model/transform-trace-data.js @@ -21,6 +21,21 @@ import type { Process, Span, SpanData, Trace, TraceData } from '../types'; type SpanWithProcess = SpanData & { process: Process }; +export function addSpanReferences(spans: Span[]) { + const spansMap = new Map(); + spans.forEach(span => spansMap.set(span.spanID, span)); + spans.forEach(span => { + span.references.forEach(ref => { + const refSpan = spansMap.get(ref.spanID); + if (refSpan) { + // eslint-disable-next-line no-param-reassign + ref.span = refSpan; + } + }); + }); + return spans; +} + /** * NOTE: Mutates `data` - Transform the HTTP response data into the form the app * generally requires. @@ -101,6 +116,9 @@ export default function transfromTraceData(data: TraceData & { spans: SpanWithPr traceID: span.traceID, }); }); + + addSpanReferences(spans); + return { spans, traceID, diff --git a/packages/jaeger-ui/src/types/index.js b/packages/jaeger-ui/src/types/index.js index 33006c2049..f6c6a61ee2 100644 --- a/packages/jaeger-ui/src/types/index.js +++ b/packages/jaeger-ui/src/types/index.js @@ -40,6 +40,8 @@ export type Process = { export type SpanReference = { refType: 'CHILD_OF' | 'FOLLOWS_FROM', + // eslint-disable-next-line no-use-before-define + span: Span, spanID: string, traceID: string, }; From 185c93ed4258d39b12b1e50d8adbfe296d670514 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Thu, 2 Aug 2018 10:47:37 +0200 Subject: [PATCH 18/18] Merging addSpanReferences with transfromTraceData and making span optional in SpanReference Signed-off-by: David-Emmanuel Divernois --- .../src/model/transform-trace-data.js | 44 +++++-------------- packages/jaeger-ui/src/types/index.js | 2 +- 2 files changed, 12 insertions(+), 34 deletions(-) diff --git a/packages/jaeger-ui/src/model/transform-trace-data.js b/packages/jaeger-ui/src/model/transform-trace-data.js index 314d43ea15..8595261b20 100644 --- a/packages/jaeger-ui/src/model/transform-trace-data.js +++ b/packages/jaeger-ui/src/model/transform-trace-data.js @@ -21,21 +21,6 @@ import type { Process, Span, SpanData, Trace, TraceData } from '../types'; type SpanWithProcess = SpanData & { process: Process }; -export function addSpanReferences(spans: Span[]) { - const spansMap = new Map(); - spans.forEach(span => spansMap.set(span.spanID, span)); - spans.forEach(span => { - span.references.forEach(ref => { - const refSpan = spansMap.get(ref.spanID); - if (refSpan) { - // eslint-disable-next-line no-param-reassign - ref.span = refSpan; - } - }); - }); - return spans; -} - /** * NOTE: Mutates `data` - Transform the HTTP response data into the form the app * generally requires. @@ -95,30 +80,23 @@ export default function transfromTraceData(data: TraceData & { spans: SpanWithPr if (spanID === '__root__') { return; } - const span: ?SpanWithProcess = spanMap.get(spanID); + const span: ?Span = (spanMap.get(spanID): any); if (!span) { return; } - spans.push({ - relativeStartTime: span.startTime - traceStartTime, - depth: depth - 1, - hasChildren: node.children.length > 0, - // spread fails with union types - duration: span.duration, - logs: span.logs, - operationName: span.operationName, - process: span.process, - processID: span.processID, - references: span.references, - spanID: span.spanID, - startTime: span.startTime, - tags: span.tags, - traceID: span.traceID, + span.relativeStartTime = span.startTime - traceStartTime; + span.depth = depth - 1; + span.hasChildren = node.children.length > 0; + span.references.forEach(ref => { + const refSpan: ?Span = (spanMap.get(ref.spanID): any); + if (refSpan) { + // eslint-disable-next-line no-param-reassign + ref.span = refSpan; + } }); + spans.push(span); }); - addSpanReferences(spans); - return { spans, traceID, diff --git a/packages/jaeger-ui/src/types/index.js b/packages/jaeger-ui/src/types/index.js index f6c6a61ee2..09b6f100b1 100644 --- a/packages/jaeger-ui/src/types/index.js +++ b/packages/jaeger-ui/src/types/index.js @@ -41,7 +41,7 @@ export type Process = { export type SpanReference = { refType: 'CHILD_OF' | 'FOLLOWS_FROM', // eslint-disable-next-line no-use-before-define - span: Span, + span: ?Span, spanID: string, traceID: string, };