From 792fe92fe3c9b9fc94b4c469ba34ebef2dc2b787 Mon Sep 17 00:00:00 2001 From: Rico Berger Date: Thu, 19 Aug 2021 22:05:44 +0200 Subject: [PATCH] Fix span order and additional fields for Jaeger (#114) The Jaeger plugin had two bugs, where spans were not shown in the correct order, because we didn't use the start time of the root span, but the start time of the first span in a trace and we didn't sorted the spans by there start date before we build the our span tree. These two bugs are fixed now. We also fixed a bug, where the additional fields from the options modal on the Jaeger page, where not used to get the list of traces, because we didn't changed the additional fields data in the toolbar component. We also improved the loading indication for several plugins by not keeping the previous data. This feature is now only used for the dashboard variables and the Prometheus plugin, where we show the spinner in the panel header when new data is loaded. Last but not least we improved the time handling in the Options component of the core package, so that we keep the selected time range when a user only changes an additional field. --- CHANGELOG.md | 1 + plugins/core/src/components/misc/Options.tsx | 15 +++++++++++++++ .../jaeger/src/components/page/TraceCompare.tsx | 1 - plugins/jaeger/src/components/page/Traces.tsx | 4 ++-- .../jaeger/src/components/page/TracesToolbar.tsx | 11 +++++++---- .../components/page/TracesToolbarOperations.tsx | 2 +- .../src/components/page/TracesToolbarServices.tsx | 2 +- plugins/jaeger/src/components/panel/Traces.tsx | 1 - .../jaeger/src/components/panel/details/Spans.tsx | 9 +++++++-- plugins/jaeger/src/utils/helpers.ts | 8 ++++++++ plugins/opsgenie/src/components/panel/Alerts.tsx | 1 - .../opsgenie/src/components/panel/Incidents.tsx | 1 - .../src/components/panel/details/Logs.tsx | 1 - .../src/components/panel/details/Notes.tsx | 1 - .../components/panel/details/alert/Details.tsx | 1 - .../panel/details/incident/Timeline.tsx | 1 - plugins/rss/src/components/panel/Feed.tsx | 1 - 17 files changed, 42 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3ac8370d..70e42273d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ NOTE: As semantic versioning states all 0.y.z releases can contain breaking chan - [#109](https://github.com/kobsio/kobs/pull/109): Fix tooltip position in Prometheus charts. - [#110](https://github.com/kobsio/kobs/pull/110): Fix Dashboard tabs showing wrong variables. - [#111](https://github.com/kobsio/kobs/pull/111): Fix usage of `memo` in Dashboards and fix resources table for CRDs when a value is undefined. +- [#114](https://github.com/kobsio/kobs/pull/114): Fix span order and additional fields for Jaeger plugin. ### Changed diff --git a/plugins/core/src/components/misc/Options.tsx b/plugins/core/src/components/misc/Options.tsx index 649b4c27f..35f52e642 100644 --- a/plugins/core/src/components/misc/Options.tsx +++ b/plugins/core/src/components/misc/Options.tsx @@ -142,6 +142,21 @@ export const Options: React.FunctionComponent = ({ // change the start and end time to the new values. If the string couldn't be parsed, the user will see an error below // the corresponding input field. const apply = (): void => { + // If the time wasn't changed by the user, we keep the selected time interval and only refresh the time for the + // selected interval and change the additional fields. This allows a user to adjust an additional field without + // switching to a custom time interval. + if (customTimeEnd === formatTime(timeEnd) && customTimeStart === formatTime(timeStart) && time !== 'custom') { + setOptions( + false, + additionalFields, + time, + Math.floor(Date.now() / 1000), + Math.floor(Date.now() / 1000) - times[time].seconds, + ); + setShow(false); + return; + } + // Get a new date object for the users current timezone. This allows us to ignore the timezone, while parsing the // provided time string. The parsed date object will be in UTC, to transform the parsed date into the users timezone // we have to add the minutes between UTC and the users timezon (getTimezoneOffset()). diff --git a/plugins/jaeger/src/components/page/TraceCompare.tsx b/plugins/jaeger/src/components/page/TraceCompare.tsx index 8e2787e33..6601e4e9c 100644 --- a/plugins/jaeger/src/components/page/TraceCompare.tsx +++ b/plugins/jaeger/src/components/page/TraceCompare.tsx @@ -54,7 +54,6 @@ const TraceCompare: React.FunctionComponent = ({ name, trace throw err; } }, - { keepPreviousData: true }, ); if (isLoading) { diff --git a/plugins/jaeger/src/components/page/Traces.tsx b/plugins/jaeger/src/components/page/Traces.tsx index e94a63d14..0cd1abd76 100644 --- a/plugins/jaeger/src/components/page/Traces.tsx +++ b/plugins/jaeger/src/components/page/Traces.tsx @@ -10,8 +10,8 @@ import { Link, useHistory, useLocation } from 'react-router-dom'; import React, { useEffect, useState } from 'react'; import { IOptions } from '../../utils/interfaces'; -import PageToolbar from './TracesToolbar'; import TracesPanel from '../panel/Traces'; +import TracesToolbar from './TracesToolbar'; import { getOptionsFromSearch } from '../../utils/helpers'; interface ITracesProps { @@ -55,7 +55,7 @@ const Traces: React.FunctionComponent = ({ name, displayName, desc

{description}

- void; } -const PageToolbar: React.FunctionComponent = ({ +const TracesToolbar: React.FunctionComponent = ({ name, limit, maxDuration, @@ -31,7 +31,7 @@ const PageToolbar: React.FunctionComponent = ({ tags, times, setOptions, -}: IPageToolbarProps) => { +}: ITracesToolbarProps) => { const [data, setData] = useState({ limit: limit, maxDuration: maxDuration, @@ -64,6 +64,9 @@ const PageToolbar: React.FunctionComponent = ({ setData({ ...tmpData, + limit: additionalFields[0].value, + maxDuration: additionalFields[1].value, + minDuration: additionalFields[2].value, times: { time: time, timeEnd: timeEnd, timeStart: timeStart }, }); } @@ -141,4 +144,4 @@ const PageToolbar: React.FunctionComponent = ({ ); }; -export default PageToolbar; +export default TracesToolbar; diff --git a/plugins/jaeger/src/components/page/TracesToolbarOperations.tsx b/plugins/jaeger/src/components/page/TracesToolbarOperations.tsx index cad84eceb..524a1d1b6 100644 --- a/plugins/jaeger/src/components/page/TracesToolbarOperations.tsx +++ b/plugins/jaeger/src/components/page/TracesToolbarOperations.tsx @@ -29,7 +29,7 @@ const TracesToolbarOperations: React.FunctionComponent= 200 && response.status < 300) { - return ['All Operations', ...json.data.map((operation: IOperation) => operation.name)]; + return ['All Operations', ...json.data.map((operation: IOperation) => operation.name).sort()]; } else { if (json.error) { throw new Error(json.error); diff --git a/plugins/jaeger/src/components/page/TracesToolbarServices.tsx b/plugins/jaeger/src/components/page/TracesToolbarServices.tsx index fe95855c3..3c871554a 100644 --- a/plugins/jaeger/src/components/page/TracesToolbarServices.tsx +++ b/plugins/jaeger/src/components/page/TracesToolbarServices.tsx @@ -23,7 +23,7 @@ const TracesToolbarServices: React.FunctionComponent= 200 && response.status < 300) { - return json.data; + return json.data.sort(); } else { if (json.error) { throw new Error(json.error); diff --git a/plugins/jaeger/src/components/panel/Traces.tsx b/plugins/jaeger/src/components/panel/Traces.tsx index 24ad23959..89cff64ec 100644 --- a/plugins/jaeger/src/components/panel/Traces.tsx +++ b/plugins/jaeger/src/components/panel/Traces.tsx @@ -58,7 +58,6 @@ const Traces: React.FunctionComponent = ({ throw err; } }, - { keepPreviousData: true }, ); return ( diff --git a/plugins/jaeger/src/components/panel/details/Spans.tsx b/plugins/jaeger/src/components/panel/details/Spans.tsx index e033edf99..b0b730dd5 100644 --- a/plugins/jaeger/src/components/panel/details/Spans.tsx +++ b/plugins/jaeger/src/components/panel/details/Spans.tsx @@ -2,7 +2,7 @@ import { Accordion, Card, CardBody } from '@patternfly/react-core'; import React from 'react'; import { ISpan, ITrace } from '../../../utils/interfaces'; -import { createSpansTree, getDuration } from '../../../utils/helpers'; +import { createSpansTree, getDuration, getRootSpan } from '../../../utils/helpers'; import Span from './Span'; import SpansChart from './SpansChart'; @@ -12,8 +12,13 @@ export interface ISpansProps { } const Spans: React.FunctionComponent = ({ name, trace }: ISpansProps) => { + const rootSpan = trace.spans.length > 0 ? getRootSpan(trace.spans) : undefined; + if (!rootSpan) { + return null; + } + const duration = getDuration(trace.spans); - const spans: ISpan[] = createSpansTree(trace.spans, trace.spans[0].startTime, duration); + const spans: ISpan[] = createSpansTree(trace.spans, rootSpan.startTime, duration); return ( diff --git a/plugins/jaeger/src/utils/helpers.ts b/plugins/jaeger/src/utils/helpers.ts index 6d6009337..272e949b4 100644 --- a/plugins/jaeger/src/utils/helpers.ts +++ b/plugins/jaeger/src/utils/helpers.ts @@ -174,6 +174,14 @@ export const createSpansTree = (spans: ISpan[], traceStartTime: number, duration const map: IMap = {}; const roots: ISpan[] = []; + spans.sort((a, b) => { + if (a.startTime < b.startTime) { + return -1; + } + + return 1; + }); + for (let i = 0; i < spans.length; i++) { map[spans[i].spanID] = i; spans[i].childs = []; diff --git a/plugins/opsgenie/src/components/panel/Alerts.tsx b/plugins/opsgenie/src/components/panel/Alerts.tsx index bb42ee2b7..a29e7655a 100644 --- a/plugins/opsgenie/src/components/panel/Alerts.tsx +++ b/plugins/opsgenie/src/components/panel/Alerts.tsx @@ -37,7 +37,6 @@ const Alerts: React.FunctionComponent = ({ name, query, times, set throw err; } }, - { keepPreviousData: true }, ); if (isLoading) { diff --git a/plugins/opsgenie/src/components/panel/Incidents.tsx b/plugins/opsgenie/src/components/panel/Incidents.tsx index 571da617d..963ee5be3 100644 --- a/plugins/opsgenie/src/components/panel/Incidents.tsx +++ b/plugins/opsgenie/src/components/panel/Incidents.tsx @@ -37,7 +37,6 @@ const Incidents: React.FunctionComponent = ({ name, query, time throw err; } }, - { keepPreviousData: true }, ); if (isLoading) { diff --git a/plugins/opsgenie/src/components/panel/details/Logs.tsx b/plugins/opsgenie/src/components/panel/details/Logs.tsx index 3beaf2716..30b6d7eda 100644 --- a/plugins/opsgenie/src/components/panel/details/Logs.tsx +++ b/plugins/opsgenie/src/components/panel/details/Logs.tsx @@ -43,7 +43,6 @@ const Logs: React.FunctionComponent = ({ name, id, type }: ILogsProp throw err; } }, - { keepPreviousData: true }, ); if (isLoading) { diff --git a/plugins/opsgenie/src/components/panel/details/Notes.tsx b/plugins/opsgenie/src/components/panel/details/Notes.tsx index 4ef11666b..f775a428f 100644 --- a/plugins/opsgenie/src/components/panel/details/Notes.tsx +++ b/plugins/opsgenie/src/components/panel/details/Notes.tsx @@ -43,7 +43,6 @@ const Notes: React.FunctionComponent = ({ name, id, type }: INotesP throw err; } }, - { keepPreviousData: true }, ); if (isLoading) { diff --git a/plugins/opsgenie/src/components/panel/details/alert/Details.tsx b/plugins/opsgenie/src/components/panel/details/alert/Details.tsx index f06c66d8b..dc5d38c2a 100644 --- a/plugins/opsgenie/src/components/panel/details/alert/Details.tsx +++ b/plugins/opsgenie/src/components/panel/details/alert/Details.tsx @@ -43,7 +43,6 @@ const Details: React.FunctionComponent = ({ name, id }: IDetailsP throw err; } }, - { keepPreviousData: true }, ); if (isLoading) { diff --git a/plugins/opsgenie/src/components/panel/details/incident/Timeline.tsx b/plugins/opsgenie/src/components/panel/details/incident/Timeline.tsx index 3870e5e61..9a84ae65f 100644 --- a/plugins/opsgenie/src/components/panel/details/incident/Timeline.tsx +++ b/plugins/opsgenie/src/components/panel/details/incident/Timeline.tsx @@ -42,7 +42,6 @@ const Details: React.FunctionComponent = ({ name, id }: IDetailsP throw err; } }, - { keepPreviousData: true }, ); if (isLoading) { diff --git a/plugins/rss/src/components/panel/Feed.tsx b/plugins/rss/src/components/panel/Feed.tsx index 73170926b..d9d345ad5 100644 --- a/plugins/rss/src/components/panel/Feed.tsx +++ b/plugins/rss/src/components/panel/Feed.tsx @@ -36,7 +36,6 @@ const Alerts: React.FunctionComponent = ({ urls, sortBy, setDetails throw err; } }, - { keepPreviousData: true }, ); if (isLoading) {