Skip to content

Commit 00e28e9

Browse files
authored
Merge pull request #3824 from github/esbena/mono-dca
Hackathon: support mono view of remote logs
2 parents d7b59da + 44dee31 commit 00e28e9

File tree

6 files changed

+54
-20
lines changed

6 files changed

+54
-20
lines changed

extensions/ql-vscode/src/compare-performance/compare-performance-view.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export class ComparePerformanceView extends AbstractWebview<
6565
const bytes = statSync(log).size;
6666
return withProgress(
6767
async (progress) =>
68-
scanLog(log, new PerformanceOverviewScanner(), progress),
68+
await scanLog(log, new PerformanceOverviewScanner(), progress),
6969

7070
{
7171
title: `Scanning evaluator log ${logDescription} (${(bytes / 1024 / 1024).toFixed(1)} MB)`,

extensions/ql-vscode/src/compare-performance/remote-logs.ts

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ export class RemoteLogs {
251251
*/
252252
public async downloadAndProcess(): Promise<
253253
| {
254-
before: string;
254+
before: string | undefined;
255255
after: string;
256256
description: ComparePerformanceDescriptionData;
257257
}
@@ -262,17 +262,26 @@ export class RemoteLogs {
262262
void extLogger.log("No targets picked, aborting download");
263263
return undefined;
264264
}
265-
const processed = await Promise.all([
266-
this.downloadAndProcessLogsForTarget(picked.before),
265+
const [processedBefore, processedAfter] = await Promise.all([
266+
...(picked.before
267+
? [this.downloadAndProcessLogsForTarget(picked.before)]
268+
: [undefined]),
267269
this.downloadAndProcessLogsForTarget(picked.after),
268270
]);
269271

270-
if (processed.some((d) => typeof d === "undefined")) {
271-
throw new Error("Silently failed to download or process some logs!?");
272+
if (picked.before && processedBefore === undefined) {
273+
throw new Error(
274+
"Silently failed to download or process the 'before' logs!?",
275+
);
276+
}
277+
if (processedAfter === undefined) {
278+
throw new Error(
279+
"Silently failed to download or process the 'after' logs!?",
280+
);
272281
}
273282
return {
274-
before: processed[0]!,
275-
after: processed[1]!,
283+
before: processedBefore,
284+
after: processedAfter,
276285
description: picked.description,
277286
};
278287
}
@@ -643,7 +652,7 @@ export class RemoteLogs {
643652

644653
private async pickTargets(progress?: ProgressCallback): Promise<
645654
| {
646-
before: ArtifactDownload;
655+
before?: ArtifactDownload;
647656
after: ArtifactDownload;
648657
description: ComparePerformanceDescriptionData;
649658
}
@@ -703,6 +712,7 @@ export class RemoteLogs {
703712
step: 4,
704713
maxStep: this.PICK_TARGETS_PROGRESS_STEPS,
705714
});
715+
const NONE = "NONE";
706716
const targetChoice2 = await window.showQuickPick(
707717
targets
708718
.filter(
@@ -714,7 +724,8 @@ export class RemoteLogs {
714724
t.info.source_id === targetInfoChoice1.info.source_id &&
715725
t.info.variant_id !== targetInfoChoice1.info.variant_id,
716726
)
717-
.map((t) => t.info.target_id),
727+
.map((t) => t.info.target_id)
728+
.concat(NONE),
718729
{
719730
title: `Pick target 2`,
720731
ignoreFocusOut: true,
@@ -726,6 +737,20 @@ export class RemoteLogs {
726737
void extLogger.log(
727738
`Picked ${experimentChoice} ${targetChoice1} ${targetChoice2}`,
728739
);
740+
if (targetChoice2 === NONE) {
741+
return {
742+
// the convention downstream is that "from" can be optional, but here it is the opposite ...
743+
before: undefined,
744+
after: targetInfoChoice1.downloads["evaluator-logs"],
745+
description: {
746+
kind: "remote-logs",
747+
experimentName: experimentChoice,
748+
fromTarget: undefined,
749+
toTarget: targetChoice1,
750+
info,
751+
},
752+
};
753+
}
729754
const targetInfoChoice2 = targets.find(
730755
(t) => t.info.target_id === targetChoice2,
731756
)!;

extensions/ql-vscode/src/extension.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,9 +1225,14 @@ async function showPerformanceComparison(
12251225
const fromLog = from?.evalutorLogPaths?.jsonSummary;
12261226
const toLog = to.evalutorLogPaths?.jsonSummary;
12271227

1228-
if (fromLog === undefined || toLog === undefined) {
1228+
if (from && fromLog === undefined) {
12291229
return extLogger.showWarningMessage(
1230-
`Cannot compare performance as the structured logs are missing. Did they queries complete normally?`,
1230+
`Cannot compare performance as the "from" structured logs are missing. Did they queries complete normally?`,
1231+
);
1232+
}
1233+
if (toLog === undefined) {
1234+
return extLogger.showWarningMessage(
1235+
`Cannot compare performance as the "to" structured logs are missing. Did they queries complete normally?`,
12311236
);
12321237
}
12331238
if (from) {

extensions/ql-vscode/src/log-insights/performance-comparison.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export type ComparePerformanceDescriptionData =
6262
| {
6363
kind: "remote-logs";
6464
experimentName: string;
65-
fromTarget: string;
65+
fromTarget?: string;
6666
toTarget: string;
6767
info: MinimalDownloadsType;
6868
};

extensions/ql-vscode/src/view/common/ComparePerformanceLocalRunDescription.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ export const ComparePerformanceLocalRunDescription = ({
1010
}: Props) => {
1111
return (
1212
<div>
13-
(fromQuery?
14-
<strong>
15-
Comparison of local runs of {fromQuery} and {toQuery}
16-
</strong>
17-
: Local run of {toQuery})
13+
{fromQuery ? (
14+
<strong>
15+
Comparison of local runs of {fromQuery} and {toQuery}
16+
</strong>
17+
) : (
18+
<strong>Local run of {toQuery}</strong>
19+
)}
1820
</div>
1921
);
2022
};

extensions/ql-vscode/src/view/common/ComparePerformanceRemoteLogsDescription.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const TargetRow = ({
3434
info,
3535
}: {
3636
kind: string;
37-
target: Props["fromTarget"] | Props["toTarget"];
37+
target: Exclude<Props["fromTarget"] | Props["toTarget"], undefined>;
3838
info: Props["info"];
3939
}) => {
4040
const targetObj = info.targets[target];
@@ -90,7 +90,9 @@ const TargetTable = ({
9090
</tr>
9191
</thead>
9292
<tbody>
93-
<TargetRow kind="from" target={fromTarget} info={info} />
93+
{fromTarget ? (
94+
<TargetRow kind="from" target={fromTarget} info={info} />
95+
) : null}
9496
<TargetRow kind="to" target={toTarget} info={info} />
9597
</tbody>
9698
</table>

0 commit comments

Comments
 (0)