diff --git a/src/app/Dashboard/AddCard.tsx b/src/app/Dashboard/AddCard.tsx index 0ebe2624fc..e235e24acd 100644 --- a/src/app/Dashboard/AddCard.tsx +++ b/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; diff --git a/src/app/Dashboard/Charts/ChartCard.tsx b/src/app/Dashboard/Charts/ChartCard.tsx index a7a6d80d32..b7d2a588d1 100644 --- a/src/app/Dashboard/Charts/ChartCard.tsx +++ b/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 = (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, + }, }, ], }; diff --git a/src/app/Dashboard/Charts/ChartController.tsx b/src/app/Dashboard/Charts/ChartController.tsx index 9dfe65a145..edddeebfa9 100644 --- a/src/app/Dashboard/Charts/ChartController.tsx +++ b/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(); + private readonly _updateRequests$ = new Subject(); + private readonly _updates$ = new Subject(); private readonly _hasRecording$ = new ReplaySubject(); - private readonly _refCount$ = new BehaviorSubject(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 { - // 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 { @@ -148,15 +112,20 @@ export class ChartController { startRecording(): Observable { 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())) ); } } diff --git a/src/app/Dashboard/Dashboard.tsx b/src/app/Dashboard/Dashboard.tsx index 553eb94e8c..6f556592c0 100644 --- a/src/app/Dashboard/Dashboard.tsx +++ b/src/app/Dashboard/Dashboard.tsx @@ -82,6 +82,7 @@ export interface PropControl { kind: 'boolean' | 'number' | 'string' | 'text' | 'select'; values?: any[] | Observable; defaultValue: any; + extras?: any; } export interface DashboardProps {}