From c00399d09b71c9811a9c80a78aef3a49b5c08e62 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Fri, 1 Nov 2019 10:13:07 -0600 Subject: [PATCH 01/10] Support trace-scoped external links similar to tag links Signed-off-by: Ruben Vargas --- .../TracePageHeader/TracePageHeader.tsx | 5 ++ .../src/components/common/ExternalLinks.tsx | 71 +++++++++++++++++++ .../jaeger-ui/src/model/link-patterns.tsx | 50 ++++++++++++- 3 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 packages/jaeger-ui/src/components/common/ExternalLinks.tsx diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx index ec4e9f2611..a220988deb 100644 --- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx +++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx @@ -34,8 +34,10 @@ import { getTraceName } from '../../../model/trace-viewer'; import { TNil } from '../../../types'; import { Trace } from '../../../types/trace'; import { formatDatetime, formatDuration } from '../../../utils/date'; +import { getTraceLinks } from '../../../model/link-patterns'; import './TracePageHeader.css'; +import ExternalLinks from '../../common/ExternalLinks'; type TracePageHeaderEmbedProps = { canCollapse: boolean; @@ -136,6 +138,8 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded return null; } + const links = getTraceLinks(trace); + const summaryItems = !hideSummary && !slimView && @@ -159,6 +163,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded )} + {links && links.length > 0 && } {canCollapse ? ( ( + + {props.children} + +); + +// export for testing +export const linkValueList = (links: Link[]) => ( + + {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} + + ))} + +); + +export default function ExternalLinks(props: ExternalLinksProps) { + const { links, className } = props; + if (links.length === 1) { + return ( + + {props.children} + + ); + } + return ( + + + {props.children} + + + ); +} diff --git a/packages/jaeger-ui/src/model/link-patterns.tsx b/packages/jaeger-ui/src/model/link-patterns.tsx index 8fcb6042ca..b5a42449e6 100644 --- a/packages/jaeger-ui/src/model/link-patterns.tsx +++ b/packages/jaeger-ui/src/model/link-patterns.tsx @@ -16,7 +16,7 @@ import _uniq from 'lodash/uniq'; import { getConfigValue } from '../utils/config/get-config'; import { getParent } from './span'; import { TNil } from '../types'; -import { Span, Link, KeyValuePair } from '../types/trace'; +import { Span, Link, KeyValuePair, Trace } from '../types/trace'; const parameterRegExp = /#\{([^{}]*)\}/g; @@ -139,6 +139,35 @@ function callTemplate(template: ProcessedTemplate, data: any) { return template.template(data); } +export function computeTraceLinks(linkPatterns: ProcessedLinkPattern[], trace: Trace) { + const result: { url: string; text: string }[] = []; + const validKeys = Object.keys(trace) + // @ts-ignore + .filter(key => typeof trace[key] === 'string' || typeof trace[key] === 'number'); + + linkPatterns + .filter(pattern => pattern.type('trace')) + .forEach(pattern => { + const parameterValues: Record = {}; + const allParameters = pattern.parameters.every(parameter => { + if (validKeys.some(name => name === parameter)) { + // @ts-ignore + parameterValues[parameter] = trace[parameter]; + return true; + } + return false; + }); + if (allParameters) { + result.push({ + url: callTemplate(pattern.url, parameterValues), + text: callTemplate(pattern.text, parameterValues), + }); + } + }); + + return result; +} + export function computeLinks( linkPatterns: ProcessedLinkPattern[], span: Span, @@ -188,6 +217,20 @@ export function computeLinks( return result; } +export function createGetTraceLinks(linkPatterns: ProcessedLinkPattern[], cache: WeakMap) { + return (trace: Trace | undefined) => { + if (!trace || linkPatterns.length === 0) { + return []; + } + let result = cache.get(trace); + if (!result) { + result = computeTraceLinks(linkPatterns, trace); + cache.set(trace, result); + } + return result; + }; +} + export function createGetLinks(linkPatterns: ProcessedLinkPattern[], cache: WeakMap) { return (span: Span, items: KeyValuePair[], itemIndex: number) => { if (linkPatterns.length === 0) { @@ -207,3 +250,8 @@ export default createGetLinks( (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(Boolean), new WeakMap() ); + +export const getTraceLinks = createGetTraceLinks( + (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(Boolean), + new WeakMap() +); From 56194fc910fecf08d234000d44be1f7e653f0dde Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Tue, 12 Nov 2019 10:22:21 -0600 Subject: [PATCH 02/10] ExternalLinks component test Signed-off-by: Ruben Vargas --- .../components/common/ExternalLinks.test.js | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 packages/jaeger-ui/src/components/common/ExternalLinks.test.js diff --git a/packages/jaeger-ui/src/components/common/ExternalLinks.test.js b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js new file mode 100644 index 0000000000..8d1882391e --- /dev/null +++ b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js @@ -0,0 +1,47 @@ +// Copyright (c) 2017 Uber Technologies, Inc. +// +// 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 React from 'react'; +import { shallow, mount } from 'enzyme'; +import ExternalLinks from './ExternalLinks'; + +describe('', () => { + describe('render links links', () => { + it('renders dropdown with multiple links', () => { + const links = [ + { url: 'http://nowhere/', text: 'some text' }, + { url: 'http://other/', text: 'other text' }, + { url: 'http://link/', text: 'link text' }, + ]; + + const wrapper = shallow(); + const dropdown = wrapper.find('Dropdown'); + expect(dropdown.length).toBe(1); + }); + + it('renders one link', () => { + const links = [{ url: 'http://nowhere/', text: 'some text' }]; + const wrapper = shallow(); + const dropdown = wrapper.find('Dropdown'); + expect(dropdown.length).toBe(0); + expect(wrapper.find('LinkValue').length).toBe(1); + }); + }); + + it('is fine when mounted', () => { + const links = [{ url: 'http://nowhere/', text: 'some text' }]; + const wrapper = mount(); + expect(wrapper).toBeDefined(); + }); +}); From 4b5793b26e51e1cef71f89c204e9d6b1d12cf60a Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Tue, 12 Nov 2019 14:50:30 -0600 Subject: [PATCH 03/10] Test trace links patterns Signed-off-by: Ruben Vargas --- .../jaeger-ui/src/model/link-patterns.test.js | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 8694c8f0ff..c81dfe180b 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -20,6 +20,8 @@ import { processLinkPattern, computeLinks, createGetLinks, + computeTraceLinks, + createGetTraceLinks, } from './link-patterns'; describe('processTemplate()', () => { @@ -305,6 +307,40 @@ describe('getParameterInAncestor()', () => { }); }); +describe('computeTraceLinks()', () => { + const linkPatterns = [ + { + type: 'trace', + url: 'http://example.com/?myKey=#{traceID}', + text: 'first link (#{traceID})', + }, + { + type: 'trace', + url: 'http://example.com/?myKey=#{myOtherKey}&myKey=#{myKey}', + text: 'second link (#{myOtherKey})', + }, + ].map(processLinkPattern); + + const trace = { + processes: [], + traceID: 'trc1', + spans: [], + startTime: 1000, + endTime: 2000, + duration: 1000, + services: [], + }; + + it('correctly computes links', () => { + expect(computeTraceLinks(linkPatterns, trace)).toEqual([ + { + url: 'http://example.com/?myKey=trc1', + text: 'first link (trc1)', + }, + ]); + }); +}); + describe('computeLinks()', () => { const linkPatterns = [ { @@ -394,3 +430,59 @@ describe('getLinks()', () => { expect(cache.get(span.tags[0])).toBe(result); }); }); + +describe('getTraceLinks()', () => { + const linkPatterns = [ + { + type: 'trace', + url: 'http://example.com/#{traceID}', + text: 'special key link (#{traceID})', + }, + ].map(processLinkPattern); + const template = jest.spyOn(linkPatterns[0].url, 'template'); + + const trace = { + processes: [], + traceID: 'trc1', + spans: [], + startTime: 1000, + endTime: 2000, + duration: 1000, + services: [], + }; + + 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 getTraceLinks = createGetTraceLinks([], cache); + expect(getTraceLinks(trace)).toEqual([]); + expect(cache.get).not.toHaveBeenCalled(); + }); + + it('returns the result from the cache', () => { + const result = []; + cache.set(trace, result); + const getTraceLinks = createGetTraceLinks(linkPatterns, cache); + expect(getTraceLinks(trace)).toBe(result); + expect(template).not.toHaveBeenCalled(); + }); + + it('adds the result to the cache', () => { + const getTraceLinks = createGetTraceLinks(linkPatterns, cache); + const result = getTraceLinks(trace); + expect(template).toHaveBeenCalledTimes(1); + expect(result).toEqual([ + { + url: 'http://example.com/trc1', + text: 'special key link (trc1)', + }, + ]); + expect(cache.get(trace)).toBe(result); + }); +}); From 964593ba72a8cfedec5ab175c7cf1648b329c715 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Sun, 17 Nov 2019 12:06:43 -0600 Subject: [PATCH 04/10] Add LinkPatternsConfig type to config.tsx Signed-off-by: Ruben Vargas --- packages/jaeger-ui/src/model/link-patterns.test.js | 6 +++--- packages/jaeger-ui/src/model/link-patterns.tsx | 2 +- packages/jaeger-ui/src/types/config.tsx | 8 ++++++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index c81dfe180b..8632956844 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -310,12 +310,12 @@ describe('getParameterInAncestor()', () => { describe('computeTraceLinks()', () => { const linkPatterns = [ { - type: 'trace', + type: 'traces', url: 'http://example.com/?myKey=#{traceID}', text: 'first link (#{traceID})', }, { - type: 'trace', + type: 'traces', url: 'http://example.com/?myKey=#{myOtherKey}&myKey=#{myKey}', text: 'second link (#{myOtherKey})', }, @@ -434,7 +434,7 @@ describe('getLinks()', () => { describe('getTraceLinks()', () => { const linkPatterns = [ { - type: 'trace', + type: 'traces', url: 'http://example.com/#{traceID}', text: 'special key link (#{traceID})', }, diff --git a/packages/jaeger-ui/src/model/link-patterns.tsx b/packages/jaeger-ui/src/model/link-patterns.tsx index b5a42449e6..ed38bf6685 100644 --- a/packages/jaeger-ui/src/model/link-patterns.tsx +++ b/packages/jaeger-ui/src/model/link-patterns.tsx @@ -146,7 +146,7 @@ export function computeTraceLinks(linkPatterns: ProcessedLinkPattern[], trace: T .filter(key => typeof trace[key] === 'string' || typeof trace[key] === 'number'); linkPatterns - .filter(pattern => pattern.type('trace')) + .filter(pattern => pattern.type('traces')) .forEach(pattern => { const parameterValues: Record = {}; const allParameters = pattern.parameters.every(parameter => { diff --git a/packages/jaeger-ui/src/types/config.tsx b/packages/jaeger-ui/src/types/config.tsx index 683c82a134..50de4fde77 100644 --- a/packages/jaeger-ui/src/types/config.tsx +++ b/packages/jaeger-ui/src/types/config.tsx @@ -30,6 +30,13 @@ export type TScript = { type: 'inline'; }; +export type LinkPatternsConfig = { + type: 'process' | 'tags' | 'logs' | 'traces'; + key?: string; + url: string; + text: string; +}; + export type Config = { archiveEnabled?: boolean; dependencies?: { dagMaxServicesLen?: number; menuEnabled?: boolean }; @@ -41,4 +48,5 @@ export type Config = { gaID: string | TNil; trackErrors: boolean | TNil; }; + linkPatterns?: LinkPatternsConfig; }; From aff3db30605dc24493a41402e7924a013e66bf2f Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Fri, 6 Dec 2019 11:06:06 -0600 Subject: [PATCH 05/10] Remove unnecessary props from ExternalLinks Signed-off-by: Ruben Vargas --- .../TracePage/TracePageHeader/TracePageHeader.tsx | 2 +- .../src/components/common/ExternalLinks.test.js | 2 +- .../src/components/common/ExternalLinks.tsx | 15 +++++---------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx index a220988deb..7456debdbe 100644 --- a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx +++ b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx @@ -163,7 +163,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded )} - {links && links.length > 0 && } + {links && links.length > 0 && } {canCollapse ? ( ( ( ); export default function ExternalLinks(props: ExternalLinksProps) { - const { links, className } = props; + const { links } = props; if (links.length === 1) { - return ( - - {props.children} - - ); + return ; } return ( - - {props.children} + + ); From 824c49f41e7f219f13719bb317d33bd65a5fa9ea Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Fri, 6 Dec 2019 11:08:28 -0600 Subject: [PATCH 06/10] Remove mount test from ExternalLinks.test.js Signed-off-by: Ruben Vargas --- .../jaeger-ui/src/components/common/ExternalLinks.test.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/jaeger-ui/src/components/common/ExternalLinks.test.js b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js index b857172273..32d70c995f 100644 --- a/packages/jaeger-ui/src/components/common/ExternalLinks.test.js +++ b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js @@ -13,7 +13,7 @@ // limitations under the License. import React from 'react'; -import { shallow, mount } from 'enzyme'; +import { shallow } from 'enzyme'; import ExternalLinks from './ExternalLinks'; describe('', () => { @@ -38,10 +38,4 @@ describe('', () => { expect(wrapper.find('LinkValue').length).toBe(1); }); }); - - it('is fine when mounted', () => { - const links = [{ url: 'http://nowhere/', text: 'some text' }]; - const wrapper = mount(); - expect(wrapper).toBeDefined(); - }); }); From f0b1ce188c7607155e5dc71c847aa5d2b817e20a Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Fri, 6 Dec 2019 12:20:11 -0600 Subject: [PATCH 07/10] Improve computeTraceLinks and replace bespoke cache with memoize Signed-off-by: Ruben Vargas --- .../components/common/ExternalLinks.test.js | 7 +- .../src/components/common/ExternalLinks.tsx | 2 +- .../jaeger-ui/src/model/link-patterns.test.js | 67 ++----------------- .../jaeger-ui/src/model/link-patterns.tsx | 56 +++++++--------- 4 files changed, 36 insertions(+), 96 deletions(-) diff --git a/packages/jaeger-ui/src/components/common/ExternalLinks.test.js b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js index 32d70c995f..e765caff45 100644 --- a/packages/jaeger-ui/src/components/common/ExternalLinks.test.js +++ b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js @@ -1,4 +1,4 @@ -// Copyright (c) 2019 Uber Technologies, Inc. +// Copyright (c) 2019 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. @@ -35,7 +35,10 @@ describe('', () => { const wrapper = shallow(); const dropdown = wrapper.find('Dropdown'); expect(dropdown.length).toBe(0); - expect(wrapper.find('LinkValue').length).toBe(1); + const linkValues = wrapper.find('LinkValue'); + expect(linkValues.length).toBe(1); + expect(linkValues.prop('href')).toBe(links[0].url); + expect(linkValues.prop('title')).toBe(links[0].text); }); }); }); diff --git a/packages/jaeger-ui/src/components/common/ExternalLinks.tsx b/packages/jaeger-ui/src/components/common/ExternalLinks.tsx index 2de6b3d6fa..f0906ad97b 100644 --- a/packages/jaeger-ui/src/components/common/ExternalLinks.tsx +++ b/packages/jaeger-ui/src/components/common/ExternalLinks.tsx @@ -1,4 +1,4 @@ -// Copyright (c) 2019 Uber Technologies, Inc. +// Copyright (c) 2019 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. diff --git a/packages/jaeger-ui/src/model/link-patterns.test.js b/packages/jaeger-ui/src/model/link-patterns.test.js index 8632956844..a92961a6d4 100644 --- a/packages/jaeger-ui/src/model/link-patterns.test.js +++ b/packages/jaeger-ui/src/model/link-patterns.test.js @@ -20,8 +20,7 @@ import { processLinkPattern, computeLinks, createGetLinks, - computeTraceLinks, - createGetTraceLinks, + computeTraceLink, } from './link-patterns'; describe('processTemplate()', () => { @@ -307,7 +306,7 @@ describe('getParameterInAncestor()', () => { }); }); -describe('computeTraceLinks()', () => { +describe('computeTraceLink()', () => { const linkPatterns = [ { type: 'traces', @@ -316,8 +315,8 @@ describe('computeTraceLinks()', () => { }, { type: 'traces', - url: 'http://example.com/?myKey=#{myOtherKey}&myKey=#{myKey}', - text: 'second link (#{myOtherKey})', + url: 'http://example.com/?myKey=#{traceID}&myKey=#{myKey}', + text: 'second link (#{myKey})', }, ].map(processLinkPattern); @@ -332,7 +331,7 @@ describe('computeTraceLinks()', () => { }; it('correctly computes links', () => { - expect(computeTraceLinks(linkPatterns, trace)).toEqual([ + expect(computeTraceLink(linkPatterns, trace)).toEqual([ { url: 'http://example.com/?myKey=trc1', text: 'first link (trc1)', @@ -430,59 +429,3 @@ describe('getLinks()', () => { expect(cache.get(span.tags[0])).toBe(result); }); }); - -describe('getTraceLinks()', () => { - const linkPatterns = [ - { - type: 'traces', - url: 'http://example.com/#{traceID}', - text: 'special key link (#{traceID})', - }, - ].map(processLinkPattern); - const template = jest.spyOn(linkPatterns[0].url, 'template'); - - const trace = { - processes: [], - traceID: 'trc1', - spans: [], - startTime: 1000, - endTime: 2000, - duration: 1000, - services: [], - }; - - 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 getTraceLinks = createGetTraceLinks([], cache); - expect(getTraceLinks(trace)).toEqual([]); - expect(cache.get).not.toHaveBeenCalled(); - }); - - it('returns the result from the cache', () => { - const result = []; - cache.set(trace, result); - const getTraceLinks = createGetTraceLinks(linkPatterns, cache); - expect(getTraceLinks(trace)).toBe(result); - expect(template).not.toHaveBeenCalled(); - }); - - it('adds the result to the cache', () => { - const getTraceLinks = createGetTraceLinks(linkPatterns, cache); - const result = getTraceLinks(trace); - expect(template).toHaveBeenCalledTimes(1); - expect(result).toEqual([ - { - url: 'http://example.com/trc1', - text: 'special key link (trc1)', - }, - ]); - expect(cache.get(trace)).toBe(result); - }); -}); diff --git a/packages/jaeger-ui/src/model/link-patterns.tsx b/packages/jaeger-ui/src/model/link-patterns.tsx index ed38bf6685..ce672636a2 100644 --- a/packages/jaeger-ui/src/model/link-patterns.tsx +++ b/packages/jaeger-ui/src/model/link-patterns.tsx @@ -17,6 +17,7 @@ import { getConfigValue } from '../utils/config/get-config'; import { getParent } from './span'; import { TNil } from '../types'; import { Span, Link, KeyValuePair, Trace } from '../types/trace'; +import memoize from 'lru-memoize'; const parameterRegExp = /#\{([^{}]*)\}/g; @@ -35,6 +36,11 @@ type ProcessedLinkPattern = { parameters: string[]; }; +type TLinksRV = { url: string; text: string }[]; +const processedLinks: ProcessedLinkPattern[] = (getConfigValue('linkPatterns') || []) + .map(processLinkPattern) + .filter(Boolean); + function getParamNames(str: string) { const names = new Set(); str.replace(parameterRegExp, (match, name) => { @@ -139,20 +145,22 @@ function callTemplate(template: ProcessedTemplate, data: any) { return template.template(data); } -export function computeTraceLinks(linkPatterns: ProcessedLinkPattern[], trace: Trace) { - const result: { url: string; text: string }[] = []; - const validKeys = Object.keys(trace) - // @ts-ignore - .filter(key => typeof trace[key] === 'string' || typeof trace[key] === 'number'); +export function computeTraceLink(linkPatterns: ProcessedLinkPattern[], trace: Trace) { + const result: TLinksRV = []; + const validKeys = (Object.keys(trace) as (keyof Trace)[]).filter( + key => typeof trace[key] === 'string' || trace[key] === 'number' + ); linkPatterns .filter(pattern => pattern.type('traces')) .forEach(pattern => { const parameterValues: Record = {}; const allParameters = pattern.parameters.every(parameter => { - if (validKeys.some(name => name === parameter)) { - // @ts-ignore - parameterValues[parameter] = trace[parameter]; + const key = parameter as keyof Trace; + if (validKeys.includes(key)) { + // At this point is safe to access to trace object using parameter variable because + // we validated parameter against validKeys, this implies that parameter a keyof Trace. + parameterValues[parameter] = trace[key]; return true; } return false; @@ -168,6 +176,14 @@ export function computeTraceLinks(linkPatterns: ProcessedLinkPattern[], trace: T return result; } +export const getTraceLinks: (trace: Trace | undefined) => TLinksRV = memoize(10)( + (trace: Trace | undefined) => { + const result: TLinksRV = []; + if (!trace) return result; + return computeTraceLink(processedLinks, trace); + } +); + export function computeLinks( linkPatterns: ProcessedLinkPattern[], span: Span, @@ -217,20 +233,6 @@ export function computeLinks( return result; } -export function createGetTraceLinks(linkPatterns: ProcessedLinkPattern[], cache: WeakMap) { - return (trace: Trace | undefined) => { - if (!trace || linkPatterns.length === 0) { - return []; - } - let result = cache.get(trace); - if (!result) { - result = computeTraceLinks(linkPatterns, trace); - cache.set(trace, result); - } - return result; - }; -} - export function createGetLinks(linkPatterns: ProcessedLinkPattern[], cache: WeakMap) { return (span: Span, items: KeyValuePair[], itemIndex: number) => { if (linkPatterns.length === 0) { @@ -246,12 +248,4 @@ export function createGetLinks(linkPatterns: ProcessedLinkPattern[], cache: Weak }; } -export default createGetLinks( - (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(Boolean), - new WeakMap() -); - -export const getTraceLinks = createGetTraceLinks( - (getConfigValue('linkPatterns') || []).map(processLinkPattern).filter(Boolean), - new WeakMap() -); +export default createGetLinks(processedLinks, new WeakMap()); From f8d2f44c67749602cdfad3d3df29cec60e883142 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Fri, 6 Dec 2019 12:25:45 -0600 Subject: [PATCH 08/10] Hardened ExternalLinks.test.js Signed-off-by: Ruben Vargas --- .../components/common/ExternalLinks.test.js | 14 +++++++++-- .../jaeger-ui/src/model/link-patterns.tsx | 25 ++++++++++--------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/jaeger-ui/src/components/common/ExternalLinks.test.js b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js index e765caff45..9a4ed9a4ce 100644 --- a/packages/jaeger-ui/src/components/common/ExternalLinks.test.js +++ b/packages/jaeger-ui/src/components/common/ExternalLinks.test.js @@ -14,6 +14,8 @@ import React from 'react'; import { shallow } from 'enzyme'; +import { Menu, Dropdown } from 'antd'; + import ExternalLinks from './ExternalLinks'; describe('', () => { @@ -26,14 +28,22 @@ describe('', () => { ]; const wrapper = shallow(); - const dropdown = wrapper.find('Dropdown'); + const dropdown = wrapper.find(Dropdown); expect(dropdown.length).toBe(1); + const linkValues = shallow(dropdown.first().props().overlay); + const submenuItems = linkValues.find(Menu.Item); + expect(submenuItems.length).toBe(links.length); + submenuItems.forEach((subMenu, i) => { + const linkValue = subMenu.find('LinkValue'); + expect(linkValue.props().href).toBe(links[i].url); + expect(linkValue.props().children).toBe(links[i].text); + }); }); it('renders one link', () => { const links = [{ url: 'http://nowhere/', text: 'some text' }]; const wrapper = shallow(); - const dropdown = wrapper.find('Dropdown'); + const dropdown = wrapper.find(Dropdown); expect(dropdown.length).toBe(0); const linkValues = wrapper.find('LinkValue'); expect(linkValues.length).toBe(1); diff --git a/packages/jaeger-ui/src/model/link-patterns.tsx b/packages/jaeger-ui/src/model/link-patterns.tsx index ce672636a2..4a8b2464da 100644 --- a/packages/jaeger-ui/src/model/link-patterns.tsx +++ b/packages/jaeger-ui/src/model/link-patterns.tsx @@ -13,11 +13,11 @@ // limitations under the License. import _uniq from 'lodash/uniq'; +import memoize from 'lru-memoize'; import { getConfigValue } from '../utils/config/get-config'; import { getParent } from './span'; import { TNil } from '../types'; import { Span, Link, KeyValuePair, Trace } from '../types/trace'; -import memoize from 'lru-memoize'; const parameterRegExp = /#\{([^{}]*)\}/g; @@ -37,9 +37,6 @@ type ProcessedLinkPattern = { }; type TLinksRV = { url: string; text: string }[]; -const processedLinks: ProcessedLinkPattern[] = (getConfigValue('linkPatterns') || []) - .map(processLinkPattern) - .filter(Boolean); function getParamNames(str: string) { const names = new Set(); @@ -176,14 +173,6 @@ export function computeTraceLink(linkPatterns: ProcessedLinkPattern[], trace: Tr return result; } -export const getTraceLinks: (trace: Trace | undefined) => TLinksRV = memoize(10)( - (trace: Trace | undefined) => { - const result: TLinksRV = []; - if (!trace) return result; - return computeTraceLink(processedLinks, trace); - } -); - export function computeLinks( linkPatterns: ProcessedLinkPattern[], span: Span, @@ -248,4 +237,16 @@ export function createGetLinks(linkPatterns: ProcessedLinkPattern[], cache: Weak }; } +const processedLinks: ProcessedLinkPattern[] = (getConfigValue('linkPatterns') || []) + .map(processLinkPattern) + .filter(Boolean); + +export const getTraceLinks: (trace: Trace | undefined) => TLinksRV = memoize(10)( + (trace: Trace | undefined) => { + const result: TLinksRV = []; + if (!trace) return result; + return computeTraceLink(processedLinks, trace); + } +); + export default createGetLinks(processedLinks, new WeakMap()); From 4c2427ff27de3c4914f0de021047154d7ec64764 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Tue, 10 Dec 2019 14:46:21 -0600 Subject: [PATCH 09/10] Change ExternalLinks icons to use NewWindowIcon component Signed-off-by: Ruben Vargas --- packages/jaeger-ui/src/components/common/ExternalLinks.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/jaeger-ui/src/components/common/ExternalLinks.tsx b/packages/jaeger-ui/src/components/common/ExternalLinks.tsx index f0906ad97b..b0c77ae9ed 100644 --- a/packages/jaeger-ui/src/components/common/ExternalLinks.tsx +++ b/packages/jaeger-ui/src/components/common/ExternalLinks.tsx @@ -15,10 +15,10 @@ import { Dropdown, Icon, Menu } from 'antd'; import * as React from 'react'; import { Link } from '../../types/trace'; +import NewWindowIcon from './NewWindowIcon'; type ExternalLinksProps = { links: Link[]; - children?: React.ReactNode; }; const LinkValue = (props: { @@ -34,7 +34,7 @@ const LinkValue = (props: { rel="noopener noreferrer" className={props.className} > - {props.children} + {props.children} ); @@ -59,7 +59,7 @@ export default function ExternalLinks(props: ExternalLinksProps) { return ( - + ); From c39d6e6907c4ee8a1b73a0d56bc456f6688eadeb Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Tue, 10 Dec 2019 15:03:44 -0600 Subject: [PATCH 10/10] Fix minor lint issues on ExternalLink component Signed-off-by: Ruben Vargas --- packages/jaeger-ui/src/components/common/ExternalLinks.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jaeger-ui/src/components/common/ExternalLinks.tsx b/packages/jaeger-ui/src/components/common/ExternalLinks.tsx index b0c77ae9ed..39959414a5 100644 --- a/packages/jaeger-ui/src/components/common/ExternalLinks.tsx +++ b/packages/jaeger-ui/src/components/common/ExternalLinks.tsx @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { Dropdown, Icon, Menu } from 'antd'; +import { Dropdown, Menu } from 'antd'; import * as React from 'react'; import { Link } from '../../types/trace'; import NewWindowIcon from './NewWindowIcon'; @@ -59,7 +59,7 @@ export default function ExternalLinks(props: ExternalLinksProps) { return ( - + );