reflect PR feedback

Signed-off-by: Lev Popov <leo@nabam.net>
jaegertracing · Dec 5, 2019 · 2eb7676 · 2eb7676
1 parent 23af4a8
commit 2eb7676
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 15 deletions.
diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx
@@ -66,7 +66,6 @@ export default deepFreeze(
         gaID: null,
         trackErrors: true,
       },
-      topTagPrefixes: ['http.'],
     },
     // fields that should be individually merged vs wholesale replaced
     '__mergeFields',

diff --git a/packages/jaeger-ui/src/model/transform-trace-data.test.js b/packages/jaeger-ui/src/model/transform-trace-data.test.js
@@ -19,14 +19,18 @@ describe('orderTags()', () => {
     const orderedTags = orderTags(
       [
         { key: 'b.ip', value: '8.8.4.4' },
-        { key: 'http.status_code', value: '200' },
+        { key: 'http.Status_code', value: '200' },
+        { key: 'z.ip', value: '8.8.8.16' },
         { key: 'a.ip', value: '8.8.8.8' },
+        { key: 'http.message', value: 'ok' },
       ],
-      ['http.']
+      ['z.', 'a.', 'HTTP.']
     );
     expect(orderedTags).toEqual([
-      { key: 'http.status_code', value: '200' },
+      { key: 'z.ip', value: '8.8.8.16' },
       { key: 'a.ip', value: '8.8.8.8' },
+      { key: 'http.message', value: 'ok' },
+      { key: 'http.Status_code', value: '200' },
       { key: 'b.ip', value: '8.8.4.4' },
     ]);
   });

diff --git a/packages/jaeger-ui/src/model/transform-trace-data.tsx b/packages/jaeger-ui/src/model/transform-trace-data.tsx
@@ -19,9 +19,10 @@ import { getConfigValue } from '../utils/config/get-config';
 import { KeyValuePair, Span, SpanData, Trace, TraceData } from '../types/trace';
 import TreeNode from '../utils/TreeNode';
 
-export function deduplicateTags(spanTags: Array<KeyValuePair>) {
+// exported for tests
+export function deduplicateTags(spanTags: KeyValuePair[]) {
   const warningsHash: Map<string, string> = new Map<string, string>();
-  const tags: Array<KeyValuePair> = spanTags.reduce<Array<KeyValuePair>>((uniqueTags, tag) => {
+  const tags: KeyValuePair[] = spanTags.reduce<KeyValuePair[]>((uniqueTags, tag) => {
     if (!uniqueTags.some(t => t.key === tag.key && t.value === tag.value)) {
       uniqueTags.push(tag);
     } else {
@@ -33,21 +34,34 @@ export function deduplicateTags(spanTags: Array<KeyValuePair>) {
   return { tags, warnings };
 }
 
-export function orderTags(spanTags: Array<KeyValuePair>, topPrefixes: Array<string>) {
-  const orderedTags: Array<KeyValuePair> = [...spanTags];
+// exported for tests
+export function orderTags(spanTags: KeyValuePair[], topPrefixes?: string[]) {
+  const orderedTags: KeyValuePair[] = spanTags.slice();
+  const tp = (topPrefixes || []).map((p: string) => p.toLowerCase());
+
   orderedTags.sort((a, b) => {
-    for (let i = 0; i < topPrefixes.length; i++) {
-      const p = topPrefixes[i];
-      if (a.key.startsWith(p) && !b.key.startsWith(p)) {
+    const aKey = a.key.toLowerCase();
+    const bKey = b.key.toLowerCase();
+
+    for (let i = 0; i < tp.length; i++) {
+      const p = tp[i];
+      if (aKey.startsWith(p) && !bKey.startsWith(p)) {
         return -1;
       }
-
-      if (!a.key.startsWith(p) && b.key.startsWith(p)) {
+      if (!aKey.startsWith(p) && bKey.startsWith(p)) {
         return 1;
       }
     }
-    return a.key.localeCompare(b.key);
+
+    if (aKey > bKey) {
+      return 1;
+    }
+    if (aKey < bKey) {
+      return -1;
+    }
+    return 0;
   });
+
   return orderedTags;
 }
 
@@ -128,7 +142,7 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
     span.tags = span.tags || [];
     span.references = span.references || [];
     const tagsInfo = deduplicateTags(span.tags);
-    span.tags = orderTags(tagsInfo.tags, getConfigValue('topTagPrefixes') || []);
+    span.tags = orderTags(tagsInfo.tags, getConfigValue('topTagPrefixes'));
     span.warnings = span.warnings.concat(tagsInfo.warnings);
     span.references.forEach(ref => {
       const refSpan = spanMap.get(ref.spanID) as Span;