Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/add-observability-to-6x/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-09-16
66 changes: 66 additions & 0 deletions openspec/changes/add-observability-to-6x/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
## Context

`node-vtex-api@6.x` and `master` (`7.x`) have diverged on telemetry:

- **`6.x` today**: `src/service/telemetry/client.ts` is a `TelemetryClientSingleton` that calls `NewTelemetryClient(...)` once and hands back a single raw `TelemetryClient`. Its only consumer is `src/service/logger/client.ts`, which uses it purely to export structured logs. It depends on `@vtex/diagnostics-nodejs@0.1.0-beta.10`. There is no metrics client, no `DiagnosticsMetrics` API, no semantic-conventions package, no auto-instrumentation.
- **`master` today**: the same singleton pattern was extended into three clients (`newTracesClient`, `newMetricsClient`, `newLogsClient`) built from `@vtex/diagnostics-nodejs@0.1.8-io`, sharing `getClusterResourceAttributes()` for resource attribution, registering `KoaInstrumentation` and `HostMetricsInstrumentation`, and exposing metrics to app/library code through `src/metrics/DiagnosticsMetrics.ts` (histogram-based latency recording, counters, gauges, and OTel-context-scoped "base attributes" merged into every call). Feature activation is gated by `DIAGNOSTICS_TELEMETRY_ENABLED`.

This was withheld from `6.x` because of a build-time constraint, not a design constraint: `builder-hub` runs `node: 6.x` app builds (including its own — `builder-hub`'s manifest declares `"builders": {"node": "6.x"}`) on the `service-runtime-node@6.x` runtime image, which is `node:16.20.2`. Historically, `yarn install` for `major < 7` builds forked directly from that Node 16 process. When `master`'s diagnostics dependency tree grew, there was no verified path to install it reliably under that process for arbitrary app dependency trees, so 7.x got an escape hatch instead (`service-runtime-node@6.41.0` ships an extra `node20` binary in the same 6.x-branch image; `builder-hub`'s `NodeBuilder`/`yarn.ts` spawns it only when `nodeBuilderMajor >= 7`).

A dependency audit (done as part of scoping this change) resolved every `package.json` under `@vtex/diagnostics-nodejs@0.1.8-io`, `@vtex/diagnostics-semconv@5.5.2`, and the full `@opentelemetry/*` tree it pulls in (36 packages) plus `@grpc/*`. The highest `engines.node` floor found is `>=14` (`@opentelemetry/api` is `>=8`); nothing requires Node 18+, nothing needs native compilation (`@grpc/grpc-js` ships prebuilt, pure-JS), and nothing is ESM-only. Node 16.20.2 — what `builder-hub` and hosted `6.x` apps already run on — clears every floor in the tree. This removes the original blocker: the port can proceed without touching `builder-hub` or `service-runtime-node`.

## Goals / Non-Goals

**Goals:**
- Bring `DiagnosticsMetrics` (latency histogram, counters, gauges, request-scoped base attributes) to `node-vtex-api@6.x`, at parity with `master`'s current public API.
- Bring the split traces/metrics/logs `TelemetryClientSingleton`, cluster resource attributes, and Koa + host-metrics auto-instrumentation to `6.x`.
- **Wire `DiagnosticsMetrics` into the actual request pipeline** at boot (`service/index.ts`) and at every existing emission point `master` already has (HTTP handler timings, request-closed/aborted/total counters, outbound HTTP client metrics, HTTP agent metrics, the `@metric` GraphQL directive) — porting the API surface alone produces no observable data; this wiring is what makes metrics actually reach the OTLP endpoint.
- Ship the feature dark (`DIAGNOSTICS_TELEMETRY_ENABLED=false` by default) so existing `6.x` apps see no behavior change until explicitly opted in.
- Keep the existing `6.x` structured-logging consumer of the telemetry client working unchanged through the rewrite.

**Non-Goals:**
- No changes to `builder-hub` or `service-runtime-node` — the dependency audit shows none are required.
- No porting of the `HeaderKeys`/`AttributeKeys` constants refactor from `master`'s `constants.ts` beyond what `AttributeKeys` diagnostics needs (`VTEX_IO_CLUSTER_ID`, `VTEX_IO_CLUSTER_ROLE`, etc.) — that refactor is unrelated cleanup and stays out of scope.
- No attempt to reconcile `6.x` and `master`'s `DiagnosticsMetrics` implementations into a shared package — this is a straight backport, duplication across branches is accepted (consistent with how `node-vtex-api` already maintains divergent major-version branches).
- No new capability surface beyond what `master` already exposes — this change ports existing behavior, it doesn't design new metrics APIs.

## Decisions

**Port `master`'s implementation as-is rather than redesigning.** `DiagnosticsMetrics.ts`, `resourceAttributes.ts`, and the split-client shape of `telemetry/client.ts` are stable, tested, and already running in production on `7.x`. Re-deriving them for `6.x` risks behavioral drift between branches for what should be the same feature. The only intentional deviations are the ones required by `6.x`'s existing shape (e.g., `6.x`'s logger already calls `getTelemetryClient()` expecting *a* client back — that call site is adapted to pull `logsClient` from the new multi-client shape rather than rewriting the logger).

**Bump `@vtex/diagnostics-nodejs` directly from `0.1.0-beta.10` to `0.1.8-io`, not incrementally.** `6.x`'s current usage surface (`NewTelemetryClient`, `TelemetryClient` type) is narrow enough that reviewing the target version's changelog/breaking changes once, at the version already proven on `master`, is lower-risk than bisecting through intermediate betas that were never shipped to a stable consumer.

**Gate the entire feature behind `DIAGNOSTICS_TELEMETRY_ENABLED`, matching `master`.** Reusing the existing flag (rather than inventing a `6.x`-specific one) keeps operational tooling (dashboards, rollout scripts, on-call runbooks) that already understands this flag from `7.x` valid for `6.x` too.

**Treat the jest mock for `@vtex/diagnostics-nodejs` as part of this change, not a follow-up.** `6.x` already carries a stub shaped for the old single-client API (added under `test(jest): stub @vtex/diagnostics-nodejs so metrics suites load under jest@25`); it must be updated to the `Exporters`/`Instrumentation`/multi-client shape `master`'s test suite mocks, or the new tests (ported alongside the implementation) won't load.

**Wire `global.diagnosticsMetrics` into the request pipeline exactly where `master` does, with the same "guarded, graceful-degradation" pattern.** `master`'s middlewares check `if (global.diagnosticsMetrics) { ... } else { console.warn('DiagnosticsMetrics not available. ... not reported.') }` at every emission point, rather than assuming it's always initialized. Porting that exact guard (rather than assuming initialization always succeeds) means a telemetry-client init failure degrades to "metrics not reported, warning logged" instead of a crash — consistent with how `6.x` already treats telemetry as best-effort for logging.

## Implementation Addenda

Decisions made during implementation that this document didn't originally anticipate:

1. **`TelemetryClientSingleton` exposes a `getTelemetryClient()` getter for the raw `@vtex/diagnostics-nodejs` `TelemetryClient`, in addition to `master`'s three pre-built clients.** `6.x`'s structured logger builds a *dynamic* per-request logs client (`newLogsClient()` with a loggerName derived from `account`/`workspace`/`appName` passed at call time), which `master`'s fixed-at-init three-client shape doesn't support. Exposing the raw client lets the logger keep this exact behavior with zero changes to its call site or public signature — the only diff is what backs `getTelemetryClient()` internally.
2. **jest's bundled resolver (jest 25) doesn't support `package.json` "exports" maps**, so `@opentelemetry/otlp-exporter-base/node-http` — a subpath pulled in transitively by the diagnostics dependency bump — failed to resolve under tests even though Node's own runtime `require()` resolves it fine. Fixed with a `moduleNameMapper` entry in `jest.config.js` pointing straight at the package's build output. This is a test-infrastructure-only fix; it doesn't affect the engines/Node-version analysis above, which is about production runtime resolution, not jest's resolver.
3. **A real breaking change surfaced between `@vtex/diagnostics-nodejs` versions**: `Exporters.CreateLogsExporterConfig`'s `ExporterOptions` type dropped `path`, `protocol`, and `headers` between `0.1.0-beta.10` and `0.1.8-io`. `6.x`'s logger passed all three; they were removed to match the new type (matching `master`'s simpler usage, which only ever passed `endpoint`).
4. **The request-pipeline wiring (this addendum's reason for existing) was missing from the first implementation pass entirely** — found only after deploying to a live test cluster (`iotest-ju2`) and observing no metrics reaching ClickHouse despite the flag being enabled. Root-caused to `service/index.ts` never calling `initializeTelemetry()`/setting `global.diagnosticsMetrics`, and none of the five consumer middlewares/directive existing on `6.x` yet. This is now folded into the Goals/Decisions above and into `tasks.md` rather than treated as a separate follow-up change, since it's required for the capability described in `specs/diagnostics-metrics/spec.md` to do anything observable.

## Risks / Trade-offs

- **[Risk]** `@vtex/diagnostics-nodejs` jumped from a `0.1.0` beta to `0.1.8-io` — an 8-patch, beta-to-"io"-tagged gap whose changelog hasn't been reviewed line-by-line yet. → **Mitigation**: review the package's changelog/tags between the two versions as an explicit task before wiring the new client shape; since `master` already runs `0.1.8-io` in production, any incompatibility surfaces as a diff against known-working behavior, not unknown territory.
- **[Risk]** Every app on the `node: 6.x` builder gains these dependencies transitively (larger `node_modules`, more install time), even for apps that never enable `DIAGNOSTICS_TELEMETRY_ENABLED`. → **Mitigation**: the dependency audit confirms no install-time failure risk (engines, no native builds); the cost is disk/time, not correctness, and is already accepted on `7.x` today.
- **[Risk]** `builder-hub`'s own runtime is Node 16.20.2 today, but that's inferred from the `service-runtime-node` `6.x` branch's `Dockerfile`, not from a live deployment check. If the pinned production image ever diverges from that branch, the "Node 16 clears `>=14`" conclusion still holds unless production somehow regresses below Node 14 — considered very unlikely but not independently verified against the live cluster. → **Mitigation**: no action required given the margin (14 vs. 16), but flagged as an assumption for whoever owns the `builder-hub` deploy pipeline to confirm if they want extra certainty.
- **[Trade-off]** Keeping `6.x` and `master`'s diagnostics code as parallel, duplicated implementations (per Non-Goals) means future diagnostics changes must be ported twice. Accepted because `node-vtex-api` already operates this way across its major-version branches.

## Migration Plan

1. Land the dependency bump, telemetry/metrics port, **and the request-pipeline wiring** on `6.x` with `DIAGNOSTICS_TELEMETRY_ENABLED` defaulting to off (matching `master`'s rollout pattern) — no consumer app sees any behavior change on upgrade. (Landing the API port without the wiring, as the first implementation pass did, ships something that compiles and tests green but produces no observable data — not a safe intermediate state to call "done.")
2. Validate against a small set of `6.x` apps in a non-production workspace with the flag manually enabled, confirming metrics land in the same backend `master`-based apps report to.
3. Roll out the `6.x` `node-vtex-api` version bump to consumer apps at their own pace (standard dependency bump, no forced migration).
4. Enable `DIAGNOSTICS_TELEMETRY_ENABLED` for interested `6.x` apps individually; no fleet-wide flip is required or planned as part of this change.
5. **Rollback**: reverting the `node-vtex-api@6.x` version bump in a consumer app fully reverts behavior — the change is additive and flag-gated, so no data migration or cleanup is needed on rollback.

## Open Questions

- Does anyone need `6.x`'s `DiagnosticsMetrics` API to diverge from `master`'s (e.g., different default histogram buckets, different max custom-attribute limit) given `6.x` apps may have different traffic/cardinality profiles, or is exact parity the right target?
- Should the `@vtex/diagnostics-nodejs` changelog review (Risk 1) block this change's merge, or can it happen as a fast-follow given `master` already validates the target version in production?
38 changes: 38 additions & 0 deletions openspec/changes/add-observability-to-6x/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
## Why

`node-vtex-api@6.x` apps currently ship with no metrics observability: the branch only has a bare `TelemetryClient` used exclusively to export structured logs, while `master` (`7.x`) has grown a full traces/metrics/logs stack (`DiagnosticsMetrics`, cluster resource attributes, Koa + host-metrics auto-instrumentation) built on `@vtex/diagnostics-nodejs`. Observability was withheld from `6.x` because `builder-hub` could not reliably build `7.x`-only dependency trees requiring Node > 16 at build time. That build-time isolation gap has since been closed (`service-runtime-node@6.41.0`+ provides a `node20` binary that `builder-hub` spawns for `node: 7.x` app builds), and a dependency audit of the diagnostics/OpenTelemetry package tree (`@vtex/diagnostics-nodejs@0.1.8-io`, `@vtex/diagnostics-semconv@5.5.2`, the `@opentelemetry/*` family) shows its highest `engines.node` floor is `>=14` — well within what the `node: 6.x` builder runtime (Node 16.20.2, per `service-runtime-node`'s `6.x` branch) already supports. There is no longer a technical blocker to bringing metrics observability to `6.x`.

## What Changes

- Bump `@vtex/diagnostics-nodejs` in `node-vtex-api@6.x` from `0.1.0-beta.10` to `0.1.8-io`, and add `@vtex/diagnostics-semconv` plus the `@opentelemetry/{api,host-metrics,instrumentation,instrumentation-koa}` dependencies already used on `master`.
- Rewrite `src/service/telemetry/client.ts` from a single bare `TelemetryClient` into the split traces/metrics/logs `TelemetryClientSingleton` shape used on `master`, preserving the existing logger consumer (`src/service/logger/client.ts`) which only needs the logs client.
- Port `src/service/telemetry/resourceAttributes.ts` (cluster id/role resource attributes) and its tests.
- Port `src/metrics/DiagnosticsMetrics.ts` (the `recordLatency` / `incrementCounter` / `setGauge` / `runWithBaseAttributes` public API) and its tests, unchanged from `master`.
- Add the supporting constants already present on `master` but missing on `6.x`: `AttributeKeys` (sourced from `@vtex/diagnostics-semconv`), `CLUSTER_ID`, `CLUSTER_ROLE`, `METRIC_CLIENT_INIT_TIMEOUT_MS`, `OTEL_EXPORTER_OTLP_ENDPOINT`, `DIAGNOSTICS_TELEMETRY_ENABLED`. The `HeaderKeys` refactor on `master` is unrelated cleanup and is **out of scope**.
- Wire Koa auto-instrumentation and host-metrics collection into the `6.x` service bootstrap, gated behind the existing `DIAGNOSTICS_TELEMETRY_ENABLED` env flag so the feature ships dark by default.
- Update jest mocking for `@vtex/diagnostics-nodejs` (`6.x` already stubs an older shape for jest@25 compatibility; the stub needs to match the new client surface).
- **Wire `DiagnosticsMetrics` into the actual request pipeline**, matching `master`: call `initializeTelemetry()` and set `global.diagnosticsMetrics = new DiagnosticsMetrics()` in `src/service/index.ts`'s `startApp()`, and add the corresponding `global.diagnosticsMetrics` emission calls (guarded by a `global.diagnosticsMetrics` existence check, matching `master`'s graceful-degradation pattern) to:
- `src/service/worker/runtime/http/middlewares/timings.ts` (per-request HTTP handler latency + counter, with request-scoped base attributes via `runWithBaseAttributes`)
- `src/service/worker/runtime/http/middlewares/requestStats.ts` (request closed/aborted/total counters)
- `src/HttpClient/middlewares/metrics.ts` (outbound HTTP client metrics)
- `src/HttpClient/middlewares/request/HttpAgentSingleton.ts` (HTTP agent metrics)
- `src/service/worker/runtime/graphql/schema/schemaDirectives/Metric.ts` (`@metric` GraphQL directive)

Without this wiring, the ported `DiagnosticsMetrics` class and telemetry client are reachable but inert: nothing in `6.x`'s request path ever calls them, so no per-request/per-operation data reaches the configured OTLP endpoint even with the feature flag on. (This gap was found only after deploying `6.53.0-beta.0` to a test cluster and observing no metrics landing in ClickHouse — the original proposal covered porting the API surface but not this wiring layer.)

No changes to `builder-hub` or `service-runtime-node` are required by this change — the build-time and runtime environments already support this dependency tree on the `node: 6.x` builder.

## Capabilities

### New Capabilities
- `diagnostics-metrics`: the `DiagnosticsMetrics` public API (latency histogram, counters, gauges, request-scoped base attributes) and its underlying `TelemetryClientSingleton` (traces/metrics/logs clients, cluster resource attributes, Koa + host-metrics instrumentation) as available to `node-vtex-api@6.x` consumers, feature-flagged via `DIAGNOSTICS_TELEMETRY_ENABLED`.

### Modified Capabilities
_None — `6.x`'s existing telemetry-backed structured logging keeps its current behavior; it is only extended, not changed._

## Impact

- **Affected code**: `src/service/telemetry/*`, `src/metrics/DiagnosticsMetrics.ts` (new), `src/constants.ts`, `__mocks__/@vtex/diagnostics-nodejs`, jest config for the new mock surface, `src/service/index.ts`, and the five request-pipeline middleware/directive files listed above.
- **Dependencies**: `package.json` gains `@vtex/diagnostics-semconv` and the `@opentelemetry/*` quartet; `@vtex/diagnostics-nodejs` is bumped across a major beta-to-stable jump (`0.1.0-beta.10` → `0.1.8-io`) — its own changelog/breaking changes need review during implementation.
- **Consumers**: every VTEX IO app on the `node: 6.x` builder gains these dependencies transitively; verified they resolve under the `6.x` builder's Node 16.20.2 runtime with no native compilation or ESM-resolution requirements.
- **Systems**: no changes needed in `builder-hub` or `service-runtime-node`; this is scoped entirely to `node-vtex-api`.
Loading