From 2efe5f9f107f736d672a5c5fe3f6b52d5d6bf3c5 Mon Sep 17 00:00:00 2001
From: wendrul <53628737+wendrul@users.noreply.github.com>
Date: Tue, 21 May 2024 19:28:05 +0200
Subject: [PATCH] Win 130 concurrency observability fixes (#3782)

* Resize badge and add filter by key button

* Make status filters icononly with tooltip

* nit: replace current job None with empty

* tooltip should be on the same row with flex

* Expand selected on ToggleButtonMore

* Make a common definition of UnifiedJob fields
---
 .../windmill-api/src/concurrency_groups.rs    |  73 +--------
 backend/windmill-api/src/jobs.rs              | 149 ++++++++++--------
 .../toggleButton-v2/ToggleButtonMore.svelte   |  31 ++--
 .../src/lib/components/runs/JobPreview.svelte |  21 ++-
 .../src/lib/components/runs/RunsFilter.svelte |   6 +-
 .../src/lib/components/runs/RunsTable.svelte  |   8 +-
 .../(logged)/runs/[...path]/+page.svelte      |   6 +-
 .../(root)/(logged)/workers/+page.svelte      |   2 -
 8 files changed, 131 insertions(+), 165 deletions(-)

diff --git a/backend/windmill-api/src/concurrency_groups.rs b/backend/windmill-api/src/concurrency_groups.rs
index 0a0802f6d2..8ddf3f0d0f 100644
--- a/backend/windmill-api/src/concurrency_groups.rs
+++ b/backend/windmill-api/src/concurrency_groups.rs
@@ -150,75 +150,6 @@ async fn get_concurrent_intervals(
         ));
     }
 
-    const QJ_FIELDS: &[&str] = &[
-        "'QueuedJob' as typ",
-        "id",
-        "workspace_id",
-        "parent_job",
-        "created_by",
-        "created_at",
-        "started_at",
-        "scheduled_for",
-        "running",
-        "script_hash",
-        "script_path",
-        "null as args",
-        "null as duration_ms",
-        "null as success",
-        "false as deleted",
-        "canceled",
-        "canceled_by",
-        "job_kind",
-        "schedule_path",
-        "permissioned_as",
-        "is_flow_step",
-        "language",
-        "false as is_skipped",
-        "email",
-        "visible_to_owner",
-        "suspend",
-        "mem_peak",
-        "tag",
-        "concurrent_limit",
-        "concurrency_time_window_s",
-        "priority",
-        "null as labels",
-    ];
-    const CJ_FIELDS: &[&str] = &[
-        "'CompletedJob' as typ",
-        "id",
-        "workspace_id",
-        "parent_job",
-        "created_by",
-        "created_at",
-        "started_at",
-        "null as scheduled_for",
-        "null as running",
-        "script_hash",
-        "script_path",
-        "null as args",
-        "duration_ms",
-        "success",
-        "deleted",
-        "canceled",
-        "canceled_by",
-        "job_kind",
-        "schedule_path",
-        "permissioned_as",
-        "is_flow_step",
-        "language",
-        "is_skipped",
-        "email",
-        "visible_to_owner",
-        "null as suspend",
-        "mem_peak",
-        "tag",
-        "null as concurrent_limit",
-        "null as concurrency_time_window_s",
-        "priority",
-        "result->'wm_labels' as labels",
-    ];
-
     let row_limit = iq.row_limit.unwrap_or(1000);
     let concurrency_key = iq.concurrency_key;
 
@@ -226,12 +157,12 @@ async fn get_concurrent_intervals(
     let lqc = lq.clone();
     let lqq: ListQueueQuery = lqc.into();
     let mut sqlb_q = SqlBuilder::select_from("queue")
-        .fields(QJ_FIELDS)
+        .fields(UnifiedJob::queued_job_fields())
         .order_by("created_at", lq.order_desc.unwrap_or(true))
         .limit(row_limit)
         .clone();
     let mut sqlb_c = SqlBuilder::select_from("completed_job")
-        .fields(CJ_FIELDS)
+        .fields(UnifiedJob::completed_job_fields())
         .order_by("started_at", lq.order_desc.unwrap_or(true))
         .limit(row_limit)
         .clone();
diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs
index 152982c50e..897dea8b1b 100644
--- a/backend/windmill-api/src/jobs.rs
+++ b/backend/windmill-api/src/jobs.rs
@@ -1247,40 +1247,7 @@ async fn list_jobs(
             per_page + offset,
             0,
             &ListCompletedQuery { order_desc: Some(true), ..lqc },
-            &[
-                "'CompletedJob' as typ",
-                "id",
-                "workspace_id",
-                "parent_job",
-                "created_by",
-                "created_at",
-                "started_at",
-                "null as scheduled_for",
-                "null as running",
-                "script_hash",
-                "script_path",
-                "null as args",
-                "duration_ms",
-                "success",
-                "deleted",
-                "canceled",
-                "canceled_by",
-                "job_kind",
-                "schedule_path",
-                "permissioned_as",
-                "is_flow_step",
-                "language",
-                "is_skipped",
-                "email",
-                "visible_to_owner",
-                "null as suspend",
-                "mem_peak",
-                "tag",
-                "null as concurrent_limit",
-                "null as concurrency_time_window_s",
-                "priority",
-                "result->'wm_labels' as labels",
-            ],
+            UnifiedJob::completed_job_fields(),
         ))
     } else {
         None
@@ -1290,40 +1257,7 @@ async fn list_jobs(
         let sqlq = list_queue_jobs_query(
             &w_id,
             &ListQueueQuery { order_desc: Some(true), ..lq.into() },
-            &[
-                "'QueuedJob' as typ",
-                "id",
-                "workspace_id",
-                "parent_job",
-                "created_by",
-                "created_at",
-                "started_at",
-                "scheduled_for",
-                "running",
-                "script_hash",
-                "script_path",
-                "null as args",
-                "null as duration_ms",
-                "null as success",
-                "false as deleted",
-                "canceled",
-                "canceled_by",
-                "job_kind",
-                "schedule_path",
-                "permissioned_as",
-                "is_flow_step",
-                "language",
-                "false as is_skipped",
-                "email",
-                "visible_to_owner",
-                "suspend",
-                "mem_peak",
-                "tag",
-                "concurrent_limit",
-                "concurrency_time_window_s",
-                "priority",
-                "null as labels",
-            ],
+            UnifiedJob::queued_job_fields(),
         );
 
         if let Some(sqlc) = sqlc {
@@ -2112,6 +2046,85 @@ pub struct UnifiedJob {
     pub labels: Option<serde_json::Value>,
 }
 
+const CJ_FIELDS: &[&str] = &[
+            "'CompletedJob' as typ",
+            "id",
+            "workspace_id",
+            "parent_job",
+            "created_by",
+            "created_at",
+            "started_at",
+            "null as scheduled_for",
+            "null as running",
+            "script_hash",
+            "script_path",
+            "null as args",
+            "duration_ms",
+            "success",
+            "deleted",
+            "canceled",
+            "canceled_by",
+            "job_kind",
+            "schedule_path",
+            "permissioned_as",
+            "is_flow_step",
+            "language",
+            "is_skipped",
+            "email",
+            "visible_to_owner",
+            "null as suspend",
+            "mem_peak",
+            "tag",
+            "null as concurrent_limit",
+            "null as concurrency_time_window_s",
+            "priority",
+            "result->'wm_labels' as labels",
+        ];
+const QJ_FIELDS: &[&str] = &[
+            "'QueuedJob' as typ",
+            "id",
+            "workspace_id",
+            "parent_job",
+            "created_by",
+            "created_at",
+            "started_at",
+            "scheduled_for",
+            "running",
+            "script_hash",
+            "script_path",
+            "null as args",
+            "null as duration_ms",
+            "null as success",
+            "false as deleted",
+            "canceled",
+            "canceled_by",
+            "job_kind",
+            "schedule_path",
+            "permissioned_as",
+            "is_flow_step",
+            "language",
+            "false as is_skipped",
+            "email",
+            "visible_to_owner",
+            "suspend",
+            "mem_peak",
+            "tag",
+            "concurrent_limit",
+            "concurrency_time_window_s",
+            "priority",
+            "null as labels",
+        ];
+
+impl UnifiedJob {
+
+    pub fn completed_job_fields() -> &'static [&'static str] {
+        CJ_FIELDS
+    }
+    pub fn queued_job_fields() -> &'static [&'static str] {
+        QJ_FIELDS
+    }
+}
+
 impl<'a> From<UnifiedJob> for Job {
     fn from(uj: UnifiedJob) -> Self {
         match uj.typ.as_ref() {
diff --git a/frontend/src/lib/components/common/toggleButton-v2/ToggleButtonMore.svelte b/frontend/src/lib/components/common/toggleButton-v2/ToggleButtonMore.svelte
index 58da0e2936..669ef6d114 100644
--- a/frontend/src/lib/components/common/toggleButton-v2/ToggleButtonMore.svelte
+++ b/frontend/src/lib/components/common/toggleButton-v2/ToggleButtonMore.svelte
@@ -34,19 +34,24 @@
 	disappearTimeout={0}
 >
 	<div {id} class="flex">
-		<Tab
-			{disabled}
-			class={twMerge(
-				' rounded-md transition-all text-xs flex gap-1 flex-row items-center',
-				small ? 'px-1.5 py-0.5 text-2xs' : 'px-2 py-1',
-				light ? 'font-medium' : '',
-				isAnOptionSelected($selected)
-					? 'bg-surface shadow-md'
-					: 'bg-surface-secondary hover:bg-surface-hover',
-				$$props.class
-			)}
-		>
+		{#if isAnOptionSelected($selected)}
+			<Tab
+				{disabled}
+				class={twMerge(
+					' rounded-md transition-all text-xs flex gap-1 flex-row items-center',
+					small ? 'px-1.5 py-0.5 text-2xs' : 'px-2 py-1',
+					light ? 'font-medium' : '',
+					isAnOptionSelected($selected)
+						? 'bg-surface shadow-md'
+						: 'bg-surface-secondary hover:bg-surface-hover',
+					$$props.class
+				)}
+			>
+				{togglableItems.find((i) => i.value === $selected)?.label}
+			</Tab>
+		{/if}
+		<div class="flex items-center">
 			<DropdownV2 {items} />
-		</Tab>
+		</div>
 	</div>
 </Popover>
diff --git a/frontend/src/lib/components/runs/JobPreview.svelte b/frontend/src/lib/components/runs/JobPreview.svelte
index c11e04bd09..125d0ea3c0 100644
--- a/frontend/src/lib/components/runs/JobPreview.svelte
+++ b/frontend/src/lib/components/runs/JobPreview.svelte
@@ -4,7 +4,7 @@
 	import DisplayResult from '../DisplayResult.svelte'
 	import JobArgs from '../JobArgs.svelte'
 	import LogViewer from '../LogViewer.svelte'
-	import { Badge, Skeleton, Tab, Tabs } from '../common'
+	import { Badge, Button, Skeleton, Tab, Tabs } from '../common'
 	import HighlightCode from '../HighlightCode.svelte'
 	import { forLater } from '$lib/forLater'
 	import FlowProgressBar from '../flows/FlowProgressBar.svelte'
@@ -14,6 +14,8 @@
 	import WorkflowTimeline from '../WorkflowTimeline.svelte'
 	import Popover from '../Popover.svelte'
 	import { truncateRev } from '$lib/utils'
+	import { createEventDispatcher } from 'svelte'
+	import { ListFilter } from 'lucide-svelte'
 
 	export let id: string
 	export let blankLink = false
@@ -53,6 +55,7 @@
 	function asWorkflowStatus(x: any): Record<string, WorkflowStatus> {
 		return x as Record<string, WorkflowStatus>
 	}
+	const dispatch = createEventDispatcher()
 </script>
 
 <TestJobLoader
@@ -97,10 +100,20 @@
 			{#if concurrencyKey}
 				<Popover notClickable>
 					<svelte:fragment slot="text">
-						This jobs has concurrency limits enabled with the key
-						{concurrencyKey}
+						This jobs has concurrency limits enabled with the key:
+						<Button
+							class="inline-text"
+							size="xs2"
+							color="light"
+							on:click={() => {
+								dispatch('filterByConcurrencyKey', concurrencyKey)
+							}}
+						>
+							{concurrencyKey}
+							<ListFilter class="inline-block" size={10} />
+						</Button>
 					</svelte:fragment>
-					<Badge>Concurrency: {truncateRev(concurrencyKey, 20)}</Badge>
+					<Badge large>Concurrency: {truncateRev(concurrencyKey, 20)}</Badge>
 				</Popover>
 			{/if}
 		</div>
diff --git a/frontend/src/lib/components/runs/RunsFilter.svelte b/frontend/src/lib/components/runs/RunsFilter.svelte
index 846dd44f02..cbf88e1ec3 100644
--- a/frontend/src/lib/components/runs/RunsFilter.svelte
+++ b/frontend/src/lib/components/runs/RunsFilter.svelte
@@ -333,21 +333,21 @@
 				<ToggleButton value={undefined} label="All" />
 				<ToggleButton
 					value={'running'}
-					label="Running"
+					tooltip="Running"
 					class="whitespace-nowrap"
 					icon={PlayCircle}
 					iconProps={{ color: success === 'running' ? 'blue' : 'gray' }}
 				/>
 				<ToggleButton
 					value={'success'}
-					label="Success"
+					tooltip="Success"
 					class="whitespace-nowrap"
 					icon={CheckCircle2}
 					iconProps={{ color: success === 'success' ? 'green' : 'gray' }}
 				/>
 				<ToggleButton
 					value={'failure'}
-					label="Failure"
+					tooltip="Failure"
 					class="whitespace-nowrap"
 					icon={AlertCircle}
 					iconProps={{ color: success === 'failure' ? 'red' : 'gray' }}
diff --git a/frontend/src/lib/components/runs/RunsTable.svelte b/frontend/src/lib/components/runs/RunsTable.svelte
index 4b05778bc8..591abeebb5 100644
--- a/frontend/src/lib/components/runs/RunsTable.svelte
+++ b/frontend/src/lib/components/runs/RunsTable.svelte
@@ -161,9 +161,11 @@
 	>
 		{#if showExternalJobs && externalJobs.length > 0}
 			<div class="w-1/12 text-2xs">
-				{jobs && jobCountString(jobs.length + externalJobs.length)}<Tooltip
-					>{externalJobs.length} jobs obscured</Tooltip
-				>
+				<div class="flex flex-row">
+					{jobs && jobCountString(jobs.length + externalJobs.length)}<Tooltip
+						>{externalJobs.length} jobs obscured</Tooltip
+					>
+				</div>
 			</div>
 		{:else}
 			<div class="w-1/12 text-2xs">{jobs && jobCountString(jobs.length)}</div>
diff --git a/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte
index 216d4ea42e..5f1c42ac14 100644
--- a/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte
+++ b/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte
@@ -622,7 +622,11 @@
 						{#if selectedId === '-'}
 							<div class="p-4">There is no information available for this job</div>
 						{:else}
-							<JobPreview id={selectedId} workspace={selectedWorkspace} />
+							<JobPreview
+								on:filterByConcurrencyKey={filterByConcurrencyKey}
+								id={selectedId}
+								workspace={selectedWorkspace}
+							/>
 						{/if}
 					{:else}
 						<div class="text-xs m-4">No job selected</div>
diff --git a/frontend/src/routes/(root)/(logged)/workers/+page.svelte b/frontend/src/routes/(root)/(logged)/workers/+page.svelte
index 3ef0f4174a..b8bc77f96e 100644
--- a/frontend/src/routes/(root)/(logged)/workers/+page.svelte
+++ b/frontend/src/routes/(root)/(logged)/workers/+page.svelte
@@ -388,8 +388,6 @@
 													View job
 												</a>
 												(workspace {current_job_workspace_id})
-											{:else}
-												None
 											{/if}
 										</Cell>
 										<Cell>