refactor, simplify controller timing

src/app/Dashboard/AddCard.tsx

@@ -301,6 +301,8 @@ const PropsConfigForm = ({ onChange, ...props }: PropsConfigFormProps) => {
               onChange={handleNumeric(ctrl.key)}
               onPlus={handleNumericStep(ctrl.key, 1)}
               onMinus={handleNumericStep(ctrl.key, -1)}
+              min={ctrl?.extras?.min}
+              max={ctrl?.extras?.max}
             />
           );
           break;

src/app/Dashboard/Charts/ChartCard.tsx

@@ -41,7 +41,7 @@ import { useSubscriptions } from '@app/utils/useSubscriptions';
 import { Button, CardActions, CardBody, CardHeader, Stack, StackItem, Text } from '@patternfly/react-core';
 import { ExternalLinkAltIcon, RedoIcon } from '@patternfly/react-icons';
 import * as React from 'react';
-import { combineLatest } from 'rxjs';
+import { combineLatest, interval } from 'rxjs';
 import { DashboardCardDescriptor, DashboardCardProps, DashboardCardSizes } from '../Dashboard';
 import { DashboardCard } from '../DashboardCard';
 import { ChartContext } from './ChartContext';
@@ -50,6 +50,7 @@ export interface ChartCardProps extends DashboardCardProps {
   theme: string;
   chartKind: string;
   duration: number;
+  period: number;
 }
 
 // TODO these need to be localized
@@ -124,15 +125,16 @@ export const ChartCard: React.FC<ChartCardProps> = (props) => {
     );
   }, [addSubscription, serviceContext.api, controllerContext, props.theme, props.chartKind, props.duration]);
 
-  React.useEffect(() => {
-    resetIFrame();
-  }, [resetIFrame]);
-
   const refresh = React.useCallback(() => {
     resetIFrame();
     controllerContext.controller.requestRefresh();
   }, [resetIFrame, controllerContext]);
 
+  React.useEffect(() => {
+    refresh();
+    addSubscription(interval(props.period * 1000).subscribe(() => refresh()));
+  }, [addSubscription, props.period, refresh]);
+
   const popout = React.useCallback(() => {
     window.open(chartSrc, '_blank');
   }, [chartSrc]);
@@ -247,11 +249,26 @@ export const ChartCardDescriptor: DashboardCardDescriptor = {
       kind: 'select',
     },
     {
-      name: 'Duration',
+      name: 'Data Window',
       key: 'duration',
-      defaultValue: 60,
+      defaultValue: 120,
       description: 'The data window width in seconds.',
       kind: 'number',
+      extras: {
+        min: 30,
+        max: 300,
+      },
+    },
+    {
+      name: 'Refresh Period',
+      key: 'period',
+      defaultValue: 60,
+      description: 'The chart refresh period in seconds.',
+      kind: 'number',
+      extras: {
+        min: 30,
+        max: 120,
+      },
     },
   ],
 };

src/app/Dashboard/Charts/ChartController.tsx

@@ -38,33 +38,16 @@
 
 import { ApiService, RecordingAttributes } from '@app/Shared/Services/Api.service';
 import { NO_TARGET, TargetService } from '@app/Shared/Services/Target.service';
-import {
-  BehaviorSubject,
-  combineLatest,
-  concatMap,
-  filter,
-  finalize,
-  map,
-  Observable,
-  of,
-  ReplaySubject,
-  tap,
-  timer,
-} from 'rxjs';
+import { combineLatest, concatMap, filter, map, Observable, of, ReplaySubject, Subject, tap, throttleTime } from 'rxjs';
 
-const RECORDING_NAME = 'dashboard_metrics';
+const RECORDING_NAME = 'dashboard-metrics';
 
 export class ChartController {
-  private readonly _timer$ = new ReplaySubject<number>();
+  private readonly _updateRequests$ = new Subject<void>();
+  private readonly _updates$ = new Subject<number>();
   private readonly _hasRecording$ = new ReplaySubject<boolean>();
-  private readonly _refCount$ = new BehaviorSubject<number>(0);
 
   constructor(private readonly _api: ApiService, private readonly _target: TargetService) {
-    // TODO extract this interval to a configurable setting
-    timer(0, 60_000)
-      .pipe(map((_) => +Date.now()))
-      .subscribe(this._timer$);
-
     this._target
       .target()
       .pipe(
@@ -100,45 +83,26 @@ export class ChartController {
       )
       .subscribe((v) => this._hasRecording$.next(v));
 
+    this._updateRequests$.pipe(throttleTime(10_000)).subscribe((_) => this._updates$.next(+Date.now()));
+
     this._target
       .target()
       .pipe(filter((v) => v !== NO_TARGET))
-      .subscribe((_) => this._timer$.next(+Date.now()));
+      .subscribe((_) => this._updates$.next(+Date.now()));
 
-    combineLatest([this.hasActiveRecording(), this._refCount$, this._timer$]).subscribe((parts) => {
-      const hasRecording = parts[0];
-      const subscribers = parts[1];
-      if (!hasRecording || subscribers === 0) {
-        return;
-      }
+    combineLatest([this.hasActiveRecording().pipe(filter((v) => v)), this._updates$]).subscribe((_) => {
       this._api.uploadActiveRecordingToGrafana(RECORDING_NAME).subscribe((_) => {
         /* do nothing */
       });
     });
   }
 
-  // TODO maybe invert this control. The controller refcounts how many subscribers and ignores
-  // the upload action if there are no current subscribers. Maybe instead the subscribers should
-  // independently request refreshes to be performed, and the controller can debounce/throttle
-  // these requests and determine when to actually do them.
   refresh(): Observable<number> {
-    // return a BehaviorSubject that immediately emits the current timestamp,
-    // and is subsequently updated along with all others according to the
-    // global timer instance. This ensures charts refresh immediately once
-    // loaded, and then all refresh in sync together later.
-    const s = new BehaviorSubject(+Date.now());
-    const subscription = this._timer$.subscribe((_) => s.next(+Date.now()));
-    this._refCount$.next(this._refCount$.value + 1);
-    return s.asObservable().pipe(
-      finalize(() => {
-        subscription.unsubscribe();
-        this._refCount$.next(this._refCount$.value - 1);
-      })
-    );
+    return this._updates$.asObservable();
   }
 
   requestRefresh(): void {
-    this._timer$.next(+Date.now());
+    this._updateRequests$.next();
   }
 
   hasActiveRecording(): Observable<boolean> {
@@ -148,15 +112,20 @@ export class ChartController {
   startRecording(): Observable<boolean> {
     const attrs: RecordingAttributes = {
       name: RECORDING_NAME,
-      events: 'template=Profiling,type=TARGET', // TODO make this configurable like for the automated analysis card
+      // TODO make this configurable like for the automated analysis card
+      events: 'template=Profiling,type=TARGET',
       options: {
         toDisk: true,
-        maxAge: 60, // TODO get this from settings
+        // TODO get this from settings? this should probably somehow be the maximum data window width of
+        // all configured dashboard cards. But how to handle when a new card is added with a new maximum?
+        // Restart recording?
+        maxAge: 60,
       },
     };
     return this._api.createRecording(attrs).pipe(
       map((resp) => resp?.ok || false),
-      tap((success) => this._hasRecording$.next(success))
+      tap((success) => this._hasRecording$.next(success)),
+      tap((_) => this._updates$.next(+Date.now()))
     );
   }
 }

src/app/Dashboard/Dashboard.tsx

@@ -82,6 +82,7 @@ export interface PropControl {
   kind: 'boolean' | 'number' | 'string' | 'text' | 'select';
   values?: any[] | Observable<any>;
   defaultValue: any;
+  extras?: any;
 }
 
 export interface DashboardProps {}