Skip to content

chore: update logdrain workflow#4247

Merged
perkinsjr merged 3 commits intomainfrom
ENG-2151-logdrain-flow
Nov 5, 2025
Merged

chore: update logdrain workflow#4247
perkinsjr merged 3 commits intomainfrom
ENG-2151-logdrain-flow

Conversation

@mcstepp
Copy link
Collaborator

@mcstepp mcstepp commented Nov 5, 2025

What does this PR do?

The job_test_logdrain.yaml workflow now:

  • Runs TypeScript type checking to catch type errors
  • Runs wrangler dry-run deployment to validate the worker builds correctly

Note: Logdrain is a Cloudflare Worker that doesn't have traditional build or test scripts in package.json. Instead, it uses:

  • TypeScript compiler for type validation
  • Wrangler's built-in bundler for building the worker

Fixes #4159

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

Note: "workflow" will be true in detect changes for all test branches because it contains the modified workflow code of this branch.

Note: The logdrain_production_deployment job uses the same detect_changes logic that we're testing in PRs. If the PR workflows show correct detection, the production deployment should work correctly after merge to main.

Scenario 1: #4244
Change: Modified logdrain worker code

PR Expected Results:

  • detect_changes outputs logdrain: true
  • test_logdrain job RUNS (TypeScript check + wrangler build)
  • Dashboard/API workflows SKIP

Scenario 2: #4245
Change: Modified logdrain README.md only

PR Expected Results:

  • detect_changes outputs logdrain: false
  • test_logdrain job SKIPS
  • All logdrain workflows SKIP (documentation-only change)
  • Dashboard/API workflows SKIP

Scenario 3: #4246
Change: Modified dashboard code

PR Expected Results:

  • detect_changes outputs logdrain: false
  • test_logdrain job SKIPS
  • Dashboard workflows RUN
  • Logdrain workflows SKIP

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Ran make fmt on /go directory
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

@linear
Copy link

linear bot commented Nov 5, 2025

@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2025

⚠️ No Changeset found

Latest commit: 16eb6a7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Nov 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
dashboard Ready Ready Preview Comment Nov 5, 2025 9:20pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
engineering Ignored Ignored Preview Nov 5, 2025 9:20pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

📝 Walkthrough

Walkthrough

This PR implements a Logdrain application CI workflow that triggers on changes to apps/logdrain/ while excluding markdown and text documentation files. It adds a new test workflow job with type checking and dry-run deployment verification, and updates path detection filters accordingly. Documentation formatting in docs.json is also refined.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow Configuration
.github/workflows/job_detect_changes.yaml
Updated logdrain path filter from apps/logdrain/** to `apps/logdrain/**/!(*.md
New Logdrain Test Workflow
.github/workflows/job_test_logdrain.yaml
New workflow file triggered via workflow_call that runs type checking (tsc --noEmit) and dry-run deployment (wrangler deploy --dry-run) for the Logdrain application on a custom Ubuntu runner
CI Pipeline Job Integration
.github/workflows/pr.yaml
Added new test_logdrain conditional job that executes the Logdrain test workflow when changes are detected in the Logdrain application
Documentation Formatting
apps/docs/docs.json
Reformatted JSON arrays in concepts/identities and errors user Too Many Requests groups to single-line format (stylistic only, no semantic changes)

Sequence Diagram

sequenceDiagram
    participant GH as GitHub (PR Event)
    participant detect as Detect Changes Job
    participant test as Test Logdrain Job
    participant checkout as Checkout
    participant node as Setup Node
    participant wrangler as Setup Wrangler
    participant tsc as Type Check
    participant deploy as Dry-run Deploy

    GH->>detect: On PR (path filter)
    detect->>detect: Check apps/logdrain/** (exclude *.md, *.txt)
    detect-->>test: outputs.logdrain = 'true'
    
    rect rgb(200, 220, 255)
    note over test,deploy: Only if logdrain files changed
    test->>checkout: actions/checkout
    checkout-->>node: Done
    test->>node: setup-node action
    node-->>wrangler: GITHUB_TOKEN
    test->>wrangler: setup-wrangler action
    wrangler-->>tsc: Ready
    test->>tsc: npx tsc --noEmit
    tsc-->>deploy: Type check pass
    test->>deploy: wrangler deploy --dry-run
    deploy-->>test: Success/Fail
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Path filter updates follow established patterns for app-specific workflows
  • New workflow file is straightforward configuration with standard action steps
  • JSON formatting change is cosmetic with no logic impact
  • All changes follow existing workflow structure and conventions

Possibly related issues

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes an unrelated change to apps/docs/docs.json (JSON array reformatting), which is out of scope for the logdrain workflow update objectives stated in issue #4159. Remove the reformatted JSON arrays from apps/docs/docs.json as they are not related to the logdrain workflow improvements and belong in a separate PR.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: update logdrain workflow' accurately summarizes the main change, which is updating the logdrain CI/CD workflow with new test steps.
Description check ✅ Passed The PR description is comprehensive with clear explanation of changes, linked issue reference #4159, type classification as Chore, detailed test scenarios, and most checklist items completed.
Linked Issues check ✅ Passed The PR implements key requirements from issue #4159: updates job_test_logdrain.yaml with TypeScript type checking and wrangler dry-run, and updates job_detect_changes.yaml with path filters excluding .md/.txt files to skip documentation-only changes.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ENG-2151-logdrain-flow

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a1f538 and 16eb6a7.

📒 Files selected for processing (4)
  • .github/workflows/job_detect_changes.yaml (1 hunks)
  • .github/workflows/job_test_logdrain.yaml (1 hunks)
  • .github/workflows/pr.yaml (1 hunks)
  • apps/docs/docs.json (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.283Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
📚 Learning: 2025-08-08T19:52:42.113Z
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3765
File: .github/workflows/job_detect_changes.yaml:43-43
Timestamp: 2025-08-08T19:52:42.113Z
Learning: In the unkeyed/unkey repository, the workflows `.github/workflows/job_test_api_local.yaml` and `.github/workflows/job_test_go_api_local.yaml` should keep their Blacksmith self-hosted runners (blacksmith-4vcpu-ubuntu-2404 and blacksmith-8vcpu-ubuntu-2404 respectively) as these are performance-critical test jobs that require dedicated resources.

Applied to files:

  • .github/workflows/pr.yaml
  • .github/workflows/job_test_logdrain.yaml
🪛 actionlint (1.7.8)
.github/workflows/job_test_logdrain.yaml

9-9: label "depot-ubuntu-24.04-4" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

⏰ 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). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/docs/docs.json (1)

116-116: Formatting changes look good.

Both changes collapse multi-line page arrays to single-line format without altering content. No functional impact on documentation structure.

Also applies to: 272-272

.github/workflows/job_detect_changes.yaml (1)

102-104: Verify path patterns work correctly with dorny/paths-filter.

The narrowed logdrain filter uses bash extended glob syntax !(*.md|*.txt) to exclude documentation files, aligning with PR objective #4159. The same syntax is used for the dashboard filter (line 77-78), suggesting it's a tested pattern. However, minimatch (used by dorny/paths-filter) may require explicit configuration to support extended glob patterns by default.

Please verify that the extended glob patterns work as intended by checking either:

  • The dorny/paths-filter action version and documentation (currently v3.0.2 per line 58) to confirm extglob is enabled
  • Or confirm via test that changes to apps/logdrain/src/worker.ts trigger the workflow while changes to apps/logdrain/README.md skip it

If minimatch does not support these patterns natively, consider using standard YAML negation syntax instead:

  logdrain:
-   - 'apps/logdrain/**/!(*.md|*.txt)'
-   - 'apps/logdrain/!(*.md|*.txt)'
+   - 'apps/logdrain/**'
+   - '!apps/logdrain/**/*.md'
+   - '!apps/logdrain/**/*.txt'
.github/workflows/pr.yaml (1)

43-47: New test_logdrain job is properly integrated.

The job structure and gating logic follow the established pattern used by test_dashboard and test_api. It correctly depends on detect_changes and gates on the logdrain output. Workflow integration looks sound.

.github/workflows/job_test_logdrain.yaml (1)

1-29: Workflow setup actions verified and configuration is correct.

The setup-node and setup-wrangler actions both exist at .github/actions/. The workflow correctly implements TypeScript type validation and Wrangler dry-run deployment verification for the Logdrain Cloudflare Worker. The depot runner configuration is valid and consistent with other workflows in the codebase.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@mcstepp
Copy link
Collaborator Author

mcstepp commented Nov 5, 2025

I fixed the scenario branches, scenarios 2 and 3 were missing the actual workflow changes 🙈

@graphite-app
Copy link

graphite-app bot commented Nov 5, 2025

Photo gif. A hand giving a thumbs-up appears in front of a photo of Harrison Ford. (Added via Giphy)

@perkinsjr perkinsjr added this pull request to the merge queue Nov 5, 2025
@graphite-app
Copy link

graphite-app bot commented Nov 5, 2025

Graphite Automations

"Post a GIF when PR approved" took an action on this PR • (11/05/25)

1 gif was posted to this PR based on Andreas Thomas's automation.

@perkinsjr perkinsjr removed this pull request from the merge queue due to a manual request Nov 5, 2025
@perkinsjr perkinsjr merged commit ecb4db7 into main Nov 5, 2025
21 of 22 checks passed
@perkinsjr perkinsjr deleted the ENG-2151-logdrain-flow branch November 5, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[APPS] [P1] Logdrain Workflow

3 participants