Add legend for duration markline.

[tirkarthi]

closes: #38420
related: #38420

I am facing two issues and not able to debug why.

1. When show landing times is not selected and on page refresh even upon checking it the legend is not added. But when show landing times is selected and on refresh it adds it to legend along with unchecking it removes it from legend.
2. On show landing times selected blue color still seems to be used for landing time though the legend says color of scheduled state to be used.

Task run duration :

Dag run duration :

Dag run duration blue color is selected for landing time too :

[tirkarthi]

Please let me know if a different color needs to be selected with respect to accessibility. I noticed sometimes when all the bars are in success state then echarts seem to select green so I specified blue explicitly for consistency.

cc: @simond @eladkal @bbovenzi

[tirkarthi]

apache/echarts#6202

Using chartInstance.clear() before setOption helps with legend issue in 1 but the issue in 2 remains.

[tirkarthi]

Using chartInstance.clear() causes the chart to be redrawn as noted in the issue losing the animations on existing chart. Fresh redraws affect UX. I also tried below patch to show the markline only when show landing pages is false but on toggling the show landing page to true the run duration markline from previous graph still gets calculated on the new graph. I tried upgrading echarts to latest version but that doesn't solve the issue.

At this point I am open to reverting the markline changes in runduration page and keep it only in task duration page to revisit this issue later to not affect the 2.9.0 release after beta since runduration is broken in the main branch too. Thoughts?

diff --git a/airflow/www/static/js/dag/details/dag/RunDurationChart.tsx b/airflow/www/static/js/dag/details/dag/RunDurationChart.tsx
index 5365c0c3df..ed2950c3c8 100644
--- a/airflow/www/static/js/dag/details/dag/RunDurationChart.tsx
+++ b/airflow/www/static/js/dag/details/dag/RunDurationChart.tsx
@@ -170,22 +170,18 @@ const RunDurationChart = ({ showLandingTimes }: Props) => {
       icon: "circle",
       formatter: formatMarkLineLegendName,
       data: [
-        ...(showLandingTimes
+        ...(!showLandingTimes
           ? [
               {
-                name: "landingDurationUnit",
-                itemStyle: { color: stateColors.scheduled },
+                name: "runDurationUnit",
+                itemStyle: { color: "blue" },
+              },
+              {
+                name: "queuedDurationUnit",
+                itemStyle: { color: stateColors.queued },
               },
             ]
           : []),
-        {
-          name: "runDurationUnit",
-          itemStyle: { color: "blue" },
-        },
-        {
-          name: "queuedDurationUnit",
-          itemStyle: { color: stateColors.queued },
-        },
       ],
     },
     series: [
@@ -199,15 +195,6 @@ const RunDurationChart = ({ showLandingTimes }: Props) => {
                 opacity: 0.6,
               },
               stack: "x",
-              markLine: {
-                silent: true,
-                data: [
-                  {
-                    type: "median",
-                    lineStyle: { color: stateColors.scheduled },
-                  },
-                ],
-              },
             } as SeriesOption,
           ]
         : []),
@@ -219,10 +206,19 @@ const RunDurationChart = ({ showLandingTimes }: Props) => {
           opacity: 0.6,
         },
         stack: "x",
-        markLine: {
-          silent: true,
-          data: [{ type: "median", lineStyle: { color: stateColors.queued } }],
-        },
+        ...(!showLandingTimes
+          ? {
+              markLine: {
+                silent: true,
+                data: [
+                  {
+                    type: "median",
+                    lineStyle: { color: stateColors.queued },
+                  },
+                ],
+              },
+            }
+          : {}),
       },
       {
         type: "bar",
@@ -233,10 +229,14 @@ const RunDurationChart = ({ showLandingTimes }: Props) => {
           color: (params) => stateColors[params.data.state],
         },
         stack: "x",
-        markLine: {
-          silent: true,
-          data: [{ type: "median", lineStyle: { color: "blue" } }],
-        },
+        ...(!showLandingTimes
+          ? {
+              markLine: {
+                silent: true,
+                data: [{ type: "median", lineStyle: { color: "red" } }],
+              },
+            }
+          : {}),
       },
     ],

[tirkarthi]

One another thing that I noticed was that the "mean run duration" was actually "mean total duration". Since the bars are stacked the value used to calculate mean was actually "queued duration + run duration" which I mistook as run duration since most of my task instances had less than few seconds of queued seconds and I assumed it was mean run duration. Below is an example of the the change in value. This is more visible when tasks take more time in queued state than actual execution where mean run duration markline will be below mean queued duration markline. But using total run means mean total run will be above queued duration markline.

To have median only for run duration we have to add "valueDim": 2 in the second markline. Is "mean total" more useful than "mean run"?

Example with mean queued, run and total plotted.

>>> import statistics
>>> queued
[1.75, 1.72, 1.9, 1.58, 1.81]
>>> run
[28.18, 20.2, 16.16, 1.19, 22.19]
>>> statistics.median(run) # mean run duration
20.2
>>> statistics.median([q + r for q, r in zip(queued, run)]) # mean total duration
21.919999999999998

[simond]

Just looking at your screenshots - you may want to change the wording to 'median total', 'median run' etc rather than 'mean' - median and mean are different things. Looks like you're using the correct one in your calculations though (median).

[tirkarthi]

@simond Thanks, I will update the legend. Please let me know if median total duration is more useful than median run duration.

[simond]

Hey @tirkarthi

I think that, given this is a stacked bar chart, a "Median Duration" should be shown. This median duration would be the median of the total which is shown on the graph.

If you had a single mark on the chart like this, you could then allow the user to toggle on/off any of the series on the chart which would allow them to decide if they want to see the median including the queued time or not.

[tirkarthi]

Thanks @simond , I have updated the string "mean" to "median". I didn't know the bars are selectable from legend which is cool and lets users to select the items they want. I have removed markline from dagrun duration since there is a bug with respect dynamically changing landing times which is not taken into account by echarts properly. I will try that in another PR. Please let me know if I missed something

Updated screenshots :

Normal chart (queued + run = total)

Only run duration

Only queued duration

[bbovenzi]

I didn't realize that echarts already allowed one to show/hide different bars.

Let's do this for tasks and refactor the run duration chart in another PR.