addressing PR comments

elastic · Feb 23, 2021 · 9aeb9ef · 9aeb9ef
1 parent 720ff46
commit 9aeb9ef
Show file tree

Hide file tree

Showing 6 changed files with 236 additions and 249 deletions.
diff --git a/x-pack/plugins/apm/public/components/app/service_overview/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/index.tsx
@@ -57,7 +57,11 @@ export function ServiceOverview({
   return (
     <AnnotationsContextProvider>
       <ChartPointerEventContextProvider>
-        <SearchBar prepend={transactionTypeLabel} showCorrelations />
+        <SearchBar
+          prepend={transactionTypeLabel}
+          showCorrelations
+          showTimeComparison
+        />
         <EuiPage>
           <EuiFlexGroup direction="column" gutterSize="s">
             <EuiFlexItem>

diff --git a/...ublic/components/app/service_overview/service_overview_transactions_table/get_columns.tsx b/...ublic/components/app/service_overview/service_overview_transactions_table/get_columns.tsx
@@ -17,7 +17,7 @@ import {
 } from '../../../../../common/utils/formatters';
 import { APIReturnType } from '../../../../services/rest/createCallApmApi';
 import { px, unit } from '../../../../style/variables';
-import { ComparisonSparkPlot } from '../../../shared/charts/comparison_spark_plot';
+import { SparkPlot } from '../../../shared/charts/spark_plot';
 import { ImpactBar } from '../../../shared/ImpactBar';
 import { TransactionDetailLink } from '../../../shared/Links/apm/transaction_detail_link';
 import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip';
@@ -34,7 +34,7 @@ export function getColumns({
   serviceName,
   latencyAggregationType,
   transactionGroupComparisonStatistics,
-  comparisonEnabled = true,
+  comparisonEnabled,
 }: {
   serviceName: string;
   latencyAggregationType?: LatencyAggregationType;
@@ -78,7 +78,7 @@ export function getColumns({
         const previousTimeseries =
           transactionGroupComparisonStatistics?.previousPeriod?.[name]?.latency;
         return (
-          <ComparisonSparkPlot
+          <SparkPlot
             color="euiColorVis1"
             compact
             series={currentTimeseries}
@@ -106,7 +106,7 @@ export function getColumns({
           transactionGroupComparisonStatistics?.previousPeriod?.[name]
             ?.throughput;
         return (
-          <ComparisonSparkPlot
+          <SparkPlot
             color="euiColorVis0"
             compact
             series={currentTimeseries}
@@ -134,7 +134,7 @@ export function getColumns({
           transactionGroupComparisonStatistics?.previousPeriod?.[name]
             ?.errorRate;
         return (
-          <ComparisonSparkPlot
+          <SparkPlot
             color="euiColorVis7"
             compact
             series={currentTimeseries}

diff --git a/x-pack/plugins/apm/public/components/shared/charts/comparison_spark_plot/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/comparison_spark_plot/index.tsx
diff --git a/x-pack/plugins/apm/public/components/shared/charts/spark_plot/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/spark_plot/index.tsx
@@ -12,6 +12,7 @@ import {
   AreaSeries,
   Chart,
   CurveType,
+  LineSeries,
   ScaleType,
   Settings,
 } from '@elastic/charts';
@@ -20,6 +21,7 @@ import { Coordinate } from '../../../../../typings/timeseries';
 import { useChartTheme } from '../../../../../../observability/public';
 import { px, unit } from '../../../../style/variables';
 import { useTheme } from '../../../../hooks/use_theme';
+import { getComparisonChartTheme } from '../../time_comparison/get_time_range_comparison';
 
 export type Color =
   | 'euiColorVis0'
@@ -33,19 +35,27 @@ export type Color =
   | 'euiColorVis8'
   | 'euiColorVis9';
 
+function isEmptyTimeseries(series: Coordinate[]) {
+  return series.every((point) => point.y === null);
+}
+
 export function SparkPlot({
   color,
   series,
+  comparisonSeries = [],
   valueLabel,
   compact,
 }: {
   color: Color;
   series?: Coordinate[] | null;
   valueLabel: React.ReactNode;
   compact?: boolean;
+  comparisonSeries?: Coordinate[];
 }) {
   const theme = useTheme();
   const defaultChartTheme = useChartTheme();
+  const comparisonChartTheme = getComparisonChartTheme(theme);
+  const hasComparisonSeries = !!comparisonSeries?.length;
 
   const sparkplotChartTheme = merge({}, defaultChartTheme, {
     chartMargins: { left: 0, right: 0, top: 0, bottom: 0 },
@@ -55,6 +65,7 @@ export function SparkPlot({
     areaSeriesStyle: {
       point: { opacity: 0 },
     },
+    ...(hasComparisonSeries ? comparisonChartTheme : {}),
   });
 
   const colorValue = theme.eui[color];
@@ -64,10 +75,12 @@ export function SparkPlot({
     width: compact ? px(unit * 3) : px(unit * 4),
   };
 
+  const SparklinesSeries = hasComparisonSeries ? LineSeries : AreaSeries;
+
   return (
     <EuiFlexGroup gutterSize="m" responsive={false}>
       <EuiFlexItem grow={false}>
-        {!series || series.every((point) => point.y === null) ? (
+        {!series || isEmptyTimeseries(series) ? (
           <EuiIcon type="visLine" color="subdued" style={chartSize} />
         ) : (
           <Chart size={chartSize}>
@@ -76,8 +89,8 @@ export function SparkPlot({
               showLegend={false}
               tooltip="none"
             />
-            <AreaSeries
-              id="area"
+            <SparklinesSeries
+              id="sparklinesSeries"
               xScaleType={ScaleType.Time}
               yScaleType={ScaleType.Linear}
               xAccessor={'x'}
@@ -86,6 +99,18 @@ export function SparkPlot({
               color={colorValue}
               curve={CurveType.CURVE_MONOTONE_X}
             />
+            {hasComparisonSeries && (
+              <AreaSeries
+                id="comparisonSeries"
+                xScaleType={ScaleType.Time}
+                yScaleType={ScaleType.Linear}
+                xAccessor={'x'}
+                yAccessors={['y']}
+                data={comparisonSeries}
+                color={theme.eui.euiColorLightestShade}
+                curve={CurveType.CURVE_MONOTONE_X}
+              />
+            )}
           </Chart>
         )}
       </EuiFlexItem>