Track SDLC metrics.#31409
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughA new webhook endpoint was added to receive GitHub Projects v2 item events, focusing on tracking status changes for specific projects. The handler verifies signatures, processes status transitions, fetches issue details, calculates time metrics, and writes results to Google BigQuery. Supporting configuration and dependencies were updated. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub
participant WebhookController as Webhook handler
participant GitHubAPI as GitHub GraphQL API
participant BigQuery
GitHub->>WebhookController: POST /api/v1/webhooks/github/projects_v2_item
WebhookController->>WebhookController: Verify signature
alt Valid and projects_v2_item event
WebhookController->>GitHubAPI: Fetch issue details
WebhookController->>BigQuery: Check for existing records
alt Status changed to "in progress"
WebhookController->>BigQuery: Save status change record
else Status changed to "awaiting qa"
WebhookController->>BigQuery: Calculate time from "in progress"
WebhookController->>BigQuery: Save QA ready metrics
else Status changed to "release"
WebhookController->>BigQuery: Calculate time from "in progress"
WebhookController->>BigQuery: Save release ready metrics
end
end
WebhookController-->>GitHub: Respond with status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
website/api/controllers/webhooks/receive-github-projects-v2-item.js (2)
12-14: Consider moving the weekend exclusion flag to configuration.The
EXCLUDE_WEEKENDSfeature flag is currently hardcoded. Moving it tosails.config.customwould make it easier to adjust without code changes.-// Feature flag for excluding weekends in time calculations -// Set to true to exclude weekends, false to include them -const EXCLUDE_WEEKENDS = false; +// Get weekend exclusion setting from config (default to false) +const EXCLUDE_WEEKENDS = sails.config.custom.excludeWeekendsInMetrics || false;
613-813: Consider refactoring to reduce code duplication.The three check functions (
checkIfIssueExists,checkIfQaReadyExists,checkIfReleaseReadyExists) share almost identical logic. A generic helper function could reduce duplication.// Add a generic helper function async function checkIfRecordExists(repo, issueNumber, gcpServiceAccountKey, tableId, additionalCondition = '') { try { const bigquery = getBigQueryClient(gcpServiceAccountKey); const datasetId = 'github_metrics'; const query = ` SELECT 1 FROM \`${gcpServiceAccountKey.project_id}.${datasetId}.${tableId}\` WHERE repo = @repo AND issue_number = @issueNumber ${additionalCondition} LIMIT 1 `; const options = { query: query, params: { repo, issueNumber } }; const [rows] = await bigquery.query(options); return rows.length > 0; } catch (err) { if (err.code === 404) { return false; } sails.log.error(`Error checking if record exists in ${tableId}:`, err); return false; } } // Then simplify the existing functions: async function checkIfIssueExists(repo, issueNumber, gcpServiceAccountKey) { return checkIfRecordExists(repo, issueNumber, gcpServiceAccountKey, 'issue_status_change', "AND status = 'in_progress'"); } async function checkIfQaReadyExists(repo, issueNumber, gcpServiceAccountKey) { return checkIfRecordExists(repo, issueNumber, gcpServiceAccountKey, 'issue_qa_ready'); } async function checkIfReleaseReadyExists(repo, issueNumber, gcpServiceAccountKey) { return checkIfRecordExists(repo, issueNumber, gcpServiceAccountKey, 'issue_release_ready'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
website/api/controllers/webhooks/receive-github-projects-v2-item.js(1 hunks)website/config/custom.js(1 hunks)website/config/routes.js(1 hunks)website/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-binaries
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (20.x)
🔇 Additional comments (8)
website/package.json (1)
8-8: BigQuery dependency version verifiedI checked the npm registry and GitHub security advisories. Version 8.1.1 is the latest release and there are no known vulnerabilities. 🎉
You can safely keep
"@google-cloud/bigquery": "^8.1.1"in yourwebsite/package.json.website/config/routes.js (1)
996-996: Route configuration looks good!The webhook route follows the established pattern and correctly disables CSRF protection, which is standard for webhook endpoints that use signature verification.
website/config/custom.js (1)
409-414: Configuration keys are well-organized.The new configuration keys for the GitHub Projects v2 webhook secret and GCP service account are properly placed in their respective sections and follow the existing naming conventions.
website/api/controllers/webhooks/receive-github-projects-v2-item.js (5)
129-160: Webhook signature verification is properly implemented.The verification uses timing-safe comparison and appropriate error handling, following security best practices.
205-281: Status change processing logic is well-structured.The function properly validates inputs, filters for relevant projects and status changes, and delegates to appropriate handlers.
284-356: GraphQL query for issue details is efficient.The function makes good use of GraphQL to fetch only the necessary fields and properly determines issue types from labels.
567-595: Error handling for BigQuery is comprehensive.The function properly handles various error scenarios and includes a smart recovery mechanism by resetting the client on connection errors.
367-565: Status transition handlers implement the business logic correctly.The handlers properly track status transitions, calculate time metrics, and prevent duplicate entries as required by the PR objectives.
| // mergeFreezeAccessToken: '…', | ||
|
|
||
| // Metrics: | ||
| // engMetricsGcpServiceAccountKey: '…', |
There was a problem hiding this comment.
env vars:
sails_custom__githubProjectsV2ItemWebhookSecret
sails_custom__engMetricsGcpServiceAccountKey
eashaw
left a comment
eashaw
left a comment
There was a problem hiding this comment.
@getvictor, I made some changes to this PR to make it match the other code in that file and website conventions to get it merged. I tested and QA'd all updated behavior with a GH webhook pointed at my local instance of the website and a throwaway BigQuery dataset.
Here's a list of changes I made:
- Added an error that is thrown if
sails.config.custom.engMetricsGcpServiceAccountKeyis not set. - Removed the
parse-gcp-service-account-key,fetch-issue-details,get-big-query-client,get-latest-in-progress-status, andcalculate-time-including-weekendshelpers and moved them inline in the webhook. We deduplicate code with helpers if it is repeated in 3+ places. (These helpers were only being used in 1-2 places) - Removed the try-catch block so we would be alerted to any unexpected errors in the added code.
- Changed messages logged with sails.log.error() to use sails.log.warn(). We don't have alerts configured for messages logged by sails.log.error(), but I am alerted in the help-p2 channel when a warning is logged.
- Updated the warnings and error messages to include more context about what caused the error/warning.
|
FYI: @getvictor, I'm merging this PR. If we need to create a new webhook for this, it should use the same URL as the webhook in the Fleet repo, and it needs to send payloads in JSON. |
Fixes #30483 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a new webhook endpoint to track GitHub Projects v2 item status changes and record engineering metrics. * Integrated with Google BigQuery for storing and analyzing issue status transition data. * **Chores** * Introduced a new POST API route for receiving GitHub Projects v2 item events. * Added configuration options for GitHub webhook secrets and Google Cloud service account keys (commented out for future use). * Added a new dependency for Google BigQuery integration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Eric <eashaw@sailsjs.com>
4 participants
Fixes #30483
Summary by CodeRabbit
New Features
Chores