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.

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

fix(logging): Add context to webhook logs #1401

Merged
merged 6 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
53 changes: 39 additions & 14 deletions modules/webhook/lambdas/webhook/src/webhook/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import { Webhooks } from '@octokit/webhooks';
import { sendActionRequest } from '../sqs';
import { CheckRunEvent, WorkflowJobEvent } from '@octokit/webhooks-types';
import { getParameterValue } from '../ssm';
import { logger as rootLogger } from './logger';
import { logger as rootLogger, LogFields } from './logger';
import { Response } from '../lambda';

const supportedEvents = ['check_run', 'workflow_job'];
const logger = rootLogger.getChildLogger();

export async function handle(headers: IncomingHttpHeaders, body: string): Promise<Response> {
Expand All @@ -24,33 +25,53 @@ export async function handle(headers: IncomingHttpHeaders, body: string): Promis
return response;
}
const payload = JSON.parse(body);
logger.info(`Received Github event ${githubEvent} from ${payload.repository.full_name}`);
LogFields.fields.event = githubEvent;
LogFields.fields.repository = payload.repository.full_name;
LogFields.fields.action = payload.action;

if (!supportedEvents.includes(githubEvent)) {
logger.warn(LogFields.fields, `Unsupported event type.`);
return {
statusCode: 202,
body: `Ignoring unsupported event ${githubEvent}`,
};
}

LogFields.fields.name = payload[githubEvent].name;
LogFields.fields.status = payload[githubEvent].status;
LogFields.fields.started_at = payload[githubEvent]?.started_at;

/*
The app subscribes to all `check_run` and `workflow_job` events.
If the event status is `completed`, log the data for workflow metrics.
*/
LogFields.fields.completed_at = payload[githubEvent]?.completed_at;
LogFields.fields.conclusion = payload[githubEvent]?.conclusion;

if (isRepoNotAllowed(payload.repository.full_name)) {
console.warn(`Received event from unauthorized repository ${payload.repository.full_name}`);
logger.error(LogFields.fields, `Received event from unauthorized repository ${payload.repository.full_name}`);
Copy link
Member

Choose a reason for hiding this comment

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

I think by a small change we can keep the log much cleaner and also readable.

So would suggest to add an function to logger.ts

  public static print(): string {
    return ' LogFields: ' + JSON.stringify(LogFields.fields);
  }

And next add the logfiels at the end of the line, for example

Suggested change
logger.error(LogFields.fields, `Received event from unauthorized repository ${payload.repository.full_name}`);
logger.error(`Received event from unauthorized repository ${payload.repository.full_name}. `, LogFields.fields);

return {
statusCode: 403,
};
}

logger.info(LogFields.fields, `Processing Github event`);

if (githubEvent == 'workflow_job') {
response = await handleWorkflowJob(payload as WorkflowJobEvent, githubEvent);
} else if (githubEvent == 'check_run') {
response = await handleCheckRun(payload as CheckRunEvent, githubEvent);
} else {
logger.warn(`Ignoring unsupported event ${githubEvent}`);
response = {
statusCode: 202,
body: `Ignoring unsupported event ${githubEvent}`,
};
}

return response;
}

async function verifySignature(githubEvent: string, signature: string, body: string): Promise<number> {
if (!signature) {
logger.error("Github event doesn't have signature. This webhook requires a secret to be configured.");
logger.error(
LogFields.fields,
"Github event doesn't have signature. This webhook requires a secret to be configured.",
);
return 500;
}

Expand All @@ -60,7 +81,7 @@ async function verifySignature(githubEvent: string, signature: string, body: str
secret: secret,
});
if (!(await webhooks.verify(body, signature))) {
logger.error('Unable to verify signature!');
logger.error(LogFields.fields, 'Unable to verify signature!');
return 401;
}
return 200;
Expand All @@ -70,7 +91,10 @@ async function handleWorkflowJob(body: WorkflowJobEvent, githubEvent: string): P
const disableCheckWorkflowJobLabelsEnv = process.env.DISABLE_CHECK_WORKFLOW_JOB_LABELS || 'false';
const disableCheckWorkflowJobLabels = JSON.parse(disableCheckWorkflowJobLabelsEnv) as boolean;
if (!disableCheckWorkflowJobLabels && !canRunJob(body)) {
logger.warn(`Received event contains runner labels '${body.workflow_job.labels}' that are not accepted.`);
logger.warn(
LogFields.fields,
`Received event contains runner labels '${body.workflow_job.labels}' that are not accepted.`,
);
return {
statusCode: 202,
body: `Received event contains runner labels '${body.workflow_job.labels}' that are not accepted.`,
Expand All @@ -90,7 +114,7 @@ async function handleWorkflowJob(body: WorkflowJobEvent, githubEvent: string): P
installationId: installationId,
});
}
console.info(`Successfully queued job for ${body.repository.full_name}`);
logger.info(LogFields.fields, `Successfully queued job for ${body.repository.full_name}`);
return { statusCode: 201 };
}

Expand All @@ -108,7 +132,7 @@ async function handleCheckRun(body: CheckRunEvent, githubEvent: string): Promise
installationId: installationId,
});
}
console.info(`Successfully queued job for ${body.repository.full_name}`);
logger.info(LogFields.fields, `Successfully queued job for ${body.repository.full_name}`);
return { statusCode: 201 };
}

Expand Down Expand Up @@ -136,6 +160,7 @@ function canRunJob(job: WorkflowJobEvent): boolean {
const runnerMatch = customWorkflowJobLabels.every((l) => runnerLabels.has(l));

logger.debug(
LogFields.fields,
`Received workflow job event with labels: '${JSON.stringify(job.workflow_job.labels)}'. The event does ${
runnerMatch ? '' : 'NOT '
}match the configured labels: '${Array.from(runnerLabels).join(',')}'`,
Expand Down
4 changes: 4 additions & 0 deletions modules/webhook/lambdas/webhook/src/webhook/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ export const logger = new Logger({
overwriteConsole: true,
type: process.env.LOG_TYPE || 'pretty',
});

export class LogFields {
static fields: { [key: string]: string } = {};
}