feat(astro): Auto-wire orchestrion build-time instrumentation - #22830
Conversation
|
bugborzer run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f72aa25. Configure here.
size-limit report 📦
|
The multi-line hook-call argument pushed the `AstroIntegrationLogger` type mismatch onto the `logger` property line, past the single `@ts-expect-error` covering the call. Collapse to a single-line call (matching the sibling deprecation test) so the suppression covers the whole argument. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # packages/astro/package.json
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| if (sdkEnabled.server && !isCloudflare) { | ||
| updateConfig({ | ||
| vite: { | ||
| plugins: [sentryOrchestrionPlugin({ buildTimeInstrumentation }) as VitePlugin], | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: The orchestrion plugin is unconditionally injected in dev mode, causing its config() hook to add ssr.noExternal settings, which unnecessarily bundles packages and degrades performance.
Severity: MEDIUM
Suggested Fix
Wrap the updateConfig call for the orchestrion plugin in a condition that checks if the Astro command is not 'dev', similar to how the sourcemaps plugin is handled. This will prevent the plugin from being injected and its config() hook from running during development.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/astro/src/integration/index.ts#L178-L184
Potential issue: The Sentry orchestrion plugin is injected unconditionally when
`sdkEnabled.server` is true, without checking if the Astro command is `dev`. While the
plugin's `applyToEnvironment` hook prevents code transformations, its `config()` hook
still executes. This hook injects `ssr.noExternal` configurations into the Vite config
during development. As a result, Vite is forced to bundle server-side packages (e.g.,
`mysql`, `ioredis`) that are not needed for the dev server, leading to degraded
performance. This affects all users running the Astro dev server with the default Sentry
integration settings.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Why would they not be needed for dev?
Add `test:dev` (TEST_ENV=development, `astro dev`) alongside the existing production run and make `test:assert` run both, mirroring the nextjs e2e apps. This exercises the auto-wired orchestrion transform in Vite's dev SSR path, not just the production bundle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This reverts commit f0e65b4.
sentryAstro()now injects the orchestrion bundler plugin into the Vite config automatically duringastro:config:setup, so instrumented server-side dependencies getdiagnostics_channelpublishers injected into the SSR bundle at build time with no manual plugin setup.The
astro-7-orchestrione2e app is migrated off its manualvite.plugins: [sentryOrchestrionPlugin()]wiring to prove the built-in path.dev server investigation postponed to #22857
closes #22686