-
-
Notifications
You must be signed in to change notification settings - Fork 801
chore(webapp): upgrade otel packages and add more metrics #2458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughAdds three boolean environment flags (INTERNAL_OTEL_HOST_METRICS_ENABLED, INTERNAL_OTEL_NODEJS_METRICS_ENABLED, INTERNAL_OTEL_ADDITIONAL_DETECTORS_ENABLED) to the environment schema (defaults true). Removes the ECS async resource attributes provider module and its exported getAsyncResourceAttributes. Introduces a CompactMetricExporter for single-line metric output and uses it as the fallback when no OTLP metrics URL is configured. Reworks tracer/metrics wiring: dynamic resource detection via ATTR_SERVICE_NAME, new node and host metrics configuration, Prisma metrics refactor, inclusion of AwsSdkInstrumentation, an early no-op trace path, and related refactors. Upgrades OpenTelemetry dependencies and adjusts test tracing setup to pass spanProcessors via constructor. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (7)
apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts (2)
98-105
: Escape/serialize label values to avoid malformed lines and support arrays.
String() will break on quotes/newlines and render arrays poorly. Prefer JSON serialization.Apply:
- return entries.map(([key, value]) => `${key}="${String(value)}"`).join(","); + return entries + .map(([key, value]) => { + // JSON.stringify handles strings, numbers, booleans, and arrays safely + const serialized = JSON.stringify(value); + return `${key}=${serialized}`; + }) + .join(",");
59-67
: Reduce log spam by batching lines per metric (optional).
Console I/O per data point can be hot; batch per metric for readability and perf.Example:
- for (const dataPoint of metric.dataPoints) { - const formattedLine = this._formatDataPoint(metricName, dataPoint); - if (formattedLine) { - console.log(formattedLine); - } - } + const lines: string[] = []; + for (const dataPoint of metric.dataPoints) { + const line = this._formatDataPoint(metricName, dataPoint); + if (line) lines.push(line); + } + if (lines.length) console.log(lines.join("\n"));apps/webapp/app/v3/tracer.server.ts (5)
238-241
: Avoid duplicate resource detectiongetResource() is called twice (tracer and logger), doubling detector work (AWS/host detectors can be expensive). Cache once.
- const provider = new NodeTracerProvider({ + const resource = getResource(); + const provider = new NodeTracerProvider({ forceFlushTimeoutMillis: 15_000, - resource: getResource(), + resource, sampler: new ParentBasedSampler({ root: new CustomWebappSampler(new TraceIdRatioBasedSampler(samplingRate)), }), @@ - const loggerProvider = new LoggerProvider({ - resource: getResource(), + const loggerProvider = new LoggerProvider({ + resource,Also applies to: 266-268
321-323
: Metric reader timeout equals interval — prefer timeout < intervalexportTimeoutMillis equal to exportIntervalMillis risks overlapping exports under slow backends.
- exportTimeoutMillis: env.INTERNAL_OTEL_METRIC_EXPORTER_INTERVAL_MS, + exportTimeoutMillis: Math.floor(env.INTERNAL_OTEL_METRIC_EXPORTER_INTERVAL_MS * 0.8),
439-441
: Shadowing “metrics” identifierLocal const metrics shadows imported metrics API; minor readability nit.
- const metrics = await metricsRegister.getMetricsAsJSON(); + const promMetrics = await metricsRegister.getMetricsAsJSON();And update subsequent references accordingly.
60-60
: Namespace internal attribute keyforceRecording is generic; prefer a vendor-namespaced key to avoid collisions.
-export const SEMINTATTRS_FORCE_RECORDING = "forceRecording"; +export const SEMINTATTRS_FORCE_RECORDING = "trigger.dev.forceRecording";
553-556
: Consider lifecycle management for HostMetricsHostMetrics is started but never stopped; in long-lived processes this is fine, but during dev reloads it can accumulate timers.
Add a shutdown hook (only if HostMetrics exposes stop/shutdown in your version):
process.once("beforeExit", () => hostMetrics.shutdown?.());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/webapp/app/env.server.ts
(1 hunks)apps/webapp/app/v3/telemetry/asyncResourceAttributes.server.ts
(0 hunks)apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts
(1 hunks)apps/webapp/app/v3/telemetry/loggerExporter.server.ts
(2 hunks)apps/webapp/app/v3/tracer.server.ts
(11 hunks)apps/webapp/package.json
(2 hunks)apps/webapp/test/utils/tracing.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/webapp/app/v3/telemetry/asyncResourceAttributes.server.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/env.server.ts
apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts
apps/webapp/test/utils/tracing.ts
apps/webapp/app/v3/telemetry/loggerExporter.server.ts
apps/webapp/app/v3/tracer.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/env.server.ts
apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts
apps/webapp/test/utils/tracing.ts
apps/webapp/app/v3/telemetry/loggerExporter.server.ts
apps/webapp/app/v3/tracer.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/env.server.ts
apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts
apps/webapp/test/utils/tracing.ts
apps/webapp/app/v3/telemetry/loggerExporter.server.ts
apps/webapp/app/v3/tracer.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/env.server.ts
apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts
apps/webapp/app/v3/telemetry/loggerExporter.server.ts
apps/webapp/app/v3/tracer.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/env.server.ts
apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts
apps/webapp/app/v3/telemetry/loggerExporter.server.ts
apps/webapp/app/v3/tracer.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/env.server.ts (1)
apps/webapp/app/utils/boolEnv.ts (1)
BoolEnv
(12-14)
apps/webapp/app/v3/tracer.server.ts (3)
apps/webapp/app/env.server.ts (1)
env
(1102-1102)apps/webapp/app/metrics.server.ts (1)
metricsRegister
(5-5)apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts (1)
CompactMetricExporter
(15-153)
⏰ 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). (41)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
apps/webapp/app/v3/telemetry/loggerExporter.server.ts (2)
17-18
: Good change: type-only imports reduce bundle size and align with OTEL v2 types.
50-57
: Correct v2 migration to parentSpanContext.
Using span.parentSpanContext?.spanId matches the SDK v2 breaking change that removed parentSpanId on ReadableSpan. The guard for all-zero IDs is fine. (newreleases.io)apps/webapp/test/utils/tracing.ts (1)
8-10
: Constructor-based spanProcessors is the right pattern on SDK v2.
NodeTracerProvider accepts spanProcessors via config; addSpanProcessor was removed in v2. (github.com)apps/webapp/app/env.server.ts (1)
355-357
: Env toggles look good and follow existing BoolEnv pattern.
Defaults to true are reasonable given PR goals.apps/webapp/package.json (1)
71-77
: Verify OpenTelemetry package compatibility
- Confirm that @opentelemetry/[email protected] is officially tested against OpenTelemetry SDK v2.x
- Ensure all @opentelemetry/* packages (core v2.0.1, sdk-node v0.203.0, instrumentation v0.203.0, instrumentation-express v0.52.0, etc.) satisfy each other’s peerDependencies and avoid duplicate or incompatible ranges
apps/webapp/app/v3/tracer.server.ts (6)
238-249
: Verify NodeTracerProvider({ spanProcessors }) support; otherwise add processors after constructionSome versions only accept a single spanProcessor; others support spanProcessors[]. If unsupported, processors are ignored.
Fallback:
- const provider = new NodeTracerProvider({ + const provider = new NodeTracerProvider({ forceFlushTimeoutMillis: 15_000, - resource: getResource(), + resource: getResource(), sampler: new ParentBasedSampler({ root: new CustomWebappSampler(new TraceIdRatioBasedSampler(samplingRate)), }), spanLimits: { attributeCountLimit: 1024, }, - spanProcessors, }); + for (const sp of spanProcessors) provider.addSpanProcessor(sp);
259-265
: LoggerProvider processors in constructor may not be supportedIf LoggerProvider doesn’t accept processors in options for your version, the batch processor won’t be used.
Fallback:
- const batchLogExporter = new BatchLogRecordProcessor(logExporter, { + const batchLogExporter = new BatchLogRecordProcessor(logExporter, { maxExportBatchSize: 64, scheduledDelayMillis: 200, exportTimeoutMillis: 30000, maxQueueSize: 512, }); - const loggerProvider = new LoggerProvider({ + const loggerProvider = new LoggerProvider({ resource: getResource(), logRecordLimits: { attributeCountLimit: 1000, }, - processors: [batchLogExporter], }); + loggerProvider.addLogRecordProcessor(batchLogExporter);Also applies to: 271-271
331-334
: Metrics wiring LGTMPrisma, Node.js, and Host metrics are registered through the MeterProvider; clear separation and gating via env flags.
286-289
: Good call adding AwsSdkInstrumentation with suppressionThis prevents self-instrumentation loops and keeps spans clean.
620-622
: Compact metric exporter fallback is a sensible defaultProvides observability without OTLP config; concise one-line format is helpful in logs.
175-188
: Resource detection may be asynchronous — confirm your implementation
In apps/webapp/app/v3/tracer.server.ts (lines 175–188),detectResources
returns a Promise in recent versions of@opentelemetry/resources
. Verify your installed version and either replace this call withdetectResourcesSync
for purely synchronous detectors or make your initialization async andawait
the async resource merge.
There was a problem hiding this 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)
apps/webapp/app/v3/tracer.server.ts (2)
205-206
: Fix NaN sampling when INTERNAL_OTEL_TRACE_SAMPLING_RATE is unset/invalidparseInt can return NaN; Math.max(NaN, 1) → NaN, breaking the sampler. Default safely.
- const samplingRate = 1.0 / Math.max(parseInt(env.INTERNAL_OTEL_TRACE_SAMPLING_RATE, 10), 1); + const parsedRate = + Number.parseInt(env.INTERNAL_OTEL_TRACE_SAMPLING_RATE ?? "1", 10) || 1; + const samplingRate = 1.0 / Math.max(parsedRate, 1);
607-607
: Do not read process.env directly in webapp server codePer guidelines, use env accessor. Also add UV_THREADPOOL_SIZE to EnvironmentSchema.
- const uvThreadpoolSize = parseInt(process.env.UV_THREADPOOL_SIZE || "4", 10); + const uvThreadpoolSize = Number.parseInt(env.UV_THREADPOOL_SIZE ?? "4", 10) || 4;Reminder: add UV_THREADPOOL_SIZE to apps/webapp/app/env.server.ts (validated as optional numeric string).
🧹 Nitpick comments (2)
apps/webapp/app/v3/tracer.server.ts (2)
320-324
: Set a distinct export timeout for metricsTimeout equal to interval can cause overlapping flushes. Consider a separate env or a sane default (e.g., min(interval - 500, interval)).
- exportTimeoutMillis: env.INTERNAL_OTEL_METRIC_EXPORTER_INTERVAL_MS, + exportTimeoutMillis: + env.INTERNAL_OTEL_METRIC_EXPORTER_TIMEOUT_MS ?? + Math.max(1_000, Number(env.INTERNAL_OTEL_METRIC_EXPORTER_INTERVAL_MS) - 500),
380-408
: Unify time units across metricsPrisma gauges use ms while Node event loop gauges use seconds. Consider normalizing to seconds (preferred by OTel) to simplify dashboards and alerts.
Also applies to: 585-604
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/webapp/app/v3/tracer.server.ts
(11 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/v3/tracer.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/v3/tracer.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/v3/tracer.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/v3/tracer.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/v3/tracer.server.ts
🧠 Learnings (2)
📚 Learning: 2025-08-29T10:06:49.283Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-08-29T10:06:49.283Z
Learning: Applies to {apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts} : Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Applied to files:
apps/webapp/app/v3/tracer.server.ts
📚 Learning: 2025-08-29T10:06:49.283Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-08-29T10:06:49.283Z
Learning: Applies to apps/webapp/app/**/*.ts : Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Applied to files:
apps/webapp/app/v3/tracer.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/v3/tracer.server.ts (3)
apps/webapp/app/env.server.ts (1)
env
(1102-1102)apps/webapp/app/metrics.server.ts (1)
metricsRegister
(5-5)apps/webapp/app/v3/telemetry/compactMetricExporter.server.ts (1)
CompactMetricExporter
(15-153)
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this 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 (1)
apps/webapp/package.json (1)
284-284
: Resolved: engines now match OTEL v2 requirements.Engines bumped to ">=18.19.0 || >=20.6.0" addresses the prior blocker for OTEL 2.x. Thanks for fixing.
🧹 Nitpick comments (1)
apps/webapp/package.json (1)
65-83
: Align @opentelemetry versions workspace-wide
- api-logs is 0.52.1 in internal-packages/tracing vs 0.203.0 elsewhere
- instrumentation-aws-sdk (^0.57.0) and instrumentation-express (^0.52.0) vs instrumentation (0.203.0)
- semantic-conventions is ^1.27.0 in internal-packages/tracing vs 1.36.0 elsewhere
Use a single release line (e.g. 0.203.x for instrumentation/api-logs, 1.36.x for semantic-conventions) or add a pnpm override to enforce consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/webapp/package.json
(3 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). (40)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/webapp/package.json (2)
57-57
: LGTM: dependency reorder only.Moving @internal/redis below @internal/cache is non-functional. No concerns.
65-83
: Fix: @opentelemetry/host-metrics 0.36.x is likely incompatible with SDK v2 — bump to a newer 0.5x+.You upgraded the core/SDK to 2.0.1 and contrib/exporters to 0.203.0, but host-metrics is still at ^0.36.0 (from the pre-v2 era). That version is known to lag SDK APIs and can break or silently no-op with v2 metrics. Please align host-metrics to a recent 0.5x–0.57.x compatible release.
Apply:
- "@opentelemetry/host-metrics": "^0.36.0", + "@opentelemetry/host-metrics": "^0.57.0",Likely an incorrect or invalid review comment.
This updates the OpenTelemetry packages to the latest versions and adds comprehensive host metrics collection including CPU, memory, and network monitoring. The changes also introduce a more readable compact metric exporter and improved AWS resource detection capabilities.
Backwards Compatibility
This change maintains backwards compatibility as the OpenTelemetry API remains stable across versions and the internal telemetry changes do not affect external interfaces.