diff --git a/apps/api/src/app/deployment/services/deployment-create-from-merge.service.ts b/apps/api/src/app/deployment/services/deployment-create-from-merge.service.ts index 3c64ff60..acd972bb 100644 --- a/apps/api/src/app/deployment/services/deployment-create-from-merge.service.ts +++ b/apps/api/src/app/deployment/services/deployment-create-from-merge.service.ts @@ -3,7 +3,10 @@ import { logger } from "../../../lib/logger"; import { DeploymentChangeType } from "@prisma/client"; import { hasChangedFilesInSubdirectory } from "./deployment-monorepo.service"; import { DataIntegrityException } from "../../errors/exceptions/data-integrity.exception"; -import { linkPullRequestsToDeployment } from "./deployment-pr-linking.service"; +import { + linkPullRequestsToDeployment, + updatePullRequestDeploymentTracking, +} from "./deployment-pr-linking.service"; import { CreateDeploymentFromPullRequestMergeArgs } from "./deployment-create-from-merge.types"; import { BusinessRuleException } from "../../errors/exceptions/business-rule.exception"; @@ -70,6 +73,12 @@ export const createDeploymentFromPullRequestMerge = async ({ pullRequestIds: [pullRequest.id], }); + await updatePullRequestDeploymentTracking({ + pullRequest, + deployment, + workspaceId, + }); + logger.info("deploymentCreateFromMergeWorker: Deployment created", { deployment, }); diff --git a/apps/api/src/app/deployment/services/deployment-create-from-merge.types.ts b/apps/api/src/app/deployment/services/deployment-create-from-merge.types.ts index 82f6aa1c..1fb34850 100644 --- a/apps/api/src/app/deployment/services/deployment-create-from-merge.types.ts +++ b/apps/api/src/app/deployment/services/deployment-create-from-merge.types.ts @@ -1,8 +1,13 @@ -import { Application, Environment, PullRequest } from "@prisma/client"; +import { + Application, + Environment, + PullRequest, + PullRequestTracking, +} from "@prisma/client"; export interface CreateDeploymentFromPullRequestMergeArgs { application: Application; environment: Environment; - pullRequest: PullRequest; + pullRequest: PullRequest & { tracking: PullRequestTracking }; workspaceId: number; } diff --git a/apps/api/src/app/deployment/services/deployment-monorepo.service.ts b/apps/api/src/app/deployment/services/deployment-monorepo.service.ts index b45bf8c0..dbe0f26b 100644 --- a/apps/api/src/app/deployment/services/deployment-monorepo.service.ts +++ b/apps/api/src/app/deployment/services/deployment-monorepo.service.ts @@ -3,11 +3,12 @@ import { FilterPullRequestsBySubdirectoryArgs, HasChangedFilesInSubdirectoryArgs, } from "./deployment-monorepo.types"; +import { PullRequest } from "@prisma/client"; -export const filterPullRequestsBySubdirectory = ({ +export const filterPullRequestsBySubdirectory = ({ pullRequests, subdirectory, -}: FilterPullRequestsBySubdirectoryArgs) => { +}: FilterPullRequestsBySubdirectoryArgs) => { if (!subdirectory) { return pullRequests; } diff --git a/apps/api/src/app/deployment/services/deployment-monorepo.types.ts b/apps/api/src/app/deployment/services/deployment-monorepo.types.ts index 68bd06de..b7bc08ee 100644 --- a/apps/api/src/app/deployment/services/deployment-monorepo.types.ts +++ b/apps/api/src/app/deployment/services/deployment-monorepo.types.ts @@ -1,7 +1,9 @@ import { PullRequest } from "@prisma/client"; -export interface FilterPullRequestsBySubdirectoryArgs { - pullRequests: PullRequest[]; +export interface FilterPullRequestsBySubdirectoryArgs< + T extends PullRequest = PullRequest, +> { + pullRequests: T[]; subdirectory?: string; } diff --git a/apps/api/src/app/deployment/services/deployment-pr-linking.service.ts b/apps/api/src/app/deployment/services/deployment-pr-linking.service.ts index 60c36f87..3a1bd891 100644 --- a/apps/api/src/app/deployment/services/deployment-pr-linking.service.ts +++ b/apps/api/src/app/deployment/services/deployment-pr-linking.service.ts @@ -1,4 +1,8 @@ -import { DeploymentChangeType } from "@prisma/client"; +import { + DeploymentChangeType, + PullRequest, + PullRequestTracking, +} from "@prisma/client"; import { getInstallationOctoKit } from "../../../lib/octokit"; import { getPrisma } from "../../../prisma"; import { @@ -7,6 +11,7 @@ import { HandleDeploymentPullRequestAutoLinkingArgs, LinkPullRequestsToDeploymentArgs, UpdateDeploymentChangeTypeInput, + UpdatePullRequestDeploymentTrackingArgs, } from "./deployment-pr-linking.types"; import { logger } from "../../../lib/logger"; import { ResourceNotFoundException } from "../../errors/exceptions/resource-not-found.exception"; @@ -16,6 +21,10 @@ import { findLatestDeployment, } from "./deployment.service"; import { findWorkspaceById } from "../../workspaces/services/workspace.service"; +import { getTimeToDeploy } from "../../github/services/github-pull-request-tracking.service"; +import { DataIntegrityException } from "../../errors/exceptions/data-integrity.exception"; +import { isBefore } from "date-fns"; +import { captureException } from "../../../lib/sentry"; export const handleDeploymentPullRequestAutoLinking = async ({ workspaceId, @@ -103,7 +112,7 @@ export const handleDeploymentPullRequestAutoLinking = async ({ return; } - const pullRequests = await findPullRequestsByCommitHashes({ + const mergedPullRequests = await findMergedPullRequestsByCommitHashes({ workspaceId: workspaceId, repositoryId: deployment.application.repositoryId, commitHashes: commits, @@ -113,8 +122,10 @@ export const handleDeploymentPullRequestAutoLinking = async ({ subdirectory?: string; }; - const filteredPullRequests = filterPullRequestsBySubdirectory({ - pullRequests, + const filteredPullRequests = filterPullRequestsBySubdirectory< + (typeof mergedPullRequests)[number] + >({ + pullRequests: mergedPullRequests, subdirectory: deploymentSettings?.subdirectory, }); @@ -124,7 +135,7 @@ export const handleDeploymentPullRequestAutoLinking = async ({ { deploymentId: deploymentId, workspaceId: workspaceId, - pullRequests, + pullRequests: mergedPullRequests, subdirectory: deploymentSettings?.subdirectory, } ); @@ -132,11 +143,91 @@ export const handleDeploymentPullRequestAutoLinking = async ({ return; } + if (filteredPullRequests.some((pr) => !pr.mergedAt)) { + throw new DataIntegrityException( + "handleDeploymentPullRequestAutoLinking: Some PRs are not merged", + { + extra: { filteredPullRequests }, + } + ); + } + await linkPullRequestsToDeployment({ workspaceId: workspaceId, deploymentId: deploymentId, pullRequestIds: filteredPullRequests.map((pr) => pr.id), }); + + await Promise.all( + filteredPullRequests.map(async (pr) => { + if (!pr.tracking) { + captureException( + new DataIntegrityException( + "[updatePullRequestDeploymentTracking] Pull Request tracking not found", + { + extra: { pr }, + } + ) + ); + + return; + } + + return updatePullRequestDeploymentTracking({ + pullRequest: pr as PullRequest & { tracking: PullRequestTracking }, + deployment, + workspaceId, + }); + }) + ); +}; + +export const updatePullRequestDeploymentTracking = async ({ + pullRequest, + deployment, + workspaceId, +}: UpdatePullRequestDeploymentTrackingArgs) => { + if (!pullRequest.mergedAt) { + throw new DataIntegrityException( + "[updatePullRequestDeploymentTracking] Deployed Pull Request is not merged", + { + extra: { pullRequest }, + } + ); + } + + const previousDeployedAt = pullRequest.tracking?.firstDeployedAt; + + if ( + previousDeployedAt && + isBefore(previousDeployedAt, deployment.deployedAt) + ) { + logger.info( + "[updatePullRequestDeploymentTracking] Previous deployment is earlier than this deployment. Skipping PR tracking update.", + { + previousDeployedAt, + deploymentDeployedAt: deployment.deployedAt, + pullRequest, + workspaceId, + } + ); + + return; + } + + await getPrisma(workspaceId).pullRequestTracking.update({ + where: { + pullRequestId: pullRequest.id, + workspaceId: workspaceId, + }, + data: { + firstDeployedAt: deployment.deployedAt, + timeToDeploy: getTimeToDeploy( + pullRequest.mergedAt, + deployment.deployedAt + ), + }, + }); }; const updateDeploymentToBaseline = async ( @@ -192,7 +283,7 @@ const getChangeTypeFromGitHubComparisonStatus = ( return statusToChangeTypeMap[status]; }; -export const findPullRequestsByCommitHashes = async ({ +export const findMergedPullRequestsByCommitHashes = async ({ workspaceId, repositoryId, commitHashes, @@ -202,6 +293,7 @@ export const findPullRequestsByCommitHashes = async ({ mergeCommitSha: { in: commitHashes }, workspaceId, repositoryId, + mergedAt: { not: null }, }, include: { deploymentEvents: true, diff --git a/apps/api/src/app/deployment/services/deployment-pr-linking.types.ts b/apps/api/src/app/deployment/services/deployment-pr-linking.types.ts index c060fb30..517169c8 100644 --- a/apps/api/src/app/deployment/services/deployment-pr-linking.types.ts +++ b/apps/api/src/app/deployment/services/deployment-pr-linking.types.ts @@ -1,4 +1,9 @@ -import { DeploymentChangeType } from "@prisma/client"; +import { + Deployment, + DeploymentChangeType, + PullRequest, + PullRequestTracking, +} from "@prisma/client"; export interface HandleDeploymentPullRequestAutoLinkingArgs { workspaceId: number; @@ -30,3 +35,9 @@ export interface LinkPullRequestsToDeploymentArgs { deploymentId: number; pullRequestIds: number[]; } + +export interface UpdatePullRequestDeploymentTrackingArgs { + workspaceId: number; + pullRequest: PullRequest & { tracking: PullRequestTracking }; + deployment: Pick; +} diff --git a/apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts b/apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts index 8f5cc802..046eba2f 100644 --- a/apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts +++ b/apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts @@ -1,4 +1,3 @@ -import { PullRequest } from "@prisma/client"; import { Job } from "bullmq"; import { SweetQueue } from "../../../bull-mq/queues"; import { createWorker } from "../../../bull-mq/workers"; @@ -11,29 +10,48 @@ import { import { ResourceNotFoundException } from "../../errors/exceptions/resource-not-found.exception"; import { findRepositoryById } from "../../repositories/services/repository.service"; import { createDeploymentFromPullRequestMerge } from "../services/deployment-create-from-merge.service"; +import { findPullRequestById } from "../../pull-requests/services/pull-request.service"; +import { PullRequest, PullRequestTracking } from "@prisma/client"; interface DeploymentTriggeredByPullRequestMergeJobData { workspaceId: number; - pullRequest: PullRequest; + pullRequestId: number; installationId: string; } export const deploymentTriggeredByPullRequestMergeWorker = createWorker( SweetQueue.DEPLOYMENT_TRIGGERED_BY_PULL_REQUEST_MERGE, async (job: Job) => { - logger.info("deploymentTriggeredByPullRequestMergeWorker", { - data: job.data, - }); + logger.info("deploymentTriggeredByPullRequestMergeWorker", job.data); - const workspaceId = job.data.workspaceId; - const pullRequest = job.data.pullRequest; + const { workspaceId, pullRequestId } = job.data; if (!workspaceId) { - throw new ResourceNotFoundException("Workspace not found", { + throw new ResourceNotFoundException("Workspace ID not found", { extra: { data: job.data }, }); } + if (!pullRequestId) { + throw new ResourceNotFoundException("Pull Request ID not found", { + extra: { data: job.data }, + }); + } + + const pullRequest = await findPullRequestById({ + workspaceId, + pullRequestId, + include: { + tracking: true, + }, + }); + + if (!pullRequest?.tracking) { + throw new ResourceNotFoundException("Pull request not found", { + extra: { data: job.data, pullRequest }, + }); + } + const repository = await findRepositoryById({ workspaceId, repositoryId: pullRequest.repositoryId, @@ -66,7 +84,7 @@ export const deploymentTriggeredByPullRequestMergeWorker = createWorker( if (!pullRequest.mergedAt || !pullRequest.mergeCommitSha) { logger.info( "deploymentTriggeredByPullRequestMergeWorker: Pull request not merged", - { data: job.data } + job.data ); return; @@ -75,7 +93,7 @@ export const deploymentTriggeredByPullRequestMergeWorker = createWorker( if (!repository.applications?.length) { logger.info( "deploymentTriggeredByPullRequestMergeWorker: No applications found", - { data: job.data } + job.data ); return; @@ -90,7 +108,9 @@ export const deploymentTriggeredByPullRequestMergeWorker = createWorker( createDeploymentFromPullRequestMerge({ application, environment, - pullRequest, + pullRequest: pullRequest as PullRequest & { + tracking: PullRequestTracking; + }, workspaceId: workspaceId, }) ); diff --git a/apps/api/src/app/github/services/github-pull-request-tracking.service.ts b/apps/api/src/app/github/services/github-pull-request-tracking.service.ts index 0deecf90..eb778f5f 100644 --- a/apps/api/src/app/github/services/github-pull-request-tracking.service.ts +++ b/apps/api/src/app/github/services/github-pull-request-tracking.service.ts @@ -86,15 +86,23 @@ export const getTimeToCode = ( */ export const getTimeToMerge = ( pullRequest: PullRequest, - firstApprovalAt?: Date | null + firstApprovalAt?: Date | null, + firstReadyAt?: Date | null ) => { - const compareWith = firstApprovalAt || pullRequest.createdAt; + const compareWith = firstApprovalAt || firstReadyAt || pullRequest.createdAt; if (!pullRequest.mergedAt) return undefined; return differenceInBusinessMilliseconds(compareWith, pullRequest.mergedAt); }; +/** + * Time between the PR merge and being deployed + */ +export const getTimeToDeploy = (mergedAt: Date, deployedAt: Date) => { + return differenceInBusinessMilliseconds(mergedAt, deployedAt); +}; + /** * Time between the first commit and the PR being merged */ diff --git a/apps/api/src/app/github/services/github-pull-request.service.ts b/apps/api/src/app/github/services/github-pull-request.service.ts index 9f796600..88b62b56 100644 --- a/apps/api/src/app/github/services/github-pull-request.service.ts +++ b/apps/api/src/app/github/services/github-pull-request.service.ts @@ -27,6 +27,7 @@ import { getTimeToCode, getTimeToMerge, } from "./github-pull-request-tracking.service"; +import { captureException } from "../../../lib/sentry"; interface Author { id: string; @@ -42,13 +43,13 @@ type RepositoryData = Omit< export interface SyncPullRequestArgs { gitInstallationId: number; pullRequestId: string; - mergeCommitSha?: string; + webhookMergeCommitSha?: string; } export const syncPullRequest = async ({ gitInstallationId, pullRequestId, - mergeCommitSha, + webhookMergeCommitSha, }: SyncPullRequestArgs) => { logger.info("syncPullRequest", { installationId: gitInstallationId, @@ -88,7 +89,7 @@ export const syncPullRequest = async ({ gitProfile.id, repository, gitPrData, - mergeCommitSha + webhookMergeCommitSha ); await upsertActivityEvents(pullRequest); @@ -192,6 +193,30 @@ const fetchPullRequest = async ( }; }; +const fetchPullRequestMergeCommitSha = async ( + installationId: number, + gitPrData: any +) => { + try { + if (!gitPrData.mergedAt) return undefined; + + const octokit = await getInstallationOctoKit(installationId); + const [owner, ...repo] = gitPrData.repository.nameWithOwner.split("/"); + + const response = await octokit.rest.pulls.get({ + owner, + repo: repo.join("/"), + pull_number: parseInt(gitPrData.number), + }); + + return response.data.merge_commit_sha || undefined; + } catch (error) { + captureException(error); + + return undefined; + } +}; + const getPullRequestFiles = async ( installationId: number, pullRequestId: string, @@ -242,12 +267,21 @@ const getPullRequestFiles = async ( const upsertPullRequest = async ( workspace: Workspace, - installationId: number, + gitInstallationId: number, gitProfileId: number, repository: Repository, gitPrData: any, - mergeCommitSha?: string + webhookMergeCommitSha?: string ) => { + let mergeCommitSha = webhookMergeCommitSha; + + if (gitPrData.mergedAt && !webhookMergeCommitSha) { + mergeCommitSha = await fetchPullRequestMergeCommitSha( + gitInstallationId, + gitPrData + ); + } + const data: Prisma.PullRequestUncheckedCreateInput = { gitProvider: GitProvider.GITHUB, gitPullRequestId: gitPrData.id, @@ -286,7 +320,7 @@ const upsertPullRequest = async ( await upsertPullRequestTracking( workspace, - installationId, + gitInstallationId, repository, pullRequest, gitPrData @@ -326,7 +360,11 @@ const upsertPullRequestTracking = async ( linesChangedCount, } = getPullRequestLinesTracked(workspace, pullRequest); const size = getPullRequestSize(workspace, linesChangedCount); - const timeToMerge = getTimeToMerge(pullRequest, tracking?.firstApprovalAt); + const timeToMerge = getTimeToMerge( + pullRequest, + tracking?.firstApprovalAt, + firstReadyAt + ); // Use author date (more stable across rebases) with fallback to committer date const candidateFirstCommitAt = parseNullableISO( @@ -404,6 +442,8 @@ const getFirstCommit = async ( return response.data.at(0); } catch (error) { + captureException(error); + return undefined; } }; diff --git a/apps/api/src/app/github/workers/github-sync-pull-request.worker.ts b/apps/api/src/app/github/workers/github-sync-pull-request.worker.ts index 703d410d..4285ed3d 100644 --- a/apps/api/src/app/github/workers/github-sync-pull-request.worker.ts +++ b/apps/api/src/app/github/workers/github-sync-pull-request.worker.ts @@ -57,9 +57,10 @@ export const syncPullRequestWorker = createWorker( gitInstallationId: installationId, pullRequestId: job.data.pull_request.node_id, // GraphQL API is unreliable for getting the merge commit SHA, so we must use webhook data or refetch from HTTP API. - // Here we prefer just using webhook data since it shouldn't ever change after a PR is merged. + // To avoid over-fetching, we attempt to use the webhook data first, and if it's not available, we refetch from the HTTP API. // See https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request - mergeCommitSha: job.data.pull_request.merge_commit_sha ?? undefined, + webhookMergeCommitSha: + job.data.pull_request.merge_commit_sha ?? undefined, }), { job, jobToken: token, installationId } ); @@ -86,7 +87,7 @@ export const syncPullRequestWorker = createWorker( await addJob(SweetQueue.ALERT_MERGED_WITHOUT_APPROVAL, job.data); await addJob(SweetQueue.DEPLOYMENT_TRIGGERED_BY_PULL_REQUEST_MERGE, { workspaceId: pullRequest.workspaceId, - pullRequest, + pullRequestId: pullRequest.id, installationId, }); } diff --git a/apps/api/src/app/pull-requests/resolvers/pull-requests.schema.ts b/apps/api/src/app/pull-requests/resolvers/pull-requests.schema.ts index dbafc058..25afdbd3 100644 --- a/apps/api/src/app/pull-requests/resolvers/pull-requests.schema.ts +++ b/apps/api/src/app/pull-requests/resolvers/pull-requests.schema.ts @@ -78,9 +78,15 @@ export default /* GraphQL */ ` "The time when the pull request received its first review" firstReviewAt: DateTime + "The time when the pull request was deployed" + firstDeployedAt: DateTime + "The time when the pull request received its first approval" firstApprovalAt: DateTime + "The duration, in milliseconds, between the first commit and the time it was ready for review" + timeToCode: BigInt + "The duration, in milliseconds, between the time the first reviewer was requested and the time it received its first review" timeToFirstReview: BigInt @@ -90,6 +96,9 @@ export default /* GraphQL */ ` "The duration, in milliseconds, between the first approval of the Pull Request and the time it was merged. Compares with creation date when merged without reviews" timeToMerge: BigInt + "The duration, in milliseconds, between the pull request being merged and being deployed" + timeToDeploy: BigInt + "The duration, in milliseconds, between the first commit and the time it was merged" cycleTime: BigInt } diff --git a/apps/api/src/app/pull-requests/resolvers/queries/code-review-pull-requests.query.ts b/apps/api/src/app/pull-requests/resolvers/queries/code-review-pull-requests.query.ts index 6de2f5fb..94bfef51 100644 --- a/apps/api/src/app/pull-requests/resolvers/queries/code-review-pull-requests.query.ts +++ b/apps/api/src/app/pull-requests/resolvers/queries/code-review-pull-requests.query.ts @@ -12,10 +12,10 @@ export const codeReviewPullRequestsQuery = createFieldResolver("CodeReview", { throw new ResourceNotFoundException("Code Review Pull Request not found"); } - const pullRequest = await findPullRequestById( - codeReview["workspaceId"], - codeReview["pullRequestId"] - ); + const pullRequest = await findPullRequestById({ + workspaceId: codeReview["workspaceId"], + pullRequestId: codeReview["pullRequestId"], + }); if (!pullRequest) { throw new ResourceNotFoundException("Pull Request not found"); diff --git a/apps/api/src/app/pull-requests/resolvers/queries/pull-requests-tracking.query.ts b/apps/api/src/app/pull-requests/resolvers/queries/pull-requests-tracking.query.ts index b87e9b1b..b1231d65 100644 --- a/apps/api/src/app/pull-requests/resolvers/queries/pull-requests-tracking.query.ts +++ b/apps/api/src/app/pull-requests/resolvers/queries/pull-requests-tracking.query.ts @@ -12,6 +12,7 @@ export const teamPullRequestsQuery = createFieldResolver("PullRequest", { if ( !pullRequest.id || !pullRequest.createdAt || + !pullRequest.state || !pullRequest["workspaceId"] ) { throw new ResourceNotFoundException("Pull Request not found"); @@ -22,10 +23,13 @@ export const teamPullRequestsQuery = createFieldResolver("PullRequest", { pullRequest.id ); - return transformPullRequestTracking( + return transformPullRequestTracking({ tracking, - parseISO(pullRequest.createdAt), - pullRequest.state - ); + pullRequest: { + createdAt: parseISO(pullRequest.createdAt), + state: pullRequest.state, + mergedAt: pullRequest.mergedAt ? parseISO(pullRequest.mergedAt) : null, + }, + }); }, }); diff --git a/apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts b/apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts index 655ef4b6..bdd1e4f5 100644 --- a/apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts +++ b/apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts @@ -1,6 +1,7 @@ import { PullRequestTracking as DatabasePullRequestTracking, PullRequestState, + PullRequest as DatabasePullRequest, } from "@prisma/client"; import { PullRequestTracking as ApiPullRequestTracking, @@ -8,11 +9,18 @@ import { } from "../../../../graphql-types"; import { differenceInBusinessMilliseconds } from "../../../../lib/date"; -export const transformPullRequestTracking = ( - tracking: DatabasePullRequestTracking | null, - prCreatedAt: Date, - prState?: PullRequestState -): ApiPullRequestTracking => { +type PullRequest = Pick< + DatabasePullRequest, + "createdAt" | "state" | "mergedAt" +>; + +export const transformPullRequestTracking = ({ + tracking, + pullRequest, +}: { + tracking: DatabasePullRequestTracking | null; + pullRequest: PullRequest; +}): ApiPullRequestTracking => { if (!tracking) { return { size: PullRequestSize.MEDIUM, @@ -21,17 +29,20 @@ export const transformPullRequestTracking = ( linesDeletedCount: 0, firstApprovalAt: null, firstReviewAt: null, + firstDeployedAt: null, + timeToCode: null, timeToFirstApproval: null, timeToFirstReview: null, timeToMerge: null, + timeToDeploy: null, cycleTime: null, }; } const startedCodingAt = - tracking.firstCommitAt && tracking.firstCommitAt > prCreatedAt - ? prCreatedAt - : (tracking.firstCommitAt ?? prCreatedAt); + tracking.firstCommitAt && tracking.firstCommitAt > pullRequest.createdAt + ? pullRequest.createdAt + : (tracking.firstCommitAt ?? pullRequest.createdAt); return { size: tracking.size as PullRequestSize, @@ -40,45 +51,66 @@ export const transformPullRequestTracking = ( linesDeletedCount: tracking.linesDeletedCount, firstApprovalAt: tracking.firstApprovalAt?.toISOString(), firstReviewAt: tracking.firstReviewAt?.toISOString(), - timeToFirstApproval: calculateTimeForEvent( - tracking.firstApprovalAt || tracking.firstReadyAt || prCreatedAt, - tracking.timeToFirstApproval, - prState - ), - timeToFirstReview: calculateTimeForEvent( - tracking.firstReadyAt || prCreatedAt, - tracking.timeToFirstReview, - prState - ), - timeToMerge: calculateTimeForEvent( - tracking.firstReadyAt || prCreatedAt, - tracking.timeToMerge, - prState - ), - cycleTime: calculateTimeForEvent( - startedCodingAt, - tracking.cycleTime, - prState - ), + timeToCode: tracking.timeToCode, + firstDeployedAt: tracking.firstDeployedAt?.toISOString(), + timeToFirstApproval: calculateTimeForEvent({ + from: + tracking.firstReviewAt || + tracking.firstReadyAt || + pullRequest.createdAt, + duration: tracking.timeToFirstApproval, + pullRequest, + uselessAfterMerge: true, + }), + timeToFirstReview: calculateTimeForEvent({ + from: tracking.firstReadyAt || pullRequest.createdAt, + duration: tracking.timeToFirstReview, + pullRequest, + uselessAfterMerge: true, + }), + timeToMerge: calculateTimeForEvent({ + from: tracking.firstReadyAt || pullRequest.createdAt, + duration: tracking.timeToMerge, + pullRequest, + uselessAfterMerge: true, + }), + timeToDeploy: calculateTimeForEvent({ + from: pullRequest.mergedAt || pullRequest.createdAt, + duration: tracking.timeToDeploy, + pullRequest, + uselessAfterMerge: false, + }), + cycleTime: calculateTimeForEvent({ + from: startedCodingAt, + duration: tracking.cycleTime, + pullRequest, + uselessAfterMerge: false, + }), }; }; -const calculateTimeForEvent = ( - prDate: Date | null, - duration: bigint | null, - prState?: PullRequestState -) => { - if (duration) return duration; +const calculateTimeForEvent = ({ + from, + duration, + pullRequest, + uselessAfterMerge = false, +}: { + from: Date | null; + duration: bigint | null; + pullRequest?: PullRequest; + uselessAfterMerge?: boolean; +}) => { + if (duration !== null) return duration; + + if (!from) return null; - if (!prDate) return null; + if (pullRequest?.state === PullRequestState.CLOSED) { + return null; + } - // If PR is already merged or closed, we don't need to calculate the time it has been open - if ( - prState === PullRequestState.MERGED || - prState === PullRequestState.CLOSED - ) { + if (uselessAfterMerge && pullRequest?.state === PullRequestState.MERGED) { return null; } - return BigInt(differenceInBusinessMilliseconds(prDate, new Date())); + return BigInt(differenceInBusinessMilliseconds(from, new Date())); }; diff --git a/apps/api/src/app/pull-requests/services/pull-request.service.ts b/apps/api/src/app/pull-requests/services/pull-request.service.ts index d7698fbb..3ece0737 100644 --- a/apps/api/src/app/pull-requests/services/pull-request.service.ts +++ b/apps/api/src/app/pull-requests/services/pull-request.service.ts @@ -8,6 +8,7 @@ import { import { InputValidationException } from "../../errors/exceptions/input-validation.exception"; import { CountPullRequestsByDeploymentIdArgs, + FindPullRequestByIdArgs, FindPullRequestsByDeploymentIdArgs, PaginatePullRequestsArgs, } from "./pull-request.types"; @@ -119,15 +120,19 @@ export const paginatePullRequests = async ( return getPrisma(workspaceId).pullRequest.findMany(query); }; -export const findPullRequestById = async ( - workspaceId: number, - pullRequestId: number -) => { +export const findPullRequestById = async ({ + workspaceId, + pullRequestId, + include = {}, +}: FindPullRequestByIdArgs) => { return getPrisma(workspaceId).pullRequest.findUnique({ where: { id: pullRequestId, workspaceId, }, + include: { + ...include, + }, }); }; diff --git a/apps/api/src/app/pull-requests/services/pull-request.types.ts b/apps/api/src/app/pull-requests/services/pull-request.types.ts index 0881293f..735bc3ea 100644 --- a/apps/api/src/app/pull-requests/services/pull-request.types.ts +++ b/apps/api/src/app/pull-requests/services/pull-request.types.ts @@ -1,4 +1,4 @@ -import { PullRequestSize, PullRequestState } from "@prisma/client"; +import { Prisma, PullRequestSize, PullRequestState } from "@prisma/client"; import { DateTimeRange } from "../../types"; export interface PaginatePullRequestsArgs { @@ -27,3 +27,9 @@ export interface CountPullRequestsByDeploymentIdArgs { workspaceId: number; deploymentId: number; } + +export interface FindPullRequestByIdArgs { + workspaceId: number; + pullRequestId: number; + include?: Partial; +} diff --git a/apps/api/src/app/sync-batch/resolvers/mutations/schedule-sync-batch.mutation.ts b/apps/api/src/app/sync-batch/resolvers/mutations/schedule-sync-batch.mutation.ts index 5b172a8b..3a18356b 100644 --- a/apps/api/src/app/sync-batch/resolvers/mutations/schedule-sync-batch.mutation.ts +++ b/apps/api/src/app/sync-batch/resolvers/mutations/schedule-sync-batch.mutation.ts @@ -11,6 +11,7 @@ import { import { transformSyncBatch } from "../transformers/sync-batch.transformer"; import { validateInputOrThrow } from "../../../validator.service"; import { scheduleSyncBatchValidationSchema } from "../../services/sync-batch.validation"; +import { isLive } from "../../../../env"; export const scheduleSyncBatchMutation = createMutationResolver({ scheduleSyncBatch: async (_, { input }) => { @@ -26,6 +27,7 @@ export const scheduleSyncBatchMutation = createMutationResolver({ const lastSyncBatch = await findLastSyncBatch(workspaceId); if ( + isLive && lastSyncBatch?.scheduledAt && isAfter(lastSyncBatch.scheduledAt, addHours(new Date(), -24)) ) { diff --git a/apps/api/src/lib/logger.ts b/apps/api/src/lib/logger.ts index 6a902c5f..825a5c8a 100644 --- a/apps/api/src/lib/logger.ts +++ b/apps/api/src/lib/logger.ts @@ -1,6 +1,7 @@ import pino from "pino"; import { env } from "../env"; import { addBreadcrumb } from "@sentry/node"; +import { pick } from "radash"; const logTailStream = pino.transport({ target: "@logtail/pino", @@ -18,9 +19,26 @@ const pinoLogger = pino( stream ); +const loggableFields = { + workspace: ["id", "name", "handle", "createdAt", "updatedAt"], + pullRequest: ["id", "title", "number", "createdAt", "updatedAt"], +}; + export const logger = { info: (msg: string, obj?: object) => { - pinoLogger.info(obj || {}, msg); + const cleanObj = { ...(obj || {}) }; + + for (const key of Object.keys(cleanObj)) { + if ( + loggableFields[key] && + typeof cleanObj[key] === "object" && + cleanObj[key] !== null + ) { + cleanObj[key] = pick(cleanObj[key], loggableFields[key]); + } + } + + pinoLogger.info(cleanObj, msg); addBreadcrumb({ message: msg, category: "log", diff --git a/apps/web/src/api/deployments.api.ts b/apps/web/src/api/deployments.api.ts index 18d48099..18b6a34f 100644 --- a/apps/web/src/api/deployments.api.ts +++ b/apps/web/src/api/deployments.api.ts @@ -162,11 +162,15 @@ export const useDeploymentQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id diff --git a/apps/web/src/api/pull-request.api.ts b/apps/web/src/api/pull-request.api.ts index efd484fb..b5302a9e 100644 --- a/apps/web/src/api/pull-request.api.ts +++ b/apps/web/src/api/pull-request.api.ts @@ -50,11 +50,14 @@ export const usePullRequestsInfiniteQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt cycleTime } author { diff --git a/apps/web/src/api/teams.api.ts b/apps/web/src/api/teams.api.ts index 9d3076eb..6a4d3363 100644 --- a/apps/web/src/api/teams.api.ts +++ b/apps/web/src/api/teams.api.ts @@ -171,11 +171,15 @@ export const useTeamPullRequestsInProgressQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id @@ -206,11 +210,15 @@ export const useTeamPullRequestsInProgressQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id @@ -241,11 +249,15 @@ export const useTeamPullRequestsInProgressQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id @@ -276,11 +288,15 @@ export const useTeamPullRequestsInProgressQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id diff --git a/apps/web/src/api/work-log.api.ts b/apps/web/src/api/work-log.api.ts index fe5f5930..0488ebee 100644 --- a/apps/web/src/api/work-log.api.ts +++ b/apps/web/src/api/work-log.api.ts @@ -73,11 +73,15 @@ export const useTeamWorkLogQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id @@ -112,11 +116,15 @@ export const useTeamWorkLogQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id @@ -150,11 +158,15 @@ export const useTeamWorkLogQuery = ( changedFilesCount linesAddedCount linesDeletedCount + timeToCode timeToFirstReview - timeToMerge timeToFirstApproval + timeToMerge + timeToDeploy firstReviewAt firstApprovalAt + firstDeployedAt + cycleTime } author { id diff --git a/apps/web/src/app/metrics-and-insights/components/card-dora-metric/dora-card-stat.tsx b/apps/web/src/app/metrics-and-insights/components/card-dora-metric/dora-card-stat.tsx index ac8b8061..653aacc5 100644 --- a/apps/web/src/app/metrics-and-insights/components/card-dora-metric/dora-card-stat.tsx +++ b/apps/web/src/app/metrics-and-insights/components/card-dora-metric/dora-card-stat.tsx @@ -81,7 +81,7 @@ export const CardDoraMetric = ({ )} - + )} - {change !== 0 && ( <> {change >= 0 ? "+" : ""} {change}% - {change >= 0 ? ( + {(change >= 0 && higherIsBetter) || + (change < 0 && !higherIsBetter) ? ( ) : ( @@ -117,6 +117,9 @@ export const CardDoraMetric = ({ + + Previous Period: + {previousPeriod?.from && previousPeriod?.to ? ( <> @@ -136,6 +139,9 @@ export const CardDoraMetric = ({ "Previous period unavailable" )} + + {name}: + {previousAmount} diff --git a/apps/web/src/app/metrics-and-insights/page.tsx b/apps/web/src/app/metrics-and-insights/page.tsx index 8d9d5a0e..faa704dd 100644 --- a/apps/web/src/app/metrics-and-insights/page.tsx +++ b/apps/web/src/app/metrics-and-insights/page.tsx @@ -21,6 +21,7 @@ import { useTeamAsyncOptions, } from "../../providers/async-options.provider"; import { + getAbbreviatedDuration, humanizeDuration, parseNullableISO, thirtyDaysAgo, @@ -129,7 +130,8 @@ export const MetricsAndInsightsPage = () => { metrics.deploymentFrequency?.currentAmount?.toString() || "0" } previousAmount={ - metrics.deploymentFrequency?.previousAmount?.toString() || "0" + (metrics.deploymentFrequency?.previousAmount || 0) + + " deployments" } amountDescription={`${metrics.deploymentFrequency?.avg} per day`} change={metrics.deploymentFrequency?.change || 0} @@ -139,7 +141,7 @@ export const MetricsAndInsightsPage = () => { previousPeriod={metrics.deploymentFrequency?.previousPeriod} /> { previousPeriod={metrics.leadTime?.previousPeriod} /> { name="MTTR" amount={ metrics.meanTimeToRecover?.currentAmount - ? humanizeDuration(metrics.meanTimeToRecover?.currentAmount) + ? getAbbreviatedDuration( + metrics.meanTimeToRecover?.currentAmount, + ) : "0 hours" } previousAmount={ metrics.meanTimeToRecover?.previousAmount - ? humanizeDuration(metrics.meanTimeToRecover?.previousAmount) + ? getAbbreviatedDuration( + metrics.meanTimeToRecover?.previousAmount, + ) : "0 hours" } change={metrics.meanTimeToRecover?.change || 0} diff --git a/apps/web/src/app/settings/workspace/resync/page.tsx b/apps/web/src/app/settings/workspace/resync/page.tsx index 664e84c2..9e730b11 100644 --- a/apps/web/src/app/settings/workspace/resync/page.tsx +++ b/apps/web/src/app/settings/workspace/resync/page.tsx @@ -203,7 +203,7 @@ export const WorkspaceResyncPage = () => { variant="default" onClick={() => navigate("/settings/workspace")} > - Cancel + Close