Use logfmt for search tag input format

package.json

@@ -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,
@@ -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",

src/components/SearchTracePage/TraceSearchForm.css

@@ -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;
+}

src/components/SearchTracePage/TraceSearchForm.js

@@ -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}`;
@@ -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) {
@@ -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,
   });
@@ -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>
 
@@ -270,6 +304,7 @@ export function mapStateToProps(state) {
     end,
     operation,
     tag: tagParams,
+    tags: logfmtTags,
     maxDuration,
     minDuration,
     lookback,
@@ -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) {

src/components/SearchTracePage/TraceSearchForm.test.js

@@ -23,10 +23,10 @@ import store from 'store';
 
 import {
   convertQueryParamsToFormDates,
+  convTagsLogfmt,
   getUnixTimeStampInMSFromForm,
   mapStateToProps,
   submitForm,
-  tagsToQuery,
   traceIDsToQuery,
   TraceSearchFormImpl as TraceSearchForm,
 } from './TraceSearchForm';
@@ -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);
     });
   });
 
@@ -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;
 
@@ -121,7 +137,6 @@ describe('submitForm()', () => {
       operation: 'op-a',
       resultsLimit: 20,
       service: 'svc-a',
-      tags: 'a|b',
     };
   });
 
@@ -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"', () => {
@@ -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);
@@ -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);
     });
   });
 
@@ -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', () => {