Skip to content

Commit

Permalink
cleanup, fix bug with no-recording target selection
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewazores committed Feb 7, 2023
1 parent 38e038b commit 279d1b5
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 111 deletions.
84 changes: 28 additions & 56 deletions src/app/Dashboard/Charts/ChartCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ import {
EmptyStateVariant,
Title,
} from '@patternfly/react-core';
import { ExternalLinkAltIcon, PlusCircleIcon, RedoIcon } from '@patternfly/react-icons';
import { ExternalLinkAltIcon, PlusCircleIcon } from '@patternfly/react-icons';
import * as React from 'react';
import { useHistory } from 'react-router-dom';
import { first, interval, timer } from 'rxjs';
import { interval } from 'rxjs';
import { DashboardCardDescriptor, DashboardCardProps, DashboardCardSizes } from '../Dashboard';
import { DashboardCard } from '../DashboardCard';
import { ChartContext } from './ChartContext';
import { MIN_REFRESH, RECORDING_NAME } from './ChartController';
import { RECORDING_NAME } from './ChartController';

export interface ChartCardProps extends DashboardCardProps {
theme: string;
Expand Down Expand Up @@ -114,10 +114,10 @@ export const ChartCard: React.FC<ChartCardProps> = (props) => {
const controllerContext = React.useContext(ChartContext);
const history = useHistory();
const addSubscription = useSubscriptions();
const [idx, setIdx] = React.useState(0);
const [hasRecording, setHasRecording] = React.useState(false);
const [chartSrc, setChartSrc] = React.useState('');
const [dashboardUrl, setDashboardUrl] = React.useState('');
const [refreshDisabled, setRefreshDisabled] = React.useState(true);

React.useEffect(() => {
addSubscription(controllerContext.controller.hasActiveRecording().subscribe(setHasRecording));
Expand All @@ -131,31 +131,17 @@ export const ChartCard: React.FC<ChartCardProps> = (props) => {
if (!dashboardUrl) {
return;
}
setRefreshDisabled(true);
const u = new URL('/d-solo/main', dashboardUrl);
u.searchParams.append('theme', props.theme);
u.searchParams.append('panelId', String(kindToId(props.chartKind)));
u.searchParams.append('to', 'now');
u.searchParams.append('from', `now-${props.duration}s`);
// TODO make this configurable
u.searchParams.append('refresh', '5s');
u.searchParams.append('refresh', `${props.period}s`);
setChartSrc(u.toString());
}, [dashboardUrl, setRefreshDisabled, props.theme, props.chartKind, props.duration, setChartSrc]);

const handleOnLoad = React.useCallback(() => {
addSubscription(
timer(MIN_REFRESH)
.pipe(first())
.subscribe((_) => setRefreshDisabled(false))
);
}, [addSubscription, setRefreshDisabled]);
}, [dashboardUrl, props.theme, props.chartKind, props.duration, props.period, setChartSrc]);

React.useEffect(() => {
addSubscription(
controllerContext.controller.refresh().subscribe((_) => {
/* do nothing */
})
);
addSubscription(controllerContext.controller.attach().subscribe(setIdx));
}, [addSubscription, controllerContext]);

const refresh = React.useCallback(() => {
Expand Down Expand Up @@ -183,6 +169,7 @@ export const ChartCard: React.FC<ChartCardProps> = (props) => {
return (
<>
<Button
key={0}
aria-label={`Pop out ${props.chartKind} chart`}
onClick={popout}
variant="plain"
Expand All @@ -193,27 +180,13 @@ export const ChartCard: React.FC<ChartCardProps> = (props) => {
);
}, [props.chartKind, popout, chartSrc, dashboardUrl]);

const refreshButton = React.useMemo(() => {
return (
<>
<Button
aria-label={`Refresh ${props.chartKind} chart`}
onClick={refresh}
variant="plain"
icon={<RedoIcon />}
isDisabled={refreshDisabled}
/>
</>
);
}, [props.chartKind, refresh, refreshDisabled]);

const actions = React.useMemo(() => {
const a = props.actions || [];
if (!hasRecording) {
return a;
}
return [popoutButton, refreshButton, ...a];
}, [props.actions, hasRecording, refreshButton, popoutButton]);
return [popoutButton, ...a];
}, [props.actions, hasRecording, popoutButton]);

const header = React.useMemo(() => {
if (hasRecording) {
Expand Down Expand Up @@ -255,35 +228,34 @@ export const ChartCard: React.FC<ChartCardProps> = (props) => {
return (
<>
<DashboardCard
key={idx}
id={props.chartKind + '-chart-card'}
dashboardId={props.dashboardId}
cardSizes={ChartCardSizes}
id={props.chartKind + '-chart-card'}
isCompact
style={cardStyle}
cardHeader={header}
>
<CardBody>
{hasRecording ? (
<iframe style={{ height: '100%', width: '100%' }} src={chartSrc} onLoad={handleOnLoad} />
<iframe style={{ height: '100%', width: '100%' }} src={chartSrc} />
) : (
<>
<Bullseye>
<EmptyState variant={EmptyStateVariant.large}>
<EmptyStateIcon icon={PlusCircleIcon} />
<Title headingLevel="h2" size="md">
Start a source recording
</Title>
<EmptyStateBody>
Metrics cards display data taken from running flight recordings with the label{' '}
<code>origin={RECORDING_NAME}</code>. No such recordings are currently available.
</EmptyStateBody>
<Bullseye>
<EmptyState variant={EmptyStateVariant.large}>
<EmptyStateIcon icon={PlusCircleIcon} />
<Title headingLevel="h2" size="md">
Start a source recording
</Title>
<EmptyStateBody>
Metrics cards display data taken from running flight recordings with the label{' '}
<code>origin={RECORDING_NAME}</code>. No such recordings are currently available.
</EmptyStateBody>

<Button variant="primary" onClick={handleCreateRecording}>
Create
</Button>
</EmptyState>
</Bullseye>
</>
<Button variant="primary" onClick={handleCreateRecording}>
Create
</Button>
</EmptyState>
</Bullseye>
)}
</CardBody>
</DashboardCard>
Expand Down
8 changes: 3 additions & 5 deletions src/app/Dashboard/Charts/ChartContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ export interface Controllers {
controller: ChartController;
}

const defaultControllers: Controllers = {
const ChartContext: React.Context<Controllers> = React.createContext({
controller: new ChartController(defaultServices.api, defaultServices.target),
};
});

const ChartContext: React.Context<Controllers> = React.createContext(defaultControllers);

export { defaultControllers, ChartContext };
export { ChartContext };
104 changes: 54 additions & 50 deletions src/app/Dashboard/Charts/ChartController.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
*/

import { ApiService } from '@app/Shared/Services/Api.service';
import { NO_TARGET, TargetService } from '@app/Shared/Services/Target.service';
import { NO_TARGET, Target, TargetService } from '@app/Shared/Services/Target.service';
import {
BehaviorSubject,
combineLatest,
catchError,
concatMap,
filter,
finalize,
Expand All @@ -49,7 +49,6 @@ import {
of,
pairwise,
ReplaySubject,
Subject,
Subscription,
throttleTime,
} from 'rxjs';
Expand All @@ -61,8 +60,7 @@ export const MIN_REFRESH = 10_000;

export class ChartController {
private readonly _refCount$ = new BehaviorSubject<number>(0);
private readonly _updateRequests$ = new Subject<void>();
private readonly _updates$ = new ReplaySubject<number>();
private readonly _updates$ = new ReplaySubject<void>();
private readonly _hasRecording$ = new ReplaySubject<boolean>();
private readonly _subscriptions: Subscription[] = [];

Expand All @@ -77,43 +75,48 @@ export class ChartController {
const curr = v[1];
if (prev && !curr) {
// last subscriber left
this._subscriptions.forEach((s) => s.unsubscribe());
this._tearDown();
}
if (!prev && curr) {
// first subscriber joined
this._init();
}
});

this._target
.target()
.pipe(concatMap((t) => this._hasRecording(t)))
.subscribe((v) => {
this._hasRecording$.next(v);
if (!v) {
this._tearDown();
}
});
}

refresh(): Observable<number> {
this._refCount$.next(this._refCount$.value + 1);
return this._updates$.asObservable().pipe(finalize(() => this._refCount$.next(this._refCount$.value - 1)));
hasActiveRecording(): Observable<boolean> {
return this._hasRecording$.asObservable();
}

requestRefresh(): void {
this._updateRequests$.next();
attach(): Observable<number> {
this._refCount$.next(this._refCount$.value + 1);
return this._refCount$.asObservable().pipe(finalize(() => this._refCount$.next(this._refCount$.value - 1)));
}

hasActiveRecording(): Observable<boolean> {
return this._hasRecording$.asObservable();
requestRefresh(): void {
this._updates$.next();
}

private _init(): void {
this._subscriptions.push(
this._target
.target()
.pipe(
concatMap((target) => {
if (target === NO_TARGET) {
return of(false);
}
private _hasRecording(target: Target): Observable<boolean> {
if (target === NO_TARGET) {
return of(false);
}

return (
this._api
/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
.graphql<any>(
`
return (
this._api
/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
.graphql<any>(
`
query ActiveRecordingsForAutomatedAnalysis($connectUrl: String) {
targetNodes(filter: { name: $connectUrl }) {
recordings {
Expand All @@ -127,33 +130,29 @@ export class ChartController {
}
}
}`,
{ connectUrl: target.connectUrl }
)
// TODO error handling
.pipe(map((resp) => resp.data.targetNodes[0].recordings.active.aggregate.count > 0))
);
})
{ connectUrl: target.connectUrl }
)
// TODO error handling
.pipe(
map((resp) => resp.data.targetNodes[0].recordings.active.aggregate.count > 0),
catchError((_) => of(false))
)
.subscribe((v) => this._hasRecording$.next(v))
);

this._subscriptions.push(
this._updateRequests$.pipe(throttleTime(MIN_REFRESH)).subscribe((_) => this._updates$.next(+Date.now()))
);

this._subscriptions.push(
this._target
.target()
.pipe(filter((v) => v !== NO_TARGET))
.subscribe((_) => this._updates$.next(+Date.now()))
);
}

private _init(): void {
this._subscriptions.push(
combineLatest([this.hasActiveRecording().pipe(filter((v) => v)), this._updates$]).subscribe((_) => {
this._api.uploadActiveRecordingToGrafana(RECORDING_NAME).subscribe((_) => {
/* do nothing */
});
})
this._updates$
.pipe(
throttleTime(MIN_REFRESH),
concatMap((_) => this._hasRecording$),
filter((v) => !!v)
)
.subscribe((_) => {
this._api.uploadActiveRecordingToGrafana(RECORDING_NAME).subscribe((_) => {
/* do nothing */
});
})
);

// TODO listen for websocket notifications about active recordings and update the hasRecording
Expand Down Expand Up @@ -204,4 +203,9 @@ export class ChartController {
// });
// })
}

private _tearDown() {
this._subscriptions.forEach((s) => s.unsubscribe());
this._subscriptions.splice(0, this._subscriptions.length);
}
}

0 comments on commit 279d1b5

Please sign in to comment.