TraceViewer: Fix show log marker in spanbar (grafana#30742)

* TraceViewer: Fix show log marker in spanbar * Revert changes and use UITooltip and UIPopover * Change logmarker color * e2e: wait for trace view * Record cypress run ONLY FOR TESTING * Move fixture to e2e * Get details TEST ONLY * test * Use jaeger's theme instead of grafana's * Revert "Record cypress run ONLY FOR TESTING" This reverts commit 35c087b. * Revert "Move fixture to e2e" This reverts commit fab88cc.
clear-street · Feb 24, 2021 · 04a067e · 04a067e
1 parent 466462d
commit 04a067e
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 45 deletions.
diff --git a/e2e/suite1/specs/trace-view-scrolling.spec.ts b/e2e/suite1/specs/trace-view-scrolling.spec.ts
@@ -3,9 +3,11 @@ import { e2e } from '@grafana/e2e';
 describe('Trace view', () => {
   it('Can lazy load big traces', () => {
     e2e.flows.login('admin', 'admin');
-    e2e().intercept('GET', '/api/traces/long-trace', {
-      fixture: 'long-trace-response.json',
-    });
+    e2e()
+      .intercept('GET', '/api/traces/long-trace', {
+        fixture: 'long-trace-response.json',
+      })
+      .as('longTrace');
 
     e2e.pages.Explore.visit();
 
@@ -21,7 +23,10 @@ describe('Trace view', () => {
 
     e2e.components.RefreshPicker.runButton().should('be.visible').click();
 
+    e2e().wait('@longTrace');
+
     e2e.components.TraceViewer.spanBar().should('have.length', 100);
+
     e2e.pages.Explore.General.scrollBar().scrollTo('center');
 
     // After scrolling we should have 140 spans instead of the first 100

diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js
@@ -29,6 +29,7 @@ describe('<SpanBar>', () => {
     hintSide: 'right',
     viewEnd: 1,
     viewStart: 0,
+    theme: {},
     getViewedBounds: (s) => {
       // Log entries
       if (s === 10) {
@@ -44,7 +45,7 @@ describe('<SpanBar>', () => {
       viewEnd: 0.75,
       color: '#000',
     },
-    tracestartTime: 0,
+    traceStartTime: 0,
     span: {
       logs: [
         {
@@ -79,12 +80,12 @@ describe('<SpanBar>', () => {
       </UIElementsContext.Provider>
     );
     expect(wrapper).toBeDefined();
-    const { onMouseOver, onMouseOut } = wrapper.find('[data-test-id="SpanBar--wrapper"]').props();
+    const { onMouseOver, onMouseLeave } = wrapper.find('[data-test-id="SpanBar--wrapper"]').props();
     const labelElm = wrapper.find('[data-test-id="SpanBar--label"]');
     expect(labelElm.text()).toBe(shortLabel);
     onMouseOver();
     expect(labelElm.text()).toBe(longLabel);
-    onMouseOut();
+    onMouseLeave();
     expect(labelElm.text()).toBe(shortLabel);
   });
 

diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.tsx
@@ -12,21 +12,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import React from 'react';
-import _groupBy from 'lodash/groupBy';
-import { onlyUpdateForKeys, compose, withState, withProps } from 'recompose';
-import { css } from 'emotion';
+import { TraceSpan } from '@grafana/data';
 import cx from 'classnames';
-
-import AccordianLogs from './SpanDetail/AccordianLogs';
-
-import { ViewedBoundsFunctionType } from './utils';
+import { css } from 'emotion';
+import _groupBy from 'lodash/groupBy';
+import React from 'react';
+import { compose, onlyUpdateForKeys, withProps, withState } from 'recompose';
+import { autoColor, createStyle, Theme } from '../Theme';
 import { TNil } from '../types';
-import { TraceSpan } from '@grafana/data';
 import { UIPopover } from '../uiElementsContext';
-import { createStyle } from '../Theme';
+import AccordianLogs from './SpanDetail/AccordianLogs';
+import { ViewedBoundsFunctionType } from './utils';
 
-const getStyles = createStyle(() => {
+const getStyles = createStyle((theme: Theme) => {
   return {
     wrapper: css`
       label: wrapper;
@@ -65,14 +63,14 @@ const getStyles = createStyle(() => {
     `,
     logMarker: css`
       label: logMarker;
-      background-color: rgba(0, 0, 0, 0.5);
+      background-color: ${autoColor(theme, '#2c3235')};
       cursor: pointer;
       height: 60%;
       min-width: 1px;
       position: absolute;
       top: 20%;
       &:hover {
-        background-color: #000;
+        background-color: ${autoColor(theme, '#464c54')};
       }
       &::before,
       &::after {
@@ -87,20 +85,11 @@ const getStyles = createStyle(() => {
         left: 0;
       }
     `,
-    logHint: css`
-      label: logHint;
-      pointer-events: none;
-      // TODO won't work with different UI elements injected
-      & .ant-popover-inner-content {
-        padding: 0.25rem;
-      }
-    `,
   };
 });
 
 type TCommonProps = {
   color: string;
-  // onClick: (evt: React.MouseEvent<any>) => void;
   onClick?: (evt: React.MouseEvent<any>) => void;
   viewEnd: number;
   viewStart: number;
@@ -116,6 +105,7 @@ type TCommonProps = {
   span: TraceSpan;
   className?: string;
   labelClassName?: string;
+  theme: Theme;
 };
 
 type TInnerProps = {
@@ -146,6 +136,7 @@ function SpanBar(props: TInnerProps) {
     rpc,
     traceStartTime,
     span,
+    theme,
     className,
     labelClassName,
   } = props;
@@ -155,13 +146,13 @@ function SpanBar(props: TInnerProps) {
     // round to the nearest 0.2%
     return toPercent(Math.round(posPercent * 500) / 500);
   });
-  const styles = getStyles();
+  const styles = getStyles(theme);
 
   return (
     <div
       className={cx(styles.wrapper, className)}
       onClick={onClick}
-      onMouseOut={setShortLabel}
+      onMouseLeave={setShortLabel}
       onMouseOver={setLongLabel}
       aria-hidden
       data-test-id="SpanBar--wrapper"
@@ -183,8 +174,6 @@ function SpanBar(props: TInnerProps) {
         {Object.keys(logGroups).map((positionKey) => (
           <UIPopover
             key={positionKey}
-            arrowPointAtCenter
-            overlayClassName={styles.logHint}
             placement="topLeft"
             content={
               <AccordianLogs interactive={false} isOpen logs={logGroups[positionKey]} timestamp={traceStartTime} />

diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBarRow.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBarRow.tsx
@@ -504,6 +504,7 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
             rpc={rpc}
             viewStart={viewStart}
             viewEnd={viewEnd}
+            theme={theme}
             getViewedBounds={getViewedBounds}
             color={color}
             shortLabel={label}

diff --git a/packages/jaeger-ui-components/src/uiElementsContext.tsx b/packages/jaeger-ui-components/src/uiElementsContext.tsx
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import React from 'react';
+import React, { ReactElement } from 'react';
 
 export type TooltipPlacement =
   | 'top'
@@ -28,11 +28,11 @@ export type TooltipPlacement =
   | 'rightTop'
   | 'rightBottom';
 export type PopoverProps = {
-  content?: React.ReactNode;
+  children: ReactElement;
+  content: ReactElement;
   arrowPointAtCenter?: boolean;
   overlayClassName?: string;
   placement?: TooltipPlacement;
-  children?: React.ReactNode;
 };
 
 export const UIPopover: React.ComponentType<PopoverProps> = function UIPopover(props: PopoverProps) {
@@ -45,12 +45,11 @@ export const UIPopover: React.ComponentType<PopoverProps> = function UIPopover(p
   );
 };
 
-type RenderFunction = () => React.ReactNode;
 export type TooltipProps = {
-  title?: React.ReactNode | RenderFunction;
+  title: string | ReactElement;
   getPopupContainer?: (triggerNode: Element) => HTMLElement;
   overlayClassName?: string;
-  children?: React.ReactNode;
+  children: ReactElement;
   placement?: TooltipPlacement;
   mouseLeaveDelay?: number;
   arrowPointAtCenter?: boolean;

diff --git a/public/app/features/explore/TraceView/uiElements.tsx b/public/app/features/explore/TraceView/uiElements.tsx
@@ -1,9 +1,17 @@
-import React from 'react';
-import { ButtonProps, Elements } from '@jaegertracing/jaeger-ui-components';
-import { Button, Input, stylesFactory, useTheme } from '@grafana/ui';
-import { css } from 'emotion';
 import { GrafanaTheme } from '@grafana/data';
+import {
+  Button,
+  Input,
+  Popover,
+  PopoverController,
+  stylesFactory,
+  Tooltip as GrafanaTooltip,
+  useTheme,
+} from '@grafana/ui';
+import { ButtonProps, Elements, PopoverProps, TooltipProps } from '@jaegertracing/jaeger-ui-components';
 import cx from 'classnames';
+import { css } from 'emotion';
+import React, { useRef } from 'react';
 
 /**
  * Right now Jaeger components need some UI elements to be injected. This is to get rid of AntD UI library that was
@@ -12,15 +20,45 @@ import cx from 'classnames';
 
 // This needs to be static to prevent remounting on every render.
 export const UIElements: Elements = {
-  Popover: (() => null as any) as any,
-  Tooltip: (() => null as any) as any,
+  Popover({ children, content, overlayClassName }: PopoverProps) {
+    const popoverRef = useRef<HTMLElement>(null);
+
+    return (
+      <PopoverController content={content} hideAfter={300}>
+        {(showPopper, hidePopper, popperProps) => {
+          return (
+            <>
+              {popoverRef.current && (
+                <Popover
+                  {...popperProps}
+                  referenceElement={popoverRef.current}
+                  wrapperClassName={overlayClassName}
+                  onMouseLeave={hidePopper}
+                  onMouseEnter={showPopper}
+                />
+              )}
+
+              {React.cloneElement(children, {
+                ref: popoverRef,
+                onMouseEnter: showPopper,
+                onMouseLeave: hidePopper,
+              })}
+            </>
+          );
+        }}
+      </PopoverController>
+    );
+  },
+  Tooltip({ children, title }: TooltipProps) {
+    return <GrafanaTooltip content={title}>{children}</GrafanaTooltip>;
+  },
   Icon: (() => null as any) as any,
   Dropdown: (() => null as any) as any,
   Menu: (() => null as any) as any,
   MenuItem: (() => null as any) as any,
   Button({ onClick, children, className }: ButtonProps) {
     return (
-      <Button variant={'secondary'} onClick={onClick} className={className}>
+      <Button variant="secondary" onClick={onClick} className={className}>
         {children}
       </Button>
     );