Traces: More visible span colors (#54513)

* Use colors array from position 0 for span colors

* Update/add tests

* Use service name hex to get color

* getTheme

* Only add colors that have a contrast ratio >= 3 for the current theme

* Add new tests

packages/jaeger-ui-components/src/TracePageHeader/SpanGraph/CanvasSpanGraph.tsx

@@ -50,7 +50,7 @@ export class UnthemedCanvasSpanGraph extends React.PureComponent<CanvasSpanGraph
     this._canvasElm = undefined;
   }
 
-  getColor = (key: string) => getRgbColorByKey(key);
+  getColor = (key: string) => getRgbColorByKey(key, this.props.theme);
 
   componentDidMount() {
     this._draw();

packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.tsx

@@ -395,12 +395,13 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
       createSpanLink,
       focusedSpanId,
       focusedSpanIdForSearch,
+      theme,
     } = this.props;
     // to avert flow error
     if (!trace) {
       return null;
     }
-    const color = getColorByKey(serviceName);
+    const color = getColorByKey(serviceName, theme);
     const isCollapsed = childrenHiddenIDs.has(spanID);
     const isDetailExpanded = detailStates.has(spanID);
     const isMatchingFilter = findMatchesIDs ? findMatchesIDs.has(spanID) : false;
@@ -414,7 +415,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
       if (rpcSpan) {
         const rpcViewBounds = this.getViewedBounds()(rpcSpan.startTime, rpcSpan.startTime + rpcSpan.duration);
         rpc = {
-          color: getColorByKey(rpcSpan.process.serviceName),
+          color: getColorByKey(rpcSpan.process.serviceName, theme),
           operationName: rpcSpan.operationName,
           serviceName: rpcSpan.process.serviceName,
           viewEnd: rpcViewBounds.end,
@@ -430,7 +431,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
     if (!span.hasChildren && peerServiceKV && isKindClient(span)) {
       noInstrumentedServer = {
         serviceName: peerServiceKV.value,
-        color: getColorByKey(peerServiceKV.value),
+        color: getColorByKey(peerServiceKV.value, theme),
       };
     }
 
@@ -490,12 +491,13 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
       focusedSpanId,
       createFocusSpanLink,
       topOfViewRefType,
+      theme,
     } = this.props;
     const detailState = detailStates.get(spanID);
     if (!trace || !detailState) {
       return null;
     }
-    const color = getColorByKey(serviceName);
+    const color = getColorByKey(serviceName, theme);
     const styles = getStyles(this.props);
     return (
       <div className={styles.row} key={key} style={{ ...style, zIndex: 1 }} {...attrs}>

packages/jaeger-ui-components/src/utils/color-generator.test.js

@@ -12,26 +12,45 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import { getColorByKey, clear } from './color-generator';
+import { createTheme } from '@grafana/data';
+import { colors } from '@grafana/ui';
+
+import { getColorByKey, getFilteredColors, clear } from './color-generator';
+
+const colorsToFilter = [...colors];
 
 it('gives the same color for the same key', () => {
   clear();
-  const colorOne = getColorByKey('serviceA');
+  const colorOne = getColorByKey('serviceA', createTheme());
-  const colorTwo = getColorByKey('serviceA');
+  const colorTwo = getColorByKey('serviceA', createTheme());
   expect(colorOne).toBe(colorTwo);
 });
 
 it('gives different colors for each for each key', () => {
   clear();
-  const colorOne = getColorByKey('serviceA');
+  const colorOne = getColorByKey('serviceA', createTheme());
-  const colorTwo = getColorByKey('serviceB');
+  const colorTwo = getColorByKey('serviceB', createTheme());
   expect(colorOne).not.toBe(colorTwo);
 });
 
 it('should not allow red', () => {
-  clear();
+  expect(colorsToFilter.indexOf('#E24D42')).toBe(4);
-  // when aPAKNMeFcF is hashed it's index is 4
+  const filteredColors = getFilteredColors(colorsToFilter, createTheme());
-  // which is red, which we disallow because it looks like an error
+  expect(filteredColors.indexOf('#E24D42')).toBe(-1);
-  const colorOne = getColorByKey('aPAKNMeFcF');
+});
-  expect(colorOne).not.toBe('#E24D42');
+
+it('should not allow colors with a contrast ratio < 3 in light mode', () => {
+  expect(colorsToFilter.indexOf('#7EB26D')).toBe(0);
+  expect(colorsToFilter.indexOf('#EAB839')).toBe(1);
+  const filteredColors = getFilteredColors(colorsToFilter, createTheme({ colors: { mode: 'light' } }));
+  expect(filteredColors.indexOf('#7EB26D')).toBe(-1);
+  expect(filteredColors.indexOf('#EAB839')).toBe(-1);
+});
+
+it('should not allow colors with a contrast ratio < 3 in dark mode', () => {
+  expect(colorsToFilter.indexOf('#890F02')).toBe(11);
+  expect(colorsToFilter.indexOf('#0A437C')).toBe(12);
+  const filteredColors = getFilteredColors(colorsToFilter, createTheme({ colors: { mode: 'dark' } }));
+  expect(filteredColors.indexOf('#890F02')).toBe(-1);
+  expect(filteredColors.indexOf('#0A437C')).toBe(-1);
 });

packages/jaeger-ui-components/src/utils/color-generator.tsx

@@ -13,7 +13,9 @@
 // limitations under the License.
 
 import memoizeOne from 'memoize-one';
+import tinycolor from 'tinycolor2';
 
+import { GrafanaTheme2 } from '@grafana/data';
 import { colors } from '@grafana/ui';
 
 // TS needs the precise return type
@@ -32,9 +34,10 @@ class ColorGenerator {
   colorsRgb: Array<[number, number, number]>;
   cache: Map<string, number>;
 
-  constructor(colorsHex: string[]) {
+  constructor(colorsHex: string[], theme: GrafanaTheme2) {
-    this.colorsHex = colorsHex;
+    const filteredColors = getFilteredColors(colorsHex, theme);
-    this.colorsRgb = colorsHex.map(strToRgb);
+    this.colorsHex = filteredColors;
+    this.colorsRgb = filteredColors.map(strToRgb);
     this.cache = new Map();
   }
 
@@ -43,7 +46,6 @@ class ColorGenerator {
     if (i == null) {
       const hash = this.hashCode(key.toLowerCase());
       const hashIndex = Math.abs(hash % this.colorsHex.length);
-      // colors[4] is red (which we want to disallow as a span color because it looks like an error)
       i = hashIndex === 4 ? hashIndex + 1 : hashIndex;
       this.cache.set(key, i);
     }
@@ -86,18 +88,36 @@ class ColorGenerator {
   }
 }
 
-const getGenerator = memoizeOne((colors: string[]) => {
+const getGenerator = memoizeOne((colors: string[], theme: GrafanaTheme2) => {
-  return new ColorGenerator(colors);
+  return new ColorGenerator(colors, theme);
 });
 
-export function clear() {
+export function clear(theme: GrafanaTheme2) {
-  getGenerator([]);
+  getGenerator([], theme);
 }
 
-export function getColorByKey(key: string) {
+export function getColorByKey(key: string, theme: GrafanaTheme2) {
-  return getGenerator(colors).getColorByKey(key);
+  return getGenerator(colors, theme).getColorByKey(key);
 }
 
-export function getRgbColorByKey(key: string): [number, number, number] {
+export function getRgbColorByKey(key: string, theme: GrafanaTheme2): [number, number, number] {
-  return getGenerator(colors).getRgbColorByKey(key);
+  return getGenerator(colors, theme).getRgbColorByKey(key);
+}
+
+export function getFilteredColors(colorsHex: string[], theme: GrafanaTheme2) {
+  // Remove red as a span color because it looks like an error
+  const redIndex = colorsHex.indexOf('#E24D42');
+  if (redIndex > -1) {
+    colorsHex.splice(redIndex, 1);
+  }
+
+  // Only add colors that have a contrast ratio >= 3 for the current theme
+  let filteredColors = [];
+  for (const color of colorsHex) {
+    if (tinycolor.readability(theme.colors.background.primary, color) >= 3) {
+      filteredColors.push(color);
+    }
+  }
+
+  return filteredColors;
 }