Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ability to view multiple focused log lines at once #4637

Merged
merged 1 commit into from
Oct 17, 2023
Merged

add ability to view multiple focused log lines at once #4637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions torchci/components/JobLinks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ export default function JobLinks({ job }: { job: JobData }) {
<TestInsightsLink job={job} separator={" | "} />
<DisableTest job={job} label={"skipped"} />
{authenticated && <UnstableJob job={job} label={"unstable"} />}
{authenticated && job.failureLine && (
{authenticated && job.failureLines && (
<ReproductionCommand
job={job}
separator={" | "}
testName={getTestName(job.failureLine)}
testName={getTestName(job.failureLines[0])}
/>
)}
</span>
Expand Down Expand Up @@ -98,7 +98,7 @@ This test was disabled because it is failing on main branch ([recent examples]($
}

function DisableTest({ job, label }: { job: JobData; label: string }) {
const hasFailureClassification = job.failureLine != null;
const hasFailureClassification = job.failureLines != null;
const swrKey = hasFailureClassification ? `/api/issue/${label}` : null;
const { data } = useSWR(swrKey, fetcher, {
// Set a 60s cache for the request, so that lots of tooltip hovers don't
Expand All @@ -114,7 +114,7 @@ function DisableTest({ job, label }: { job: JobData; label: string }) {
return null;
}

const testName = getTestName(job.failureLine!);
const testName = job.failureLines ? getTestName(job.failureLines[0]) : null;
// - The failure classification is not a python unittest or pytest failure.
if (testName === null) {
return null;
Expand Down
48 changes: 34 additions & 14 deletions torchci/components/LogViewer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,22 @@ function Log({ url, line }: { url: string; line: number | null }) {
export default function LogViewer({
job,
logRating = LogAnnotation.NULL,
showAnnotationToggle = false,
showAnnotationToggle = process.env.DEBUG_LOG_CLASSIFIER === "true",
maxNumFailureLines = process.env.DEBUG_LOG_CLASSIFIER === "true" ? 2 : 1,
}: {
job: JobData;
logRating?: LogAnnotation;
showAnnotationToggle?: boolean;
maxNumFailureLines?: number;
}) {
const [showLogViewer, setShowLogViewer] = useState(false);
// const numFailureLines =
// Math.min(job.failureLines?.length || 0, maxNumFailureLines);

// we will replace this with the code above once we support having multiple failure lines in rockset
const numFailureLines = maxNumFailureLines;
const [showLogViewer, setShowLogViewer] = useState<boolean[]>(
Array.from({ length: numFailureLines }, () => false)
);

useEffect(() => {
document.addEventListener("copy", (e) => {
Expand All @@ -213,28 +222,39 @@ export default function LogViewer({
});
});

if (!job.failureLine && !isFailure(job.conclusion)) {
if (!job.failureLines && !isFailure(job.conclusion)) {
return null;
}

function handleClick() {
setShowLogViewer(!showLogViewer);
}
const toggleLogViewer = (index: number) => {
// Make a copy of the current array state
const updatedShowLogViewer = [...showLogViewer];

// Toggle the boolean value at the given index
updatedShowLogViewer[index] = !updatedShowLogViewer[index];

// Update the state
setShowLogViewer(updatedShowLogViewer);
};
return (
<div>
<button
style={{ background: "none", cursor: "pointer" }}
onClick={handleClick}
>
{showLogViewer ? "▼ " : "▶ "}
<code>{job.failureLine ?? "Show log"}</code>
</button>
{showLogViewer && <Log url={job.logUrl!} line={job.failureLineNumber!} />}
{showLogViewer.map((show, index) => (
<div key={index}>
<button
style={{ background: "none", cursor: "pointer" }}
onClick={() => toggleLogViewer(index)}
>
{show ? "▼ " : "▶ "}
<code>{job.failureLines ?? "Show log"}</code>
</button>
{show && <Log url={job.logUrl!} line={job.failureLineNumbers![0]} />}
</div>
))}
{showAnnotationToggle && (
<div>
<LogAnnotationToggle
job={job}
// send in real metadata later
log_metadata={{ job_id: "1" }}
annotation={logRating}
/>
Expand Down
8 changes: 4 additions & 4 deletions torchci/lib/drciUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ export async function hasSimilarFailures(
head_sha: record.sha as string,
head_branch: record.branch as string,
failure_captures: record.failureCaptures as string[],
failure_line: record.failureLine,
failure_lines: record.failureLines ? record.failureLines : undefined,
};

// Only count different jobs with the same failure
Expand All @@ -333,9 +333,9 @@ export function isInfraFlakyJob(job: RecentWorkflowsData): boolean {
// the workflow summary tab
return (
job.conclusion === "failure" &&
(job.failure_line === null ||
job.failure_line === undefined ||
job.failure_line === "") &&
(job.failure_lines === null ||
job.failure_lines === undefined ||
job.failure_lines[0] === "") &&
(job.runnerName === null ||
job.runnerName === undefined ||
job.runnerName === "")
Expand Down
5 changes: 3 additions & 2 deletions torchci/lib/searchUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,11 @@ export async function searchSimilarFailures(
time: data.completed_at,
conclusion: data.conclusion,
htmlUrl: data.html_url,
failureLine: data.torchci_classification.line,
failureLineNumber: data.torchci_classification.line_num,
failureLines: [data.torchci_classification.line],
failureLineNumbers: [data.torchci_classification.line_num],
failureCaptures: data.torchci_classification.captures,
});
});

return { jobs: jobs };
}
6 changes: 3 additions & 3 deletions torchci/lib/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export interface JobData extends BasicJobData {
logUrl?: string;
durationS?: number;
queueTimeS?: number;
failureLine?: string;
failureLineNumber?: number;
failureLines?: string[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a correct understanding that this is only a cosmetic change atm because the current log classifier will always return only one failure line? So this would be an array one just one string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep it's just cosmetic atm

failureLineNumbers?: number[];
Copy link
Contributor

@huydhn huydhn Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might want to consider updating RecentWorkflowsData as well for consistency. It also has a single failure_line string field.

For the context, this failure line field is pretty important because it now influences how Dr.CI and mergebot detects flaky failures (I search for similar failures using this field as the input)

A request I have for later is to make sure that the failures are sorted so that the last failure is at index 0 and the first failure is the last one in the list, thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be it's better to do this in a separate PR, as this one is pretty big already.

failureCaptures?: string[];
repo?: string;
failureAnnotation?: string;
Expand All @@ -41,7 +41,7 @@ export interface RecentWorkflowsData extends BasicJobData {
head_branch?: string | null;
pr_number?: number;
failure_captures: string[];
failure_line?: string | null;
failure_lines?: string[] | null;
}

export interface Artifact {
Expand Down
4 changes: 2 additions & 2 deletions torchci/pages/api/drci/drci.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,8 +520,8 @@ function isFlaky(job: RecentWorkflowsData, flakyRules: FlakyRule[]): boolean {
failureCapture.match(captureRegex)
);
const matchFailureLine: boolean =
job.failure_line != null &&
job.failure_line.match(captureRegex) != null;
job.failure_lines != null &&
job.failure_lines[0].match(captureRegex) != null;

// Accept both failure captures array and failure line string to make sure
// that nothing is missing
Expand Down
1 change: 0 additions & 1 deletion torchci/pages/metrics.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,6 @@ export default function Page() {
additionalOptions={{ yAxis: { scale: true } }}
/>
</Grid>

<JobsDuration
title={"Job time-to-signal, all branches"}
branchName={"%"}
Expand Down
4 changes: 2 additions & 2 deletions torchci/rockset/commons/__sql/annotated_flaky_jobs.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ select
PARSE_TIMESTAMP_ISO8601(job.completed_at)
) as durationS,
w.repository.full_name as repo,
job.torchci_classification.line as failureLine,
ARRAY_CREATE(job.torchci_classification.line) as failureLines,
job.torchci_classification.captures as failureCaptures,
job.torchci_classification.line_num as failureLineNumber,
ARRAY_CREATE(job.torchci_classification.line_num) as failureLineNumbers,
from
commons.job_annotation a
join commons.workflow_job job on job.id = a.jobID
Expand Down
Loading
Loading