Do not render quotes around string tag values (#587)

* don't rely on json-markup for non-json strings Signed-off-by: Simeon Manolov <simeon.manolov@sumup.com> * prettier Signed-off-by: Simeon Manolov <simeon.manolov@sumup.com> * fix quotes around non-json strings; test coverage Signed-off-by: Simeon Manolov <simeon.manolov@sumup.com> * lint Signed-off-by: Simeon Manolov <simeon.manolov@sumup.com> * prettier Signed-off-by: Simeon Manolov <simeon.manolov@sumup.com> * don't use toString in tests Signed-off-by: Simeon Manolov <simeon.manolov@sumup.com>
jaegertracing · Jun 2, 2020 · 4dbd576 · 4dbd576
1 parent eb6d985
commit 4dbd576
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 17 deletions.
diff --git a/.../jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js b/.../jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.test.js
@@ -46,9 +46,10 @@ describe('<KeyValuesTable>', () => {
   let wrapper;
 
   const data = [
-    { key: 'span.kind', value: 'client' },
-    { key: 'omg', value: 'mos-def' },
-    { key: 'numericString', value: '12345678901234567890' },
+    { key: 'span.kind', value: 'client', expected: 'client' },
+    { key: 'omg', value: 'mos-def', expected: 'mos-def' },
+    { key: 'numericString', value: '12345678901234567890', expected: '12345678901234567890' },
+    { key: 'numeric', value: 123456789, expected: '123456789' },
     { key: 'jsonkey', value: JSON.stringify({ hello: 'world' }) },
   ];
 
@@ -138,7 +139,7 @@ describe('<KeyValuesTable>', () => {
     expect(el.length).toBe(data.length);
     el.forEach((valueDiv, i) => {
       if (data[i].key !== 'jsonkey') {
-        expect(valueDiv.html()).toMatch(`"${data[i].value}"`);
+        expect(valueDiv.html()).toMatch(data[i].expected);
       }
     });
   });

diff --git a/...ages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.tsx b/...ages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/KeyValuesTable.tsx
@@ -25,16 +25,42 @@ import './KeyValuesTable.css';
 
 const jsonObjectOrArrayStartRegex = /^(\[|\{)/;
 
-function parseIfComplexJson(value: any) {
+function tryParseJson(value: string) {
   // if the value is a string representing actual json object or array, then use json-markup
-  if (typeof value === 'string' && jsonObjectOrArrayStartRegex.test(value)) {
-    // otherwise just return as is
-    try {
-      return JSON.parse(value);
-      // eslint-disable-next-line no-empty
-    } catch (_) {}
+  // otherwise just return as is
+  try {
+    return jsonObjectOrArrayStartRegex.test(value) ? JSON.parse(value) : value;
+  } catch (_) {
+    return value;
   }
-  return value;
+}
+
+const stringMarkup = (value: string) => (
+  <div className="json-markup">
+    <span className="json-markup-string">{value}</span>
+  </div>
+);
+
+function _jsonMarkup(value: any) {
+  const markup = { __html: jsonMarkup(value) };
+
+  return (
+    // eslint-disable-next-line react/no-danger
+    <div dangerouslySetInnerHTML={markup} />
+  );
+}
+
+function formatValue(value: any) {
+  let content;
+
+  if (typeof value === 'string') {
+    const parsed = tryParseJson(value);
+    content = typeof parsed === 'string' ? stringMarkup(parsed) : _jsonMarkup(parsed);
+  } else {
+    content = _jsonMarkup(value);
+  }
+
+  return <div className="ub-inline-block">{content}</div>;
 }
 
 export const LinkValue = (props: { href: string; title?: string; children: React.ReactNode }) => (
@@ -71,11 +97,7 @@ export default function KeyValuesTable(props: KeyValuesTableProps) {
       <table className="u-width-100">
         <tbody className="KeyValueTable--body">
           {data.map((row, i) => {
-            const markup = {
-              __html: jsonMarkup(parseIfComplexJson(row.value)),
-            };
-            // eslint-disable-next-line react/no-danger
-            const jsonTable = <div className="ub-inline-block" dangerouslySetInnerHTML={markup} />;
+            const jsonTable = formatValue(row.value);
             const links = linksGetter ? linksGetter(data, i) : null;
             let valueMarkup;
             if (links && links.length === 1) {