Fixed yarn dev:analytics #25551
Fixed yarn dev:analytics #25551
Conversation
|
Warning Rate limit exceeded@jonatansberg has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR integrates Tinybird analytics into the development environment. It updates compose.dev.analytics.yaml to mount a read-only shared-config volume into ghost-dev, adds Tinybird-related environment variables, and makes ghost-dev depend on tb-cli completing successfully. The ghost-dev image now copies an entrypoint script and sets it as ENTRYPOINT; the script reads /mnt/shared-config/.env.tinybird (if present), exports workspace and admin token values as tinybird__workspaceId and tinybird__adminToken, logs warnings on missing data, and then execs the container CMD. The dev gateway receives a new analytics proxy route and ANALYTICS_PROXY_TARGET env var. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
compose.dev.analytics.yaml(1 hunks)docker/ghost-dev/Dockerfile(1 hunks)docker/ghost-dev/entrypoint.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Analytics using Tinybird should reference scripts in `ghost/core/core/server/data/tinybird/scripts/` and datafiles in `ghost/core/core/server/data/tinybird/`
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24862
File: .github/workflows/ci-docker.yml:320-324
Timestamp: 2025-09-10T21:24:49.363Z
Learning: In GitHub Actions workflows using docker compose, the `docker compose pull` command works correctly with fork PRs even when some images (like the Ghost development image) are only loaded locally and not available in remote registries. The command doesn't fail when it encounters locally-loaded images during the pull process.
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Analytics using Tinybird should reference scripts in `ghost/core/core/server/data/tinybird/scripts/` and datafiles in `ghost/core/core/server/data/tinybird/`
Applied to files:
docker/ghost-dev/entrypoint.shcompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:dev` to start Ghost in Docker with hot reload
Applied to files:
docker/ghost-dev/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Applied to files:
docker/ghost-dev/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Applied to files:
docker/ghost-dev/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn lint` to lint all packages or `cd ghost/core && yarn lint` to lint Ghost core (server, shared, frontend, tests)
Applied to files:
docker/ghost-dev/Dockerfile
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:build` to build Docker images and delete ephemeral volumes
Applied to files:
docker/ghost-dev/Dockerfile
📚 Learning: 2025-11-25T13:09:33.918Z
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
Applied to files:
docker/ghost-dev/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Applied to files:
docker/ghost-dev/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
docker/ghost-dev/Dockerfile
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
compose.dev.analytics.yaml
📚 Learning: 2025-10-07T12:19:15.174Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25031
File: ghost/core/test/utils/fixtures/config/defaults.json:68-80
Timestamp: 2025-10-07T12:19:15.174Z
Learning: In Ghost's configuration system (ghost/core/core/shared/config/), environment-specific config files (e.g., config.development.json, config.production.json) automatically fall back to values defined in defaults.json. It's only necessary to specify changed overrides on a per-env basis. Missing keys in env configs are not an issue—they're intentional and will use the default values.
Applied to files:
compose.dev.analytics.yaml
⏰ 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). (2)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push
🔇 Additional comments (4)
docker/ghost-dev/Dockerfile (1)
30-39: LGTM: Entrypoint script integration is correctly structured.The COPY destination (line 31) copies to
/home/ghost/entrypoint.shrelative to the WORKDIR at that point, and the absolute ENTRYPOINT path (line 38) correctly references this location. The chmod +x (line 32) ensures executability, and the CMD is properly preserved to maintain the node watch mode behavior.docker/ghost-dev/entrypoint.sh (1)
1-22: LGTM: Entrypoint script has solid error handling and graceful fallback behavior.The script correctly uses conditional checks before sourcing, employs parameter defaults to avoid
set -ufailures, and outputs appropriate warnings to stderr when Tinybird configuration is unavailable. Theexec "$@"at line 21 correctly transfers control to the CMD while replacing the shell process.compose.dev.analytics.yaml (2)
61-81: Volume and dependency setup is sound; verify tb-cli produces expected output file.The shared-config volume mount is correctly configured as read-only (line 63), and the
service_completed_successfullycondition on tb-cli (line 81) ensures tb-cli runs to completion before ghost-dev starts. However, the integration relies on tb-cli creating/mnt/shared-config/.env.tinybirdwith theTINYBIRD_WORKSPACE_IDandTINYBIRD_ADMIN_TOKENvariables—this behavior should be verified to ensure the entrypoint.sh script can successfully load the configuration.
64-76: Verify Ghost config system correctly parses the Tinybird environment variable names.The environment variables use mixed naming conventions (e.g.,
TB_HOSTuppercase vs.tinybird__stats__endpointlowercase with double underscores). Confirm that Ghost's configuration system properly parses both conventions and maps these to the expected internal config structure, especially thetinybird__*variables with double underscores.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jonatansberg
force-pushed
the
ber-3077-fixed-yarn-dev-forward-analytics
branch
2 times, most recently
from
a9c89ed to
ab0b15d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker/dev-gateway/Caddyfile (1)
36-45: Consider adding client IP headers for analytics tracking.The analytics proxy forwards
HostandX-Forwarded-Hostbut omitsX-Real-IPandX-Forwarded-For, which the/ghost/api/*handler includes (lines 28-29). If the analytics service logs or processes client IP addresses for traffic analysis, these headers should be forwarded.Apply this diff if client IP tracking is needed:
handle @analytics_paths { rewrite * {re.analytics_match.2} reverse_proxy {env.ANALYTICS_PROXY_TARGET} { header_up Host {host} header_up X-Forwarded-Host {host} + header_up X-Real-IP {remote_host} + header_up X-Forwarded-For {remote_host} } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
compose.dev.analytics.yaml(1 hunks)docker/dev-gateway/Caddyfile(1 hunks)docker/dev-gateway/Dockerfile(1 hunks)docker/ghost-dev/Dockerfile(1 hunks)docker/ghost-dev/entrypoint.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docker/ghost-dev/Dockerfile
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Analytics using Tinybird should reference scripts in `ghost/core/core/server/data/tinybird/scripts/` and datafiles in `ghost/core/core/server/data/tinybird/`
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:dev` to start Ghost in Docker with hot reload
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24557
File: apps/admin-x-settings/src/components/settings/general/TimeZone.tsx:7-7
Timestamp: 2025-08-01T12:44:07.467Z
Learning: In Ghost development, PRs may depend on unpublished changes from SDK packages. When this occurs, TypeScript compilation errors for missing exports are expected and documented in the PR description until the dependency packages are published and updated. This is normal workflow for cross-repository feature development.
📚 Learning: 2025-11-25T13:09:33.918Z
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
Applied to files:
docker/dev-gateway/Dockerfiledocker/dev-gateway/Caddyfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Applied to files:
docker/dev-gateway/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Applied to files:
docker/dev-gateway/Dockerfilecompose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs
Applied to files:
docker/dev-gateway/Caddyfile
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Analytics using Tinybird should reference scripts in `ghost/core/core/server/data/tinybird/scripts/` and datafiles in `ghost/core/core/server/data/tinybird/`
Applied to files:
docker/ghost-dev/entrypoint.shcompose.dev.analytics.yaml
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Applied to files:
compose.dev.analytics.yaml
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
compose.dev.analytics.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:dev` to start Ghost in Docker with hot reload
Applied to files:
compose.dev.analytics.yaml
📚 Learning: 2025-10-07T12:19:15.174Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25031
File: ghost/core/test/utils/fixtures/config/defaults.json:68-80
Timestamp: 2025-10-07T12:19:15.174Z
Learning: In Ghost's configuration system (ghost/core/core/shared/config/), environment-specific config files (e.g., config.development.json, config.production.json) automatically fall back to values defined in defaults.json. It's only necessary to specify changed overrides on a per-env basis. Missing keys in env configs are not an issue—they're intentional and will use the default values.
Applied to files:
compose.dev.analytics.yaml
⏰ 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). (2)
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push
🔇 Additional comments (6)
docker/dev-gateway/Dockerfile (1)
14-15: LGTM!The
ANALYTICS_PROXY_TARGETenvironment variable is correctly configured to point to the analytics service.docker/ghost-dev/entrypoint.sh (2)
1-18: LGTM! Tinybird configuration loading is well-structured.The script correctly:
- Uses strict error handling with
set -euo pipefail- Conditionally loads Tinybird configuration from the shared volume
- Validates required variables before exporting
- Provides clear warning messages when configuration is unavailable
The use of
sourceis acceptable in this dev environment context since the file is created by the trustedtb-cliservice and mounted read-only.
20-21: LGTM!Correct use of
exec "$@"to replace the shell process with the container's CMD, following entrypoint best practices.compose.dev.analytics.yaml (3)
61-63: LGTM!The read-only volume mount correctly provides ghost-dev access to the Tinybird configuration created by tb-cli.
68-76: LGTM! Tinybird environment variables are correctly configured.The configuration properly separates:
- Internal endpoints (tinybird-local:7181) for server-side calls
- External endpoints (localhost:7181, localhost:2368/.ghost/analytics/*) for browser-side calls
- The tracker endpoint will correctly route through the dev gateway's analytics proxy to the analytics service
80-81: LGTM!The
tb-clidependency withservice_completed_successfullycondition ensures that Tinybird configuration is created before ghost-dev starts, which is necessary for the entrypoint script to load credentials.
jonatansberg
force-pushed
the
ber-3077-fixed-yarn-dev-forward-analytics
branch
from
ab0b15d to
1994a94
Compare
refs https://linear.app/ghost/issue/BER-3077/improve-e2e-testing-integration The yarn dev:forward analytics integration was incomplete. These changes ensure we're properly configuring the necessary environment variables.
jonatansberg
force-pushed
the
ber-3077-fixed-yarn-dev-forward-analytics
branch
from
1994a94 to
7939449
Compare
9larsons
left a comment
9larsons
left a comment
There was a problem hiding this comment.
Works great, thanks for patching this up! Been using a different configuration and I realized in testing this that the Ghost config seems to be given precedence. I didn't track that down as the goal here is 'no setup needed'. I'll probably clean up the other methods now and use this by default so it's easy for us all to be on the same page.
I'll also adapt my generator script to use the events API so it's easier to use with this, as well as not require a db reset.
cmraible
left a comment
cmraible
left a comment
There was a problem hiding this comment.
Took this for a spin and it worked nicely, thanks!
| # Analytics configuration | ||
| analytics__url: http://analytics:3000 | ||
| analytics__enabled: "true" |
There was a problem hiding this comment.
nit: these config values are unnecessary and can be removed
refs https://linear.app/ghost/issue/BER-3077/improve-e2e-testing-integration
While working on improving the E2E testing integration with the new dev env I found that the Tinybird and analytics integration was incomplete.
These changes ensure we're properly configuring the necessary environment variables when running
yarn dev:analytics.