From 85a1a97ff8a26139dd119473185405768e6eead5 Mon Sep 17 00:00:00 2001 From: Lev Popov Date: Thu, 28 Nov 2019 12:20:03 +0100 Subject: [PATCH 1/4] sort span tags in alphabetical order Signed-off-by: Lev Popov --- packages/jaeger-ui/src/model/transform-trace-data.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/jaeger-ui/src/model/transform-trace-data.tsx b/packages/jaeger-ui/src/model/transform-trace-data.tsx index 46eeb83c3d..db98ea7a7c 100644 --- a/packages/jaeger-ui/src/model/transform-trace-data.tsx +++ b/packages/jaeger-ui/src/model/transform-trace-data.tsx @@ -109,6 +109,9 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[] span.tags = span.tags || []; span.references = span.references || []; const tagsInfo = deduplicateTags(span.tags); + tagsInfo.tags.sort((a, b) => { + return a.key.localeCompare(b.key); + }); span.tags = tagsInfo.tags; span.warnings = span.warnings.concat(tagsInfo.warnings); span.references.forEach(ref => { From ca5884d8e7fa81d0d3850c902c93159f2fa9fc6a Mon Sep 17 00:00:00 2001 From: Lev Popov Date: Thu, 28 Nov 2019 14:36:55 +0100 Subject: [PATCH 2/4] ability to configure top tag keys for sorting Signed-off-by: Lev Popov --- .../src/constants/default-config.tsx | 1 + .../src/model/transform-trace-data.test.js | 33 +++++++++++++++++++ .../src/model/transform-trace-data.tsx | 24 +++++++++++--- packages/jaeger-ui/src/types/config.tsx | 1 + 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 packages/jaeger-ui/src/model/transform-trace-data.test.js diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx index a7b332cb89..ac133f849e 100644 --- a/packages/jaeger-ui/src/constants/default-config.tsx +++ b/packages/jaeger-ui/src/constants/default-config.tsx @@ -66,6 +66,7 @@ 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 new file mode 100644 index 0000000000..28abae417a --- /dev/null +++ b/packages/jaeger-ui/src/model/transform-trace-data.test.js @@ -0,0 +1,33 @@ +// Copyright (c) 2019 The Jaeger Authors. +// +// 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. + +import { orderTags } from './transform-trace-data'; + +describe('orderTags()', () => { + it('correctly orders tags', () => { + const orderedTags = orderTags( + [ + { key: 'b.ip', value: '8.8.4.4' }, + { key: 'http.status_code', value: '200' }, + { key: 'a.ip', value: '8.8.8.8' }, + ], + ['http.'] + ); + expect(orderedTags).toEqual([ + { key: 'http.status_code', value: '200' }, + { key: 'a.ip', value: '8.8.8.8' }, + { 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 index db98ea7a7c..1b23c33890 100644 --- a/packages/jaeger-ui/src/model/transform-trace-data.tsx +++ b/packages/jaeger-ui/src/model/transform-trace-data.tsx @@ -15,6 +15,7 @@ import _isEqual from 'lodash/isEqual'; import { getTraceSpanIdsAsTree } from '../selectors/trace'; +import { getConfigValue } from '../utils/config/get-config'; import { KeyValuePair, Span, SpanData, Trace, TraceData } from '../types/trace'; import TreeNode from '../utils/TreeNode'; @@ -32,6 +33,24 @@ function deduplicateTags(spanTags: Array) { return { tags, warnings }; } +export function orderTags(spanTags: Array, topPrefixes: Array) { + const orderedTags: Array = [...spanTags]; + 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)) { + return -1; + } + + if (!a.key.startsWith(p) && b.key.startsWith(p)) { + return 1; + } + } + return a.key.localeCompare(b.key); + }); + return orderedTags; +} + /** * NOTE: Mutates `data` - Transform the HTTP response data into the form the app * generally requires. @@ -109,10 +128,7 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[] span.tags = span.tags || []; span.references = span.references || []; const tagsInfo = deduplicateTags(span.tags); - tagsInfo.tags.sort((a, b) => { - return a.key.localeCompare(b.key); - }); - span.tags = tagsInfo.tags; + 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; diff --git a/packages/jaeger-ui/src/types/config.tsx b/packages/jaeger-ui/src/types/config.tsx index 9527b7c068..683c82a134 100644 --- a/packages/jaeger-ui/src/types/config.tsx +++ b/packages/jaeger-ui/src/types/config.tsx @@ -36,6 +36,7 @@ export type Config = { menu: (ConfigMenuGroup | ConfigMenuItem)[]; search?: { maxLookback: { label: string; value: string } }; scripts?: TScript[]; + topTagPrefixes?: string[]; tracking?: { gaID: string | TNil; trackErrors: boolean | TNil; From 23af4a88c4bfd8b4474312cb6410c0b841e419a3 Mon Sep 17 00:00:00 2001 From: Lev Popov Date: Thu, 28 Nov 2019 15:53:29 +0100 Subject: [PATCH 3/4] test tags deduplication Signed-off-by: Lev Popov --- .../src/model/transform-trace-data.test.js | 20 ++++++++++++++++++- .../src/model/transform-trace-data.tsx | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/jaeger-ui/src/model/transform-trace-data.test.js b/packages/jaeger-ui/src/model/transform-trace-data.test.js index 28abae417a..8d7c35620b 100644 --- a/packages/jaeger-ui/src/model/transform-trace-data.test.js +++ b/packages/jaeger-ui/src/model/transform-trace-data.test.js @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { orderTags } from './transform-trace-data'; +import { orderTags, deduplicateTags } from './transform-trace-data'; describe('orderTags()', () => { it('correctly orders tags', () => { @@ -31,3 +31,21 @@ describe('orderTags()', () => { ]); }); }); + +describe('deduplicateTags()', () => { + it('deduplicates tags', () => { + const tagsInfo = deduplicateTags([ + { key: 'b.ip', value: '8.8.4.4' }, + { key: 'b.ip', value: '8.8.8.8' }, + { key: 'b.ip', value: '8.8.4.4' }, + { key: 'a.ip', value: '8.8.8.8' }, + ]); + + expect(tagsInfo.tags).toEqual([ + { key: 'b.ip', value: '8.8.4.4' }, + { key: 'b.ip', value: '8.8.8.8' }, + { key: 'a.ip', value: '8.8.8.8' }, + ]); + expect(tagsInfo.warnings).toEqual(['Duplicate tag "b.ip: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 index 1b23c33890..9efa681f58 100644 --- a/packages/jaeger-ui/src/model/transform-trace-data.tsx +++ b/packages/jaeger-ui/src/model/transform-trace-data.tsx @@ -19,7 +19,7 @@ import { getConfigValue } from '../utils/config/get-config'; import { KeyValuePair, Span, SpanData, Trace, TraceData } from '../types/trace'; import TreeNode from '../utils/TreeNode'; -function deduplicateTags(spanTags: Array) { +export function deduplicateTags(spanTags: Array) { const warningsHash: Map = new Map(); const tags: Array = spanTags.reduce>((uniqueTags, tag) => { if (!uniqueTags.some(t => t.key === tag.key && t.value === tag.value)) { From 2eb7676dba03e85c0b259383ce9461ce6b3efc45 Mon Sep 17 00:00:00 2001 From: Lev Popov Date: Thu, 5 Dec 2019 10:53:53 +0100 Subject: [PATCH 4/4] reflect PR feedback Signed-off-by: Lev Popov --- .../src/constants/default-config.tsx | 1 - .../src/model/transform-trace-data.test.js | 10 ++++-- .../src/model/transform-trace-data.tsx | 36 +++++++++++++------ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/packages/jaeger-ui/src/constants/default-config.tsx b/packages/jaeger-ui/src/constants/default-config.tsx index ac133f849e..a7b332cb89 100644 --- 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 index 8d7c35620b..99c2bbad25 100644 --- 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 index 9efa681f58..6fc358a7cf 100644 --- 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) { +// exported for tests +export function deduplicateTags(spanTags: KeyValuePair[]) { const warningsHash: Map = new Map(); - const tags: Array = spanTags.reduce>((uniqueTags, tag) => { + const tags: KeyValuePair[] = spanTags.reduce((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) { return { tags, warnings }; } -export function orderTags(spanTags: Array, topPrefixes: Array) { - const orderedTags: Array = [...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;