feat: OpenTelemetry för kapacitetsmätning#215
Open
johlju wants to merge 1 commit into
Open
Conversation
Contributor
WalkthroughThis PR adds OpenTelemetry-based observability for capacity events across the kravhantering application. It introduces environment configuration helpers, updates the Next.js instrumentation entry point to initialize OTel SDKs with HTTP exporters, implements capacity event recording for metrics, spans, and logs, and provides a complete Grafana LGTM dashboard for visualization. The implementation uses optional opt-in flags ( 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #215 +/- ##
==========================================
+ Coverage 69.31% 69.42% +0.10%
==========================================
Files 387 389 +2
Lines 22451 22544 +93
Branches 8250 8429 +179
==========================================
+ Hits 15562 15651 +89
- Misses 6708 6712 +4
Partials 181 181
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.devcontainer/docker-compose.yml (1)
95-95: ⚡ Quick winPin
grafana/otel-lgtmto a fixed tag or digest.Using
:latestmakes local environments non-reproducible and can introduce surprise breakages across rebuilds.Suggested change
- image: grafana/otel-lgtm:latest + image: grafana/otel-lgtm:<pin-a-tested-version>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.devcontainer/docker-compose.yml at line 95, Replace the floating image tag "grafana/otel-lgtm:latest" with a fixed, immutable reference (either a specific version tag like "grafana/otel-lgtm:vX.Y.Z" or a content digest "grafana/otel-lgtm@sha256:...") so local environments are reproducible; locate the image entry currently set to "grafana/otel-lgtm:latest" in the docker-compose service definition and update it to the chosen tag or digest, then rebuild to verify the pinned image resolves correctly..devcontainer/elevated/docker-compose.yml (1)
109-109: ⚡ Quick winPin
grafana/otel-lgtmto a fixed tag (or digest) in both compose files.Both elevated and default devcontainer profiles use
grafana/otel-lgtm:latest, which undermines reproducibility. Pin to a tested version in.devcontainer/docker-compose.yml(line 95) and.devcontainer/elevated/docker-compose.yml(line 109).Suggested change
- image: grafana/otel-lgtm:latest + image: grafana/otel-lgtm:<pin-a-tested-version>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.devcontainer/elevated/docker-compose.yml at line 109, Replace the floating image reference "image: grafana/otel-lgtm:latest" with a fixed, tested tag or digest to ensure reproducible builds; update the image line in both .devcontainer/docker-compose.yml and .devcontainer/elevated/docker-compose.yml (the lines currently showing "image: grafana/otel-lgtm:latest") to a specific version (e.g., "grafana/otel-lgtm:<version>" or "grafana/otel-lgtm@sha256:<digest>") and ensure both files use the same pinned value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@instrumentation.ts`:
- Around line 10-32: The dynamic imports and call to registerOTel can throw and
currently will reject register(), risking startup failure; wrap the entire async
block that imports `@vercel/otel`, OTLPLogExporter, OTLPMetricExporter,
BatchLogRecordProcessor, PeriodicExportingMetricReader and the call to
registerOTel(...) in a try/catch, log the caught error (including error details)
and return/degrade gracefully (do not rethrow) so telemetry failures do not
crash startup; ensure you still call getOpenTelemetryServiceName() only inside
the try if needed and preserve the existing configuration (logRecordProcessors
and metricReaders) when successful.
---
Nitpick comments:
In @.devcontainer/docker-compose.yml:
- Line 95: Replace the floating image tag "grafana/otel-lgtm:latest" with a
fixed, immutable reference (either a specific version tag like
"grafana/otel-lgtm:vX.Y.Z" or a content digest "grafana/otel-lgtm@sha256:...")
so local environments are reproducible; locate the image entry currently set to
"grafana/otel-lgtm:latest" in the docker-compose service definition and update
it to the chosen tag or digest, then rebuild to verify the pinned image resolves
correctly.
In @.devcontainer/elevated/docker-compose.yml:
- Line 109: Replace the floating image reference "image:
grafana/otel-lgtm:latest" with a fixed, tested tag or digest to ensure
reproducible builds; update the image line in both
.devcontainer/docker-compose.yml and .devcontainer/elevated/docker-compose.yml
(the lines currently showing "image: grafana/otel-lgtm:latest") to a specific
version (e.g., "grafana/otel-lgtm:<version>" or
"grafana/otel-lgtm@sha256:<digest>") and ensure both files use the same pinned
value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c78c25e0-d7a0-4131-beeb-71917620fc36
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
.devcontainer/.env.example.devcontainer/devcontainer.json.devcontainer/docker-compose.yml.devcontainer/elevated/devcontainer.json.devcontainer/elevated/docker-compose.yml.env.development.env.example.env.prodlikecspell.jsoncdev/grafana/provisioning/dashboard-provider.ymldev/grafana/provisioning/dashboards/kravhantering-capacity-observability.jsondocker-compose.otel.ymldocs/arkitekturbeskrivning-kravhantering.mddocs/capacity-management.mddocs/manual-test-cases.mddocs/poc-rhel10-podman.mdinstrumentation.tslib/observability/capacity-otel.tslib/observability/capacity.tslib/observability/otel-config.tspackage.jsontests/unit/capacity-observability.test.tstests/unit/capacity-otel.test.tstests/unit/otel-config.test.ts
Comment on lines
+10
to
+32
| const [ | ||
| { registerOTel }, | ||
| { OTLPLogExporter }, | ||
| { OTLPMetricExporter }, | ||
| { BatchLogRecordProcessor }, | ||
| { PeriodicExportingMetricReader }, | ||
| ] = await Promise.all([ | ||
| import('@vercel/otel'), | ||
| import('@opentelemetry/exporter-logs-otlp-http'), | ||
| import('@opentelemetry/exporter-metrics-otlp-http'), | ||
| import('@opentelemetry/sdk-logs'), | ||
| import('@opentelemetry/sdk-metrics'), | ||
| ]) | ||
|
|
||
| registerOTel({ | ||
| logRecordProcessors: [new BatchLogRecordProcessor(new OTLPLogExporter())], | ||
| metricReaders: [ | ||
| new PeriodicExportingMetricReader({ | ||
| exporter: new OTLPMetricExporter(), | ||
| }), | ||
| ], | ||
| serviceName: getOpenTelemetryServiceName(), | ||
| }) |
Contributor
There was a problem hiding this comment.
Guard OTel bootstrap failures to avoid startup breakage.
If any dynamic import or registerOTel(...) setup throws, register() rejects and can take down startup when telemetry is enabled. Please wrap initialization in try/catch and degrade gracefully.
Suggested patch
export async function register(): Promise<void> {
if (process.env.NEXT_RUNTIME === 'edge') return
if (!isOpenTelemetryEnabled()) return
- const [
- { registerOTel },
- { OTLPLogExporter },
- { OTLPMetricExporter },
- { BatchLogRecordProcessor },
- { PeriodicExportingMetricReader },
- ] = await Promise.all([
- import('`@vercel/otel`'),
- import('`@opentelemetry/exporter-logs-otlp-http`'),
- import('`@opentelemetry/exporter-metrics-otlp-http`'),
- import('`@opentelemetry/sdk-logs`'),
- import('`@opentelemetry/sdk-metrics`'),
- ])
-
- registerOTel({
- logRecordProcessors: [new BatchLogRecordProcessor(new OTLPLogExporter())],
- metricReaders: [
- new PeriodicExportingMetricReader({
- exporter: new OTLPMetricExporter(),
- }),
- ],
- serviceName: getOpenTelemetryServiceName(),
- })
+ try {
+ const [
+ { registerOTel },
+ { OTLPLogExporter },
+ { OTLPMetricExporter },
+ { BatchLogRecordProcessor },
+ { PeriodicExportingMetricReader },
+ ] = await Promise.all([
+ import('`@vercel/otel`'),
+ import('`@opentelemetry/exporter-logs-otlp-http`'),
+ import('`@opentelemetry/exporter-metrics-otlp-http`'),
+ import('`@opentelemetry/sdk-logs`'),
+ import('`@opentelemetry/sdk-metrics`'),
+ ])
+
+ registerOTel({
+ logRecordProcessors: [new BatchLogRecordProcessor(new OTLPLogExporter())],
+ metricReaders: [
+ new PeriodicExportingMetricReader({
+ exporter: new OTLPMetricExporter(),
+ }),
+ ],
+ serviceName: getOpenTelemetryServiceName(),
+ })
+ } catch (error) {
+ // eslint-disable-next-line no-console
+ console.error(
+ '[capacity-observability] failed to initialize OpenTelemetry',
+ error instanceof Error ? error.message : String(error),
+ )
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@instrumentation.ts` around lines 10 - 32, The dynamic imports and call to
registerOTel can throw and currently will reject register(), risking startup
failure; wrap the entire async block that imports `@vercel/otel`, OTLPLogExporter,
OTLPMetricExporter, BatchLogRecordProcessor, PeriodicExportingMetricReader and
the call to registerOTel(...) in a try/catch, log the caught error (including
error details) and return/degrade gracefully (do not rethrow) so telemetry
failures do not crash startup; ensure you still call
getOpenTelemetryServiceName() only inside the try if needed and preserve the
existing configuration (logRecordProcessors and metricReaders) when successful.
Member
Author
|
Follow-up from the platform best-practices cleanup: this PR covers the application-side OTel/capacity observability work, but it does not close the remaining Platform Log Routing work. What still remains after this PR:
In other words: #215 is a good app-side observability foundation, especially for capacity events, but the platform-side routing, ownership, retention, and production verification still need separate implementation/tracking. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Screenshots (if applicable)
Related Issues
Type of Change
Testing
npm run checkpasses locallyChecklist
Checklist
This change is