Skip to content

Commit

Permalink
Merge pull request #147 from jaegertracing/issue-145-logfmt-tag-searc…
Browse files Browse the repository at this point in the history 
…h-input

Use logfmt for search tag input format
  • Loading branch information
@tiffon
tiffon authored Dec 22, 2017
2 parents 674b2db + 8d8c058 commit b95786c
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 61 deletions.
13 changes: 7 additions & 6 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
"license": "MIT",
"proxy": {
"/api": {
"target": "http://localhost:16686",
"logLevel": "silent",
"secure": false,
"changeOrigin": true,
"ws": true,
"xfwd": true
"target": "http://localhost:16686",
"logLevel": "silent",
"secure": false,
"changeOrigin": true,
"ws": true,
"xfwd": true
}
},
"homepage": null,
Expand Down Expand Up @@ -54,6 +54,7 @@
"jest": "^21.2.1",
"json-markup": "^1.1.0",
"lodash": "^4.17.4",
"logfmt": "^1.2.0",
"moment": "^2.18.1",
"prop-types": "^15.5.10",
"query-string": "^5.0.0",
Expand Down
23 changes: 23 additions & 0 deletions src/components/SearchTracePage/TraceSearchForm.css
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
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.
*/

.SearchForm--tagsHintInfo {
padding-left: 1.7em;
}

.SearchForm--tagsHintEg {
color: teal;
}
98 changes: 87 additions & 11 deletions src/components/SearchTracePage/TraceSearchForm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,23 @@
// limitations under the License.

import React from 'react';
import logfmtParser from 'logfmt/lib/logfmt_parser';
import { stringify as logfmtStringify } from 'logfmt/lib/stringify';
import moment from 'moment';
import PropTypes from 'prop-types';
import queryString from 'query-string';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import { Field, reduxForm, formValueSelector } from 'redux-form';
import { Popup } from 'semantic-ui-react';
import store from 'store';

import SearchDropdownInput from './SearchDropdownInput';
import * as jaegerApiActions from '../../actions/jaeger-api';
import { formatDate, formatTime } from '../../utils/date';

import './TraceSearchForm.css';

export function getUnixTimeStampInMSFromForm({ startDate, startDateTime, endDate, endDateTime }) {
const start = `${startDate} ${startDateTime}`;
const end = `${endDate} ${endDateTime}`;
Expand All @@ -34,11 +39,20 @@ export function getUnixTimeStampInMSFromForm({ startDate, startDateTime, endDate
};
}

export function tagsToQuery(tags) {
export function convTagsLogfmt(tags) {
if (!tags) {
return null;
}
return tags.split('|');
const data = logfmtParser.parse(tags);
Object.keys(data).forEach(key => {
const value = data[key];
// make sure all values are strings
// https://github.com/jaegertracing/jaeger/issues/550#issuecomment-352850811
if (typeof value !== 'string') {
data[key] = String(value);
}
});
return JSON.stringify(data);
}

export function traceIDsToQuery(traceIDs) {
Expand Down Expand Up @@ -117,7 +131,7 @@ export function submitForm(fields, searchTraces) {
lookback,
start,
end,
tag: tagsToQuery(tags) || undefined,
tags: convTagsLogfmt(tags) || undefined,
minDuration: minDuration || null,
maxDuration: maxDuration || null,
});
Expand Down Expand Up @@ -154,14 +168,34 @@ export function TraceSearchFormImpl(props) {
)}

<div className="search-form--tags field">
<label htmlFor="tags">Tags</label>
<div className="ui input">
<Field
name="tags"
type="text"
component="input"
placeholder="http.status_code:400|http.status_code:200"
<label htmlFor="tags">
Tags{' '}
<Popup
on="click"
wide="very"
trigger={<i className="info circle icon grey" />}
content={
<div>
<h5>
Values should be in the{' '}
<a href="https://brandur.org/logfmt" rel="noopener noreferrer" target="_blank">
logfmt
</a>{' '}
format.
</h5>
<ul className="SearchForm--tagsHintInfo">
<li>Use space for conjunctions</li>
<li>Values containing whitespace should be enclosed in quotes</li>
</ul>
<code className="SearchForm--tagsHintEg">
error=true db.statement=&quot;select * from User&quot;
</code>
</div>
}
/>
</label>
<div className="ui input">
<Field name="tags" type="text" component="input" placeholder="http.status_code=200 error=true" />
</div>
</div>

Expand Down Expand Up @@ -270,6 +304,7 @@ export function mapStateToProps(state) {
end,
operation,
tag: tagParams,
tags: logfmtTags,
maxDuration,
minDuration,
lookback,
Expand Down Expand Up @@ -307,8 +342,49 @@ export function mapStateToProps(state) {
} = convertQueryParamsToFormDates({ start, end });

let tags;
// continue to parse tagParams to remain backward compatible with older URLs
// but, parse to logfmt format instead of the former "key:value|k2:v2"
if (tagParams) {
tags = tagParams instanceof Array ? tagParams.join('|') : tagParams;
// eslint-disable-next-line no-inner-declarations
function convFormerTag(accum, value) {
const parts = value.split(':', 2);
const key = parts[0];
if (key) {
// eslint-disable-next-line no-param-reassign
accum[key] = parts[1] == null ? '' : parts[1];
return true;
}
return false;
}

let data;
if (Array.isArray(tagParams)) {
data = tagParams.reduce((accum, str) => {
convFormerTag(accum, str);
return accum;
}, {});
} else if (typeof tagParams === 'string') {
const target = {};
data = convFormerTag(target, tagParams) ? target : null;
}
if (data) {
try {
tags = logfmtStringify(data);
} catch (_) {
tags = 'Parse Error';
}
} else {
tags = 'Parse Error';
}
}
if (logfmtTags) {
let data;
try {
data = JSON.parse(logfmtTags);
tags = logfmtStringify(data);
} catch (_) {
tags = 'Parse Error';
}
}
let traceIDs;
if (traceIDParams) {
Expand Down
122 changes: 79 additions & 43 deletions src/components/SearchTracePage/TraceSearchForm.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import store from 'store';

import {
convertQueryParamsToFormDates,
convTagsLogfmt,
getUnixTimeStampInMSFromForm,
mapStateToProps,
submitForm,
tagsToQuery,
traceIDsToQuery,
TraceSearchFormImpl as TraceSearchForm,
} from './TraceSearchForm';
Expand Down Expand Up @@ -93,10 +93,26 @@ describe('conversion utils', () => {
});
});

describe('tagsToQuery()', () => {
it('splits on "|"', () => {
const strs = ['a', 'b', 'c'];
expect(tagsToQuery(strs.join('|'))).toEqual(strs);
describe('convTagsLogfmt()', () => {
it('converts logfmt formatted string to JSON', () => {
const input = 'http.status_code=404 span.kind=client key="with a long value"';
const target = JSON.stringify({
'http.status_code': '404',
'span.kind': 'client',
key: 'with a long value',
});
expect(convTagsLogfmt(input)).toBe(target);
});

// https://github.com/jaegertracing/jaeger/issues/550#issuecomment-352850811
it('converts all values to strings', () => {
const input = 'aBoolKey error=true num=9';
const target = JSON.stringify({
aBoolKey: 'true',
error: 'true',
num: '9',
});
expect(convTagsLogfmt(input)).toBe(target);
});
});

Expand All @@ -110,7 +126,7 @@ describe('conversion utils', () => {

describe('submitForm()', () => {
const LOOKBACK_VALUE = 2;
const LOOKBACK_UNIT = 'd';
const LOOKBACK_UNIT = 's';
let searchTraces;
let fields;

Expand All @@ -121,7 +137,6 @@ describe('submitForm()', () => {
operation: 'op-a',
resultsLimit: 20,
service: 'svc-a',
tags: 'a|b',
};
});

Expand All @@ -142,7 +157,7 @@ describe('submitForm()', () => {
const { start, end } = calls[0][0];
const diffMs = (Number(end) - Number(start)) / 1000;
const duration = moment.duration(diffMs);
expect(duration.asDays()).toBe(LOOKBACK_VALUE);
expect(duration.asSeconds()).toBe(LOOKBACK_VALUE);
});

it('processes form dates when `lookback` is "custom"', () => {
Expand All @@ -165,7 +180,7 @@ describe('submitForm()', () => {
});
});

describe('`fields.tag`', () => {
describe('`fields.tags`', () => {
it('is ignored when `fields.tags` is falsy', () => {
fields.tags = undefined;
submitForm(fields, searchTraces);
Expand All @@ -175,14 +190,19 @@ describe('submitForm()', () => {
expect(tag).toBe(undefined);
});

it('is parsed `fields.tags` is truthy', () => {
const tagStrs = ['a', 'b'];
fields.tags = tagStrs.join('|');
it('is parsed when `fields.tags` is truthy', () => {
const input = 'http.status_code=404 span.kind=client key="with a long value"';
const target = JSON.stringify({
'http.status_code': '404',
'span.kind': 'client',
key: 'with a long value',
});
fields.tags = input;
submitForm(fields, searchTraces);
const { calls } = searchTraces.mock;
expect(calls.length).toBe(1);
const { tag } = calls[0][0];
expect(tag).toEqual(tagStrs);
const { tags } = calls[0][0];
expect(tags).toEqual(target);
});
});

Expand Down Expand Up @@ -271,36 +291,52 @@ describe('mapStateToProps()', () => {
store.get = oldStoreGet;
});

it('derives values from `state.router.location.search` when available', () => {
const { date: startSrc, dateStr: startDate, dateTimeStr: startDateTime } = makeDateParams(-1);
const { date: endSrc, dateStr: endDate, dateTimeStr: endDateTime } = makeDateParams(0);
const common = {
lookback: '2h',
maxDuration: null,
minDuration: null,
operation: 'Driver::findNearest',
service: 'driver',
};
const params = {
...common,
end: `${endSrc.valueOf()}000`,
limit: '999',
start: `${startSrc.valueOf()}000`,
tag: ['error:true', 'span.kind:client'],
};
const expected = {
...common,
endDate,
endDateTime,
startDate,
startDateTime,
resultsLimit: params.limit,
tags: params.tag.join('|'),
traceIDs: null,
};
describe('deriving values from `state.router.location.search`', () => {
let params;
let expected;

state.router.location.search = queryString.stringify(params);
expect(mapStateToProps(state).initialValues).toEqual(expected);
beforeEach(() => {
const { date: startSrc, dateStr: startDate, dateTimeStr: startDateTime } = makeDateParams(-1);
const { date: endSrc, dateStr: endDate, dateTimeStr: endDateTime } = makeDateParams(0);
const tagsJSON = '{"error":"true","span.kind":"client"}';
const tagsLogfmt = 'error=true span.kind=client';
const common = {
lookback: '2h',
maxDuration: null,
minDuration: null,
operation: 'Driver::findNearest',
service: 'driver',
};
params = {
...common,
end: `${endSrc.valueOf()}000`,
limit: '999',
start: `${startSrc.valueOf()}000`,
tags: tagsJSON,
};
expected = {
...common,
endDate,
endDateTime,
startDate,
startDateTime,
resultsLimit: params.limit,
tags: tagsLogfmt,
traceIDs: null,
};
});

it('derives values when available', () => {
state.router.location.search = queryString.stringify(params);
expect(mapStateToProps(state).initialValues).toEqual(expected);
});

it('parses `tag` values in the former format to logfmt', () => {
delete params.tags;
params.tag = ['error:true', 'span.kind:client'];
state.router.location.search = queryString.stringify(params);
expect(mapStateToProps(state).initialValues).toEqual(expected);
});
});

it('fallsback to default values', () => {
Expand Down
Loading

0 comments on commit b95786c

Please sign in to comment.