Skip to content

Conversation

@miklosbarabas
Copy link
Contributor

@miklosbarabas miklosbarabas commented Sep 9, 2025

Summary by CodeRabbit

  • New Features

    • Optional Sentry integration for error reporting, performance tracing, profiling, and event-loop block detection; initialized before the server starts and requires a DSN when enabled.
  • Documentation

    • Example environment file and runtime environment schema updated with Sentry keys and defaults (enabled, DSN, send PII, traces/profile sampling rates, event-loop threshold).
  • Dependencies

    • Added Sentry runtime packages.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@coderabbitai
Copy link

coderabbitai bot commented Sep 9, 2025

Walkthrough

Adds Sentry integration to the controlplane: adds Sentry dependencies, adds a Sentry config module, extends env schema and .env.example with Sentry variables, and initializes Sentry conditionally at startup via top‑level await before app creation.

Changes

Cohort / File(s) Summary of changes
Dependencies
controlplane/package.json
Added dependencies @sentry/node, @sentry/node-native, and @sentry/profiling-node at ^10.11.0.
Sentry config module
controlplane/src/core/sentry.config.ts
New file exporting SentryConfig interface and init(opts: SentryConfig). Initializes Sentry when enabled using dsn, integrates eventLoopBlockIntegration({ threshold: ... }) and nodeProfilingIntegration(), and configures profileSessionSampleRate, sendDefaultPii, and tracesSampleRate.
Entrypoint initialization
controlplane/src/index.ts
Adds top-level await Sentry initialization: parses SENTRY_* env vars, requires SENTRY_DSN when SENTRY_ENABLED is true, dynamically imports core/sentry.config.js, and calls init before creating the app.
Env schema
controlplane/src/core/env.schema.ts
Added SENTRY_ENABLED, SENTRY_DSN, SENTRY_SEND_DEFAULT_PII, SENTRY_TRACES_SAMPLE_RATE, SENTRY_PROFILE_SESSION_SAMPLE_RATE, and SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS with parsing/coercion and defaults (boolean/string/number coercions as appropriate).
Environment example
controlplane/.env.example
Appended six Sentry env vars with defaults: SENTRY_ENABLED, SENTRY_DSN, SENTRY_SEND_DEFAULT_PII, SENTRY_TRACES_SAMPLE_RATE, SENTRY_PROFILE_SESSION_SAMPLE_RATE, SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: setup sentry for the controlplane" is concise and accurately describes the primary change—adding Sentry integration to the controlplane (dependencies, config, env vars, and initialization)—and uses a conventional commit prefix so teammates can quickly understand the intent.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 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 c02e57e and 325aad5.

📒 Files selected for processing (1)
  • controlplane/src/core/env.schema.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • controlplane/src/core/env.schema.ts
⏰ 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). (16)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: image_scan
  • GitHub Check: build_push_image
  • GitHub Check: integration_test (./events)
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: build_test
  • GitHub Check: build_test
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: build_test
✨ Finishing touches
  • 📝 Generate Docstrings

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

github-actions bot commented Sep 9, 2025

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-783ac9ed5d41d9c8710e093b92c2786cf762b44f

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (4)
controlplane/src/core/sentry.config.ts (2)

7-18: Simplify gating and align with index.ts.

Index already guards on SENTRY_ENABLED === "true". This extra check is redundant and looser. Consider removing or aligning equality to reduce surprises.

Option A (remove outer if):

-if (process.env.SENTRY_ENABLED) {
     Sentry.init({
       // ...
     });
-}

Option B (align equality):

-if (process.env.SENTRY_ENABLED) {
+if (process.env.SENTRY_ENABLED === 'true') {

8-17: Consider setting release and environment tags.

Helps grouping and deploy tracking.

     Sentry.init({
         dsn: process.env.SENTRY_DSN,
+        release: process.env.SENTRY_RELEASE,
+        environment: process.env.SENTRY_ENVIRONMENT ?? process.env.NODE_ENV,
         integrations: [
controlplane/src/index.ts (2)

173-180: Initialization order may limit auto-instrumentation; prefer Node’s --import.

Sentry recommends initializing before any other module loads. With ESM, use node --import ./instrument.mjs (or NODE_OPTIONS="--import ./instrument.mjs"). Your current dynamic import happens after other imports, so some auto-instrumentation may be missed.

Reference. (npmjs.com)

Minimal path if you keep this approach:

  • Keep dynamic import, but document that only profiling/event-loop integrations are guaranteed.
  • Or add an instrument.ts that calls Sentry.init(...) and wire it via NODE_OPTIONS in start scripts.

173-180: Good guardrail on DSN; consider schema-based validation.

Throwing when SENTRY_ENABLED is true but DSN missing is fine. Optional: move Sentry vars into env.schema.ts for centralized validation and defaults.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 735372d and 375a33c.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • Makefile (1 hunks)
  • controlplane/package.json (2 hunks)
  • controlplane/src/core/sentry.config.ts (1 hunks)
  • controlplane/src/index.ts (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). (15)
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: image_scan
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: integration_test (./events)
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: build_test
  • GitHub Check: build_test
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: build_push_image
🔇 Additional comments (1)
controlplane/package.json (1)

54-56: Validate Sentry package compatibility and runtime requirements.

The additions look right. Please confirm:

  • @sentry/node-native is needed for eventLoopBlockIntegration (it is), and threshold defaults are sane. (docs.sentry.io)
  • nodeProfilingIntegration() requires @sentry/node plus @sentry/profiling-node; ensure Node runtime compatibility in environments where this runs. (docs.sentry.dev, docs.sentry.io)

@miklosbarabas miklosbarabas force-pushed the miklos/eng-8036-setup-sentry-for-the-controlplane branch from 375a33c to 20b6dff Compare September 10, 2025 14:53
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
controlplane/.env.example (1)

78-84: Reorder and tune Sentry vars; set a saner default threshold.

  • Order keys alphabetically to satisfy dotenv-linter.
  • Default event-loop block threshold to 500ms to avoid noise. You can keep 100ms locally if you want more sensitivity. (docs.sentry.io)

Apply this diff:

-# Sentry integration
-SENTRY_ENABLED="false"
-SENTRY_DSN=""
-SENTRY_SEND_DEFAULT_PII="false"
-SENTRY_TRACES_SAMPLE_RATE="1.0"
-SENTRY_PROFILE_SESSION_SAMPLE_RATE="1.0"
-SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS="100"
+# Sentry integration
+SENTRY_DSN=""
+SENTRY_ENABLED="false"
+SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS="500"
+SENTRY_PROFILE_SESSION_SAMPLE_RATE="1.0"
+SENTRY_SEND_DEFAULT_PII="false"
+SENTRY_TRACES_SAMPLE_RATE="1.0"

Note: dotenv-linter also flags quotes; if you want a clean run, drop quotes for booleans/numbers across the file for consistency.

controlplane/src/core/sentry.config.ts (2)

20-20: Minor: simplify re-export.

You already import Sentry above; you can re-export the same symbol to avoid a second module resolution.

-export * as Sentry from "@sentry/node";
+export { Sentry };

10-16: Optional: consider adding profilesSampleRate if you want per-transaction profiling control.

Current setup uses profileSessionSampleRate (session-level gate) which is valid. If you later need per-transaction sampling, expose profilesSampleRate via env as well. (docs.sentry.io)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 375a33c and 20b6dff.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • controlplane/.env.example (1 hunks)
  • controlplane/package.json (2 hunks)
  • controlplane/src/core/sentry.config.ts (1 hunks)
  • controlplane/src/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • controlplane/src/index.ts
  • controlplane/package.json
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
controlplane/.env.example

[warning] 79-79: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [UnorderedKey] The SENTRY_DSN key should go before the SENTRY_ENABLED key

(UnorderedKey)


[warning] 81-81: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 82-82: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [UnorderedKey] The SENTRY_PROFILE_SESSION_SAMPLE_RATE key should go before the SENTRY_SEND_DEFAULT_PII key

(UnorderedKey)


[warning] 84-84: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 84-84: [UnorderedKey] The SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS key should go before the SENTRY_PROFILE_SESSION_SAMPLE_RATE key

(UnorderedKey)

⏰ 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). (15)
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: build_push_image
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: build_push_image
  • GitHub Check: integration_test (./events)
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: image_scan
  • GitHub Check: build_test
  • GitHub Check: Analyze (go)
  • GitHub Check: build_test
  • GitHub Check: build_test

@miklosbarabas miklosbarabas force-pushed the miklos/eng-8036-setup-sentry-for-the-controlplane branch from 20b6dff to 99794d7 Compare September 10, 2025 16:07
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
controlplane/src/core/sentry.config.ts (2)

10-13: Profiling integration invocation looks correct now.

Good catch switching to nodeProfilingIntegration() (invoked) rather than passing the function reference.


7-18: Fix boolean gating, DSN guard, safe parsing/clamping, and set environment.

Current truthy checks and Number(...) parsing can enable Sentry accidentally and pass NaN/unsafe values. Apply guarded parsing, clamp sample rates to [0,1], default event-loop threshold to 500ms, and tag environment.

-if (process.env.SENTRY_ENABLED) {
-    Sentry.init({
-        dsn: process.env.SENTRY_DSN,
-        integrations: [
-            eventLoopBlockIntegration({ threshold: Number(process.env.SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS) }),
-            nodeProfilingIntegration()
-        ],
-        profileSessionSampleRate: Number(process.env.SENTRY_PROFILE_SESSION_SAMPLE_RATE),
-        sendDefaultPii: Boolean(process.env.SENTRY_SEND_DEFAULT_PII),
-        tracesSampleRate: Number(process.env.SENTRY_TRACES_SAMPLE_RATE)
-    });
-}
+const sentryEnabled = /^(1|true|yes|on)$/i.test(process.env.SENTRY_ENABLED ?? "");
+if (sentryEnabled) {
+    const dsn = process.env.SENTRY_DSN;
+    if (!dsn) {
+        // eslint-disable-next-line no-console
+        console.warn("[sentry] SENTRY_ENABLED is true but SENTRY_DSN is empty; skipping Sentry.init()");
+    } else {
+        const clamp01 = (n: number) => Math.max(0, Math.min(1, n));
+        const parseMs = (v: string | undefined, fallback: number) => {
+            const n = Number(v);
+            return Number.isFinite(n) && n >= 0 ? n : fallback;
+        };
+        Sentry.init({
+            dsn,
+            integrations: [
+                eventLoopBlockIntegration({
+                    threshold: parseMs(process.env.SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS, 500),
+                }),
+                nodeProfilingIntegration(),
+            ],
+            environment: process.env.SENTRY_ENVIRONMENT ?? process.env.NODE_ENV,
+            profileSessionSampleRate: clamp01(Number(process.env.SENTRY_PROFILE_SESSION_SAMPLE_RATE ?? (process.env.NODE_ENV === "production" ? "0" : "1"))),
+            sendDefaultPii: (process.env.SENTRY_SEND_DEFAULT_PII ?? "").toLowerCase() === "true",
+            tracesSampleRate: clamp01(Number(process.env.SENTRY_TRACES_SAMPLE_RATE ?? (process.env.NODE_ENV === "production" ? "0.1" : "1"))),
+        });
+    }
+}
🧹 Nitpick comments (2)
controlplane/.env.example (1)

79-84: dotenv-linter: remove all value quotes or allow them in config; sort keys to fix ordering
controlplane/.env.example currently triggers 62 QuoteCharacter and UnorderedKey warnings. Either strip all quotes from the values (e.g. SENTRY_ENABLED=false) or update your dotenv-linter.toml to allow quoting, and reorder the keys (e.g. alphabetically) to clear the UnorderedKey errors.

controlplane/src/core/sentry.config.ts (1)

1-18: Export and invoke an explicit initSentry() instead of relying on import-time side effects
Verified that only src/index.ts dynamically imports core/sentry.config.ts under a strict process.env.SENTRY_ENABLED === "true" guard and no other modules reference it, and there are no typos in the SENTRY_* env-var names. You can refactor sentry.config.ts to:

  • Remove the top‐level if (process.env.SENTRY_ENABLED)
  • Export a function initSentry() that encapsulates the enable/DSN checks and calls Sentry.init()
  • In src/index.ts, replace the dynamic import with a call to initSentry() before bootstrapping

This makes Sentry setup explicit and avoids hidden side effects.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20b6dff and 99794d7.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • controlplane/.env.example (1 hunks)
  • controlplane/package.json (3 hunks)
  • controlplane/src/core/sentry.config.ts (1 hunks)
  • controlplane/src/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • controlplane/package.json
  • controlplane/src/index.ts
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
controlplane/.env.example

[warning] 79-79: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [UnorderedKey] The SENTRY_DSN key should go before the SENTRY_ENABLED key

(UnorderedKey)


[warning] 81-81: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 82-82: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [UnorderedKey] The SENTRY_PROFILE_SESSION_SAMPLE_RATE key should go before the SENTRY_SEND_DEFAULT_PII key

(UnorderedKey)


[warning] 84-84: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 84-84: [UnorderedKey] The SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS key should go before the SENTRY_PROFILE_SESSION_SAMPLE_RATE key

(UnorderedKey)

⏰ 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). (14)
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: image_scan
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: build_test
  • GitHub Check: integration_test (./events)
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: Analyze (go)
🔇 Additional comments (1)
controlplane/.env.example (1)

78-84: Verify dotenv-linter QuoteCharacter enforcement in CI
No dotenv-linter configuration or CI references found—please confirm that your CI doesn’t enforce the QuoteCharacter rule globally. If it does, drop the quotes in this block for consistency.

@miklosbarabas miklosbarabas force-pushed the miklos/eng-8036-setup-sentry-for-the-controlplane branch from 99794d7 to 71af95e Compare September 10, 2025 16:20
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
controlplane/.env.example (1)

78-84: Reorder Sentry keys to satisfy dotenv-linter; add helpful comments (no functional change).

This addresses the UnorderedKey warnings and documents gotchas. Keeping quotes for consistency with the rest of the file; if you plan to adopt dotenv-linter’s no-quotes rule, apply it file-wide later.

-# Sentry integration
-SENTRY_ENABLED="false"
-SENTRY_DSN=""
-SENTRY_SEND_DEFAULT_PII="false"
-SENTRY_TRACES_SAMPLE_RATE="1.0"
-SENTRY_PROFILE_SESSION_SAMPLE_RATE="1.0"
-SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS="100"
+# Sentry integration
+# Enable Sentry only with a valid DSN. Leave disabled by default.
+SENTRY_DSN=""
+SENTRY_ENABLED="false"
+# Optional: Sentry environment (defaults to NODE_ENV if unset).
+# SENTRY_ENVIRONMENT=""
+# Optional: release identifier for accurate source maps (e.g., git SHA).
+# SENTRY_RELEASE=""
+# Threshold in ms for event loop block detection.
+SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS="100"
+# Profiling/session sampling rate (0.0–1.0). See SDK docs for the exact option name.
+SENTRY_PROFILE_SESSION_SAMPLE_RATE="1.0"
+# Send PII only with proper legal basis.
+SENTRY_SEND_DEFAULT_PII="false"
+# Tracing sample rate (0.0–1.0). High rates can be costly in prod.
+SENTRY_TRACES_SAMPLE_RATE="1.0"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 99794d7 and 71af95e.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • controlplane/.env.example (1 hunks)
  • controlplane/package.json (3 hunks)
  • controlplane/src/core/sentry.config.ts (1 hunks)
  • controlplane/src/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • controlplane/src/index.ts
  • controlplane/package.json
  • controlplane/src/core/sentry.config.ts
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
controlplane/.env.example

[warning] 79-79: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [UnorderedKey] The SENTRY_DSN key should go before the SENTRY_ENABLED key

(UnorderedKey)


[warning] 81-81: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 82-82: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [UnorderedKey] The SENTRY_PROFILE_SESSION_SAMPLE_RATE key should go before the SENTRY_SEND_DEFAULT_PII key

(UnorderedKey)


[warning] 84-84: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 84-84: [UnorderedKey] The SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS key should go before the SENTRY_PROFILE_SESSION_SAMPLE_RATE key

(UnorderedKey)

⏰ 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). (16)
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: integration_test (./events)
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: build_push_image
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: image_scan
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: build_test
  • GitHub Check: build_test

@miklosbarabas miklosbarabas force-pushed the miklos/eng-8036-setup-sentry-for-the-controlplane branch from 8ee96d1 to b478289 Compare September 11, 2025 12:05
@miklosbarabas miklosbarabas force-pushed the miklos/eng-8036-setup-sentry-for-the-controlplane branch from b478289 to 39c2c25 Compare September 12, 2025 11:40
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
controlplane/.env.example (2)

79-81: Avoid truthy string pitfalls for booleans.

"false" is truthy if parsed with Boolean(). Ensure strict boolean parsing (or z.coerce.boolean()) for SENTRY_ENABLED and SENTRY_SEND_DEFAULT_PII. This was noted earlier; echoing here to keep it visible in the .env example context.


78-84: Use correct Sentry env var names (profilesSampleRate, eventLoopBlockIntegration threshold).

  • Rename SENTRY_PROFILE_SESSION_SAMPLE_RATE → SENTRY_PROFILES_SAMPLE_RATE (maps to profilesSampleRate).
  • Consider renaming SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS → SENTRY_EVENT_LOOP_BLOCK_INTEGRATION_THRESHOLD_MS for clarity with eventLoopBlockIntegration({ threshold }). (docs.sentry.io)

Apply within this block:

 # Sentry integration
 SENTRY_ENABLED="false"
 SENTRY_DSN=""
 SENTRY_SEND_DEFAULT_PII="false"
 SENTRY_TRACES_SAMPLE_RATE="1.0"
-SENTRY_PROFILE_SESSION_SAMPLE_RATE="1.0"
-SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS="100"
+SENTRY_PROFILES_SAMPLE_RATE="1.0"
+SENTRY_EVENT_LOOP_BLOCK_INTEGRATION_THRESHOLD_MS="100"

Follow-up: update controlplane/src/index.ts, controlplane/src/core/env.schema.ts, and controlplane/src/core/sentry.config.ts to use PROFILES naming and the new threshold key.

🧹 Nitpick comments (1)
controlplane/src/index.ts (1)

8-8: Mark type-only import to avoid unnecessary runtime import.

-import { SentryConfig } from './core/sentry.config.js';
+import type { SentryConfig } from './core/sentry.config.js';
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b478289 and 39c2c25.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • controlplane/.env.example (1 hunks)
  • controlplane/package.json (3 hunks)
  • controlplane/src/core/env.schema.ts (1 hunks)
  • controlplane/src/core/sentry.config.ts (1 hunks)
  • controlplane/src/index.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • controlplane/src/core/sentry.config.ts
  • controlplane/package.json
  • controlplane/src/core/env.schema.ts
🧰 Additional context used
🧬 Code graph analysis (1)
controlplane/src/index.ts (1)
controlplane/src/core/sentry.config.ts (1)
  • SentryConfig (7-16)
🪛 dotenv-linter (3.3.0)
controlplane/.env.example

[warning] 79-79: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 80-80: [UnorderedKey] The SENTRY_DSN key should go before the SENTRY_ENABLED key

(UnorderedKey)


[warning] 81-81: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 82-82: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 83-83: [UnorderedKey] The SENTRY_PROFILE_SESSION_SAMPLE_RATE key should go before the SENTRY_SEND_DEFAULT_PII key

(UnorderedKey)


[warning] 84-84: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)


[warning] 84-84: [UnorderedKey] The SENTRY_EVENT_LOOP_BLOCK_THRESHOLD_MS key should go before the SENTRY_PROFILE_SESSION_SAMPLE_RATE key

(UnorderedKey)

⏰ 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). (16)
  • GitHub Check: build_test
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: build_push_image
  • GitHub Check: image_scan
  • GitHub Check: integration_test (./events)
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: build_test
  • GitHub Check: build_test

@miklosbarabas
Copy link
Contributor Author

@endigma could you please re-review and resolve your previous comments if you're satisfied

@miklosbarabas miklosbarabas merged commit aed082e into main Sep 12, 2025
57 of 59 checks passed
@miklosbarabas miklosbarabas deleted the miklos/eng-8036-setup-sentry-for-the-controlplane branch September 12, 2025 13:40
@Noroth Noroth mentioned this pull request Sep 30, 2025
5 tasks
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.

3 participants