Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 356 maximum lookback should be retention period #384

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 100 additions & 13 deletions packages/jaeger-ui/src/components/SearchTracePage/SearchForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@

import * as React from 'react';
import { Form, Input, Button, Popover, Select } from 'antd';
import _get from 'lodash/get';
import logfmtParser from 'logfmt/lib/logfmt_parser';
import { stringify as logfmtStringify } from 'logfmt/lib/stringify';
import moment from 'moment';
import memoizeOne from 'memoize-one';
import PropTypes from 'prop-types';
import queryString from 'query-string';
import IoHelp from 'react-icons/lib/io/help';
Expand Down Expand Up @@ -71,6 +73,95 @@ 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: '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 const optionsWithinMaxLookback = memoizeOne(maxLookback => {
const now = new Date();
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 }) => (
<Option key={value} value={value}>
Last {label}
</Option>
));
});

export function traceIDsToQuery(traceIDs) {
if (!traceIDs) {
return null;
Expand Down Expand Up @@ -133,13 +224,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;
end = moment(now).valueOf() * 1000;
start = lookbackToTimestamp(lookback, now);
end = now * 1000;
} else {
const times = getUnixTimeStampInMSFromForm({
startDate,
Expand Down Expand Up @@ -171,6 +258,7 @@ export class SearchFormImpl extends React.PureComponent {
const {
handleSubmit,
invalid,
searchMaxLookback,
selectedLookback,
selectedService = '-',
services,
Expand Down Expand Up @@ -264,13 +352,7 @@ export class SearchFormImpl extends React.PureComponent {

<FormItem label="Lookback">
<Field name="lookback" component={AdaptedSelect} props={{ disabled, defaultValue: '1h' }}>
<Option value="1h">Last Hour</Option>
<Option value="2h">Last 2 Hours</Option>
<Option value="3h">Last 3 Hours</Option>
<Option value="6h">Last 6 Hours</Option>
<Option value="12h">Last 12 Hours</Option>
<Option value="24h">Last 24 Hours</Option>
<Option value="2d">Last 2 Days</Option>
{optionsWithinMaxLookback(searchMaxLookback)}
<Option value="custom">Custom Time Range</Option>
</Field>
</FormItem>
Expand Down Expand Up @@ -381,6 +463,10 @@ SearchFormImpl.propTypes = {
handleSubmit: PropTypes.func.isRequired,
invalid: PropTypes.bool,
submitting: PropTypes.bool,
searchMaxLookback: PropTypes.shape({
label: PropTypes.string.isRequired,
value: PropTypes.string.isRequired,
}).isRequired,
services: PropTypes.arrayOf(
PropTypes.shape({
name: PropTypes.string,
Expand Down Expand Up @@ -512,6 +598,7 @@ export function mapStateToProps(state) {
maxDuration: maxDuration || null,
traceIDs: traceIDs || null,
},
searchMaxLookback: _get(state, 'config.search.maxLookback'),
selectedService: searchSideBarFormSelector(state, 'service'),
selectedLookback: searchSideBarFormSelector(state, 'lookback'),
};
Expand Down
106 changes: 105 additions & 1 deletion packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import {
convertQueryParamsToFormDates,
convTagsLogfmt,
getUnixTimeStampInMSFromForm,
lookbackToTimestamp,
mapStateToProps,
optionsWithinMaxLookback,
submitForm,
traceIDsToQuery,
SearchFormImpl as SearchForm,
Expand Down Expand Up @@ -53,8 +55,12 @@ function makeDateParams(dateOffset = 0) {
const DATE_FORMAT = 'YYYY-MM-DD';
const TIME_FORMAT = 'HH:mm';
const defaultProps = {
services: [{ name: 'svcA', operations: ['A', 'B'] }, { name: 'svcB', operations: ['A', 'B'] }],
dataCenters: ['dc1'],
searchMaxLookback: {
label: '2 Days',
value: '2d',
},
services: [{ name: 'svcA', operations: ['A', 'B'] }, { name: 'svcB', operations: ['A', 'B'] }],
};

describe('conversion utils', () => {
Expand Down Expand Up @@ -126,6 +132,104 @@ describe('conversion utils', () => {
});
});

describe('lookback utils', () => {
describe('lookbackToTimestamp', () => {
const hourInMicroseconds = 60 * 60 * 1000 * 1000;
const now = new Date();
const nowInMicroseconds = now * 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', () => {
const threeHoursOfExpectedOptions = [
{
label: 'Hour',
value: '1h',
},
{
label: '2 Hours',
value: '2h',
},
{
label: '3 Hours',
value: '3h',
},
];

it('memoizes correctly', () => {
const firstCallOptions = optionsWithinMaxLookback(threeHoursOfExpectedOptions[0]);
const secondCallOptions = optionsWithinMaxLookback(threeHoursOfExpectedOptions[0]);
const thirdCallOptions = optionsWithinMaxLookback(threeHoursOfExpectedOptions[1]);
expect(secondCallOptions).toBe(firstCallOptions);
expect(thirdCallOptions).not.toBe(firstCallOptions);
});

it('returns options within config.search.maxLookback', () => {
const configValue = threeHoursOfExpectedOptions[2];
const options = optionsWithinMaxLookback(configValue);

expect(options.length).toBe(threeHoursOfExpectedOptions.length);
options.forEach(({ props }, i) => {
expect(props.value).toBe(threeHoursOfExpectedOptions[i].value);
expect(props.children[1]).toBe(threeHoursOfExpectedOptions[i].label);
});
});

it("includes config.search.maxLookback if it's not part of standard options", () => {
const configValue = {
label: '4 Hours - configValue',
value: '4h',
};
const expectedOptions = [...threeHoursOfExpectedOptions, configValue];
const options = optionsWithinMaxLookback(configValue);

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

it('uses config.search.maxLookback in place of standard option it is not equal to but is equivalent to', () => {
const configValue = {
label: '180 minutes is equivalent to 3 hours',
value: '180m',
};
const expectedOptions = [threeHoursOfExpectedOptions[0], threeHoursOfExpectedOptions[1], configValue];
const options = optionsWithinMaxLookback(configValue);

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';
Expand Down
16 changes: 11 additions & 5 deletions packages/jaeger-ui/src/constants/default-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ export default deepFreeze(
menuEnabled: true,
},
linkPatterns: [],
tracking: {
gaID: null,
trackErrors: true,
},
menu: [
{
label: 'About Jaeger',
Expand Down Expand Up @@ -60,10 +56,20 @@ export default deepFreeze(
],
},
],
search: {
maxLookback: {
label: '2 Days',
value: '2d',
},
},
tracking: {
gaID: null,
trackErrors: true,
},
},
// fields that should be individually merged vs wholesale replaced
'__mergeFields',
{ value: ['tracking', 'dependencies'] }
{ value: ['dependencies', 'search', 'tracking'] }
)
);

Expand Down
4 changes: 4 additions & 0 deletions packages/jaeger-ui/src/setupTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,7 @@ 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
// This file is executed before each test file, so this value may be overridden safely
window.getJaegerUiConfig = () => ({});
tiffon marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 3 additions & 2 deletions packages/jaeger-ui/src/types/config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ 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 } };
tracking?: {
gaID: string | TNil;
trackErrors: boolean | TNil;
};
menu: (ConfigMenuGroup | ConfigMenuItem)[];
};
15 changes: 11 additions & 4 deletions packages/jaeger-ui/src/utils/config/get-config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,27 @@ import processDeprecation from './process-deprecation';
import defaultConfig, { deprecations } from '../../constants/default-config';

describe('getConfig()', () => {
const warnFn = jest.fn();
let oldWarn;
let warnFn;

beforeEach(() => {
beforeAll(() => {
oldWarn = console.warn;
warnFn = jest.fn();
console.warn = warnFn;
});

afterEach(() => {
beforeEach(() => {
warnFn.mockClear();
});

afterAll(() => {
console.warn = oldWarn;
});

describe('`window.getJaegerUiConfig` is not a function', () => {
beforeAll(() => {
window.getJaegerUiConfig = undefined;
});

it('warns once', () => {
getConfig();
expect(warnFn.mock.calls.length).toBe(1);
Expand Down