From ac7a1945842a3120cb15ece7b3cbbae4d490d454 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Mon, 20 May 2019 17:09:24 -0400 Subject: [PATCH 1/5] WIP - Add maxLookback config Signed-off-by: Everett Ross --- .../components/SearchTracePage/SearchForm.js | 119 ++++++++++++++++-- .../src/constants/default-config.tsx | 16 ++- 2 files changed, 118 insertions(+), 17 deletions(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js index 52a9af505b..22ed32d30d 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js @@ -30,6 +30,7 @@ import { trackFormInput } from './SearchForm.track'; import VirtSelect from '../common/VirtSelect'; import * as jaegerApiActions from '../../actions/jaeger-api'; import { formatDate, formatTime } from '../../utils/date'; +import { getConfigValue } from '../../utils/config/get-config'; import reduxFormFieldAdapter from '../../utils/redux-form-field-adapter'; import { DEFAULT_OPERATION, DEFAULT_LIMIT, DEFAULT_LOOKBACK } from '../../constants/search-form'; @@ -71,6 +72,107 @@ export function convTagsLogfmt(tags) { return JSON.stringify(data); } +export function lookbackToTimestamp(lookback, from) { + const unit = lookback.substr(-1); + return moment(from) + .subtract(parseInt(lookback, 10), unit) + .valueOf() * 1000; +} + +const lookbackOptions = [ + { + label: 'Hour', + value: '1h', + }, + { + label: '2 Hours', + value: '2h', + }, + { + label: '3 Hours', + value: '3h', + }, + { + label: '4 Hours', + value: '4h', + }, + { + label: '6 Hours', + value: '6h', + }, + { + label: '12 Hours', + value: '12h', + }, + { + label: '24 Hours', + value: '24h', + }, + { + label: '2 Days', + value: '2d', + }, + { + label: '3 Days', + value: '3d', + }, + { + label: '5 Days', + value: '5d', + }, + { + label: '7 Days', + value: '7d', + }, + { + label: '2 Weeks', + value: '2w', + }, + { + label: '3 Weeks', + value: '3w', + }, + { + label: '4 Weeks', + value: '4w', + }, +]; + +export function optionsWithinMaxLookback() { + // console.log(lookbackOptions); + const maxLookback = getConfigValue('search.maxLookback'); + const { label: maxLookbackLabel, value: maxLookbackValue } = maxLookback; + const now = new Date(); + const minTimestamp = lookbackToTimestamp(maxLookbackValue, now); + const options = lookbackOptions.map(({ label, value }) => { + const timeStamp = lookbackToTimestamp(value, now); + return timeStamp >= minTimestamp + ? { + label, + timeStamp, + } + : null + }).filter(Boolean); + if (options[options.length - 1].timeStamp !== minTimestamp) { + options.push({ label: maxLookbackLabel, timeStamp: minTimestamp }); + } + return options.map(({ label, timeStamp }) => ()); + + /* + const maxLookback = getConfigValue('search.maxLookback'); + const { value: maxLookbackValue } = maxLookback; + const now = new Date(); + const minTimestamp = lookbackToTimestamp(maxLookbackValue, now); + const options = lookbackOptions.filter(({ value }) => { + return lookbackToTimestamp(value, now) >= minTimestamp; + }); + if (options[options.length - 1].value !== maxLookbackValue) { + options.push(maxLookback); + } + return options.map(({ label, value }) => ()); + */ +} + export function traceIDsToQuery(traceIDs) { if (!traceIDs) { return null; @@ -133,12 +235,9 @@ export function submitForm(fields, searchTraces) { let start; let end; if (lookback !== 'custom') { - const unit = lookback.substr(-1); const now = new Date(); - start = - moment(now) - .subtract(parseInt(lookback, 10), unit) - .valueOf() * 1000; + start = lookback; + // start = lookbackToTimestamp(lookback, now) end = moment(now).valueOf() * 1000; } else { const times = getUnixTimeStampInMSFromForm({ @@ -264,13 +363,9 @@ export class SearchFormImpl extends React.PureComponent { - - - - - - - + { /* lookbackOptions.map(({ label, value }) => ()) */ } + { /* */ } + { optionsWithinMaxLookback() } diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx index 250d133761..8c5816d57e 100644 --- a/packages/jaeger-ui/src/constants/default-config.tsx +++ b/packages/jaeger-ui/src/constants/default-config.tsx @@ -25,10 +25,6 @@ export default deepFreeze( menuEnabled: true, }, linkPatterns: [], - tracking: { - gaID: null, - trackErrors: true, - }, menu: [ { label: 'About Jaeger', @@ -60,10 +56,20 @@ export default deepFreeze( ], }, ], + search: { + maxLookback: { + label: '4 Days', + value: '3d', + }, + }, + tracking: { + gaID: null, + trackErrors: true, + }, }, // fields that should be individually merged vs wholesale replaced '__mergeFields', - { value: ['tracking', 'dependencies'] } + { value: ['dependencies', 'search', 'tracking'] } ) ); From e054650e85bd953634444f04a566d755216e06a3 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Tue, 21 May 2019 11:50:49 -0400 Subject: [PATCH 2/5] Finish adding and testing maxLookback config Signed-off-by: Everett Ross --- .../components/SearchTracePage/SearchForm.js | 70 ++++++---------- .../SearchTracePage/SearchForm.test.js | 83 +++++++++++++++++++ .../src/constants/default-config.tsx | 4 +- packages/jaeger-ui/src/setupTests.js | 3 + packages/jaeger-ui/src/types/config.tsx | 3 +- .../src/utils/config/get-config.test.js | 15 +++- 6 files changed, 130 insertions(+), 48 deletions(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js index 22ed32d30d..eae8b47be4 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js @@ -14,6 +14,7 @@ import * as React from 'react'; import { Form, Input, Button, Popover, Select } from 'antd'; +import _memoize from 'lodash/memoize'; import logfmtParser from 'logfmt/lib/logfmt_parser'; import { stringify as logfmtStringify } from 'logfmt/lib/stringify'; import moment from 'moment'; @@ -74,12 +75,14 @@ export function convTagsLogfmt(tags) { export function lookbackToTimestamp(lookback, from) { const unit = lookback.substr(-1); - return moment(from) - .subtract(parseInt(lookback, 10), unit) - .valueOf() * 1000; + return ( + moment(from) + .subtract(parseInt(lookback, 10), unit) + .valueOf() * 1000 + ); } -const lookbackOptions = [ +export const lookbackOptions = [ { label: 'Hour', value: '1h', @@ -138,40 +141,24 @@ const lookbackOptions = [ }, ]; -export function optionsWithinMaxLookback() { - // console.log(lookbackOptions); - const maxLookback = getConfigValue('search.maxLookback'); - const { label: maxLookbackLabel, value: maxLookbackValue } = maxLookback; - const now = new Date(); - const minTimestamp = lookbackToTimestamp(maxLookbackValue, now); - const options = lookbackOptions.map(({ label, value }) => { - const timeStamp = lookbackToTimestamp(value, now); - return timeStamp >= minTimestamp - ? { - label, - timeStamp, - } - : null - }).filter(Boolean); - if (options[options.length - 1].timeStamp !== minTimestamp) { - options.push({ label: maxLookbackLabel, timeStamp: minTimestamp }); - } - return options.map(({ label, timeStamp }) => ()); - - /* - const maxLookback = getConfigValue('search.maxLookback'); - const { value: maxLookbackValue } = maxLookback; - const now = new Date(); - const minTimestamp = lookbackToTimestamp(maxLookbackValue, now); - const options = lookbackOptions.filter(({ value }) => { - return lookbackToTimestamp(value, now) >= minTimestamp; - }); - if (options[options.length - 1].value !== maxLookbackValue) { - options.push(maxLookback); - } - return options.map(({ label, value }) => ()); - */ -} +export const optionsWithinMaxLookback = _memoize( + () => { + const maxLookback = getConfigValue('search.maxLookback'); + const { value: maxLookbackValue } = maxLookback; + const now = new Date(); + const minTimestamp = lookbackToTimestamp(maxLookbackValue, now); + const options = lookbackOptions.filter(({ value }) => lookbackToTimestamp(value, now) >= minTimestamp); + if (options[options.length - 1].value !== maxLookbackValue) { + options.push(maxLookback); + } + return options.map(({ label, value }) => ( + + )); + }, + () => getConfigValue('search.maxLookback') +); export function traceIDsToQuery(traceIDs) { if (!traceIDs) { @@ -236,8 +223,7 @@ export function submitForm(fields, searchTraces) { let end; if (lookback !== 'custom') { const now = new Date(); - start = lookback; - // start = lookbackToTimestamp(lookback, now) + start = lookbackToTimestamp(lookback, now); end = moment(now).valueOf() * 1000; } else { const times = getUnixTimeStampInMSFromForm({ @@ -363,9 +349,7 @@ export class SearchFormImpl extends React.PureComponent { - { /* lookbackOptions.map(({ label, value }) => ()) */ } - { /* */ } - { optionsWithinMaxLookback() } + {optionsWithinMaxLookback()} diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js index 3f2edecc03..748b77bdf5 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js @@ -25,13 +25,17 @@ import { convertQueryParamsToFormDates, convTagsLogfmt, getUnixTimeStampInMSFromForm, + lookbackOptions, + lookbackToTimestamp, mapStateToProps, + optionsWithinMaxLookback, submitForm, traceIDsToQuery, SearchFormImpl as SearchForm, validateDurationFields, } from './SearchForm'; import * as markers from './SearchForm.markers'; +import * as getConfig from '../../utils/config/get-config'; function makeDateParams(dateOffset = 0) { const date = new Date(); @@ -126,6 +130,85 @@ describe('conversion utils', () => { }); }); +describe('lookback utils', () => { + describe('lookbackToTimestamp', () => { + const hourInMicroseconds = 60 * 60 * 1000 * 1000; + const now = new Date(); + const nowInMicroseconds = moment(now).valueOf() * 1000; + + it('creates timestamp for hours ago', () => { + [1, 2, 4, 7].forEach(lookbackNum => { + expect(nowInMicroseconds - lookbackToTimestamp(`${lookbackNum}h`, now)).toBe( + lookbackNum * hourInMicroseconds + ); + }); + }); + + it('creates timestamp for days ago', () => { + [1, 2, 4, 7].forEach(lookbackNum => { + expect(nowInMicroseconds - lookbackToTimestamp(`${lookbackNum}d`, now)).toBe( + lookbackNum * 24 * hourInMicroseconds + ); + }); + }); + + it('creates timestamp for weeks ago', () => { + [1, 2, 4, 7].forEach(lookbackNum => { + expect(nowInMicroseconds - lookbackToTimestamp(`${lookbackNum}w`, now)).toBe( + lookbackNum * 7 * 24 * hourInMicroseconds + ); + }); + }); + }); + + describe('optionsWithinMaxLookback', () => { + let getConfigValueSpy; + + beforeAll(() => { + getConfigValueSpy = jest.spyOn(getConfig, 'getConfigValue'); + }); + + it('memoizes correctly', () => { + getConfigValueSpy.mockReturnValue(lookbackOptions[0]); + const firstCallOptions = optionsWithinMaxLookback(); + const secondCallOptions = optionsWithinMaxLookback(); + getConfigValueSpy.mockReturnValue(lookbackOptions[1]); + const thirdCallOptions = optionsWithinMaxLookback(); + expect(secondCallOptions).toBe(firstCallOptions); + expect(thirdCallOptions).not.toBe(firstCallOptions); + }); + + it('returns options within config.search.maxLookback', () => { + const expectedOptionsLength = 8; + const expectedOptions = lookbackOptions.slice(0, expectedOptionsLength); + getConfigValueSpy.mockReturnValue(lookbackOptions[expectedOptionsLength - 1]); + const options = optionsWithinMaxLookback(); + + expect(options.length).toBe(expectedOptionsLength); + options.forEach(({ props }, i) => { + expect(props.value).toBe(expectedOptions[i].value); + expect(props.children[1]).toBe(expectedOptions[i].label); + }); + }); + + it("includes config.search.maxLookback if it's not part of standard options", () => { + const configValue = { + label: '4 Days - configValue', + value: '4d', + }; + const expectedOptions = [...lookbackOptions.slice(0, 9), configValue]; + getConfigValueSpy.mockReturnValue(configValue); + const options = optionsWithinMaxLookback(); + + expect(options.length).toBe(expectedOptions.length); + options.forEach(({ props }, i) => { + expect(props.value).toBe(expectedOptions[i].value); + expect(props.children[1]).toBe(expectedOptions[i].label); + }); + }); + }); +}); + describe('submitForm()', () => { const LOOKBACK_VALUE = 2; const LOOKBACK_UNIT = 's'; diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx index 8c5816d57e..a7b332cb89 100644 --- a/packages/jaeger-ui/src/constants/default-config.tsx +++ b/packages/jaeger-ui/src/constants/default-config.tsx @@ -58,8 +58,8 @@ export default deepFreeze( ], search: { maxLookback: { - label: '4 Days', - value: '3d', + label: '2 Days', + value: '2d', }, }, tracking: { diff --git a/packages/jaeger-ui/src/setupTests.js b/packages/jaeger-ui/src/setupTests.js index 03e035d9b4..8e55394847 100644 --- a/packages/jaeger-ui/src/setupTests.js +++ b/packages/jaeger-ui/src/setupTests.js @@ -25,3 +25,6 @@ const createSerializer = require('enzyme-to-json').createSerializer; Enzyme.configure({ adapter: new EnzymeAdapter() }); expect.addSnapshotSerializer(createSerializer({ mode: 'deep' })); + +// Calls to get-config.tsx warn if this global is not a function +window.getJaegerUiConfig = () => ({}); diff --git a/packages/jaeger-ui/src/types/config.tsx b/packages/jaeger-ui/src/types/config.tsx index fadf8bc6ce..22f3544b42 100644 --- a/packages/jaeger-ui/src/types/config.tsx +++ b/packages/jaeger-ui/src/types/config.tsx @@ -28,9 +28,10 @@ export type ConfigMenuGroup = { export type Config = { archiveEnabled: boolean | TNil; dependencies?: { dagMaxServicesLen?: number; menuEnabled?: boolean }; + menu: (ConfigMenuGroup | ConfigMenuItem)[]; + search: { maxLookback: { label: string; value: string } }; tracking?: { gaID: string | TNil; trackErrors: boolean | TNil; }; - menu: (ConfigMenuGroup | ConfigMenuItem)[]; }; diff --git a/packages/jaeger-ui/src/utils/config/get-config.test.js b/packages/jaeger-ui/src/utils/config/get-config.test.js index a2ed3f8854..5300c9eb13 100644 --- a/packages/jaeger-ui/src/utils/config/get-config.test.js +++ b/packages/jaeger-ui/src/utils/config/get-config.test.js @@ -22,19 +22,30 @@ import defaultConfig, { deprecations } from '../../constants/default-config'; describe('getConfig()', () => { let oldWarn; + let oldWindowGetJaegerUiConfig; let warnFn; - beforeEach(() => { + beforeAll(() => { oldWarn = console.warn; + oldWindowGetJaegerUiConfig = window.getJaegerUiConfig; warnFn = jest.fn(); console.warn = warnFn; }); - afterEach(() => { + beforeEach(() => { + warnFn.mockClear(); + }); + + afterAll(() => { console.warn = oldWarn; + window.getJaegerUiConfig = oldWindowGetJaegerUiConfig; }); describe('`window.getJaegerUiConfig` is not a function', () => { + beforeAll(() => { + window.getJaegerUiConfig = undefined; + }); + it('warns once', () => { getConfig(); expect(warnFn.mock.calls.length).toBe(1); From ef2021f689c39813772fbcee41f6be87ed6a3fdc Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Wed, 22 May 2019 14:08:37 -0400 Subject: [PATCH 3/5] Mark optional config properties as optional Signed-off-by: Everett Ross --- packages/jaeger-ui/src/types/config.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jaeger-ui/src/types/config.tsx b/packages/jaeger-ui/src/types/config.tsx index 22f3544b42..95b015b2c1 100644 --- a/packages/jaeger-ui/src/types/config.tsx +++ b/packages/jaeger-ui/src/types/config.tsx @@ -26,10 +26,10 @@ export type ConfigMenuGroup = { }; export type Config = { - archiveEnabled: boolean | TNil; + archiveEnabled?: boolean; dependencies?: { dagMaxServicesLen?: number; menuEnabled?: boolean }; menu: (ConfigMenuGroup | ConfigMenuItem)[]; - search: { maxLookback: { label: string; value: string } }; + search?: { maxLookback: { label: string; value: string } }; tracking?: { gaID: string | TNil; trackErrors: boolean | TNil; From 1bd52f5142d60fd967458242fe14a21bd70b2a1c Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Fri, 24 May 2019 10:42:59 -0400 Subject: [PATCH 4/5] Remove new option, use moment less, use config label for equivalent options Signed-off-by: Everett Ross --- .../components/SearchTracePage/SearchForm.js | 26 ++++++++++++------- .../SearchTracePage/SearchForm.test.js | 18 ++++++++++++- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js index eae8b47be4..fe4a537b28 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js @@ -95,10 +95,6 @@ export const lookbackOptions = [ label: '3 Hours', value: '3h', }, - { - label: '4 Hours', - value: '4h', - }, { label: '6 Hours', value: '6h', @@ -144,12 +140,22 @@ export const lookbackOptions = [ export const optionsWithinMaxLookback = _memoize( () => { const maxLookback = getConfigValue('search.maxLookback'); - const { value: maxLookbackValue } = maxLookback; const now = new Date(); - const minTimestamp = lookbackToTimestamp(maxLookbackValue, now); - const options = lookbackOptions.filter(({ value }) => lookbackToTimestamp(value, now) >= minTimestamp); - if (options[options.length - 1].value !== maxLookbackValue) { - options.push(maxLookback); + const minTimestamp = lookbackToTimestamp(maxLookback.value, now); + const lookbackToTimestampMap = new Map(); + const options = lookbackOptions.filter(({ value }) => { + const lookbackTimestamp = lookbackToTimestamp(value, now); + lookbackToTimestampMap.set(value, lookbackTimestamp); + return lookbackTimestamp >= minTimestamp; + }); + const lastInRangeIndex = options.length - 1; + const lastInRangeOption = options[lastInRangeIndex]; + if (lastInRangeOption.label !== maxLookback.label) { + if (lookbackToTimestampMap.get(lastInRangeOption.value) !== minTimestamp) { + options.push(maxLookback); + } else { + options.splice(lastInRangeIndex, 1, maxLookback); + } } return options.map(({ label, value }) => (