feat(telemetry): add cluster dimensions to diagnostics

[juliobguedes]

What is the purpose of this pull request?

Add Kubernetes cluster identity and role as shared diagnostics resource attributes:

• VTEX_CLUSTER_ID → cluster_id
• VTEX_CLUSTER_ROLE → cluster_role

The values are trimmed, omitted independently when empty, and applied centrally to diagnostics metrics and logs without consuming per-call custom attribute limits.

What problem is this solving?

Diagnostics telemetry could not be grouped or filtered by its originating Kubernetes cluster, making cross-cluster dashboards and incident isolation harder.

How should this be manually tested?

• yarn build
• yarn lint
• yarn test --runInBand
• Start the service with VTEX_CLUSTER_ID and VTEX_CLUSTER_ROLE, then confirm the diagnostics resource attributes contain cluster_id and cluster_role.

Screenshots or example usage

Not applicable.

Types of changes

• Bug fix (a non-breaking change which fixes an issue)
• New feature (a non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Requires change to documentation, which has been updated accordingly.

[silvadenisaraujo]

LGTM 🔥

[dk-pr-review]

DK Review — Audit Summary

Verdict: ⚠️ RESTRICTED

Severity	Count
BLOCK	0
RESTRICT	6
SUGGEST	11

Scenarios evaluated: dependency-governance, general-review, quality-ratchet
Scenarios skipped: pipeline-config (no matching files), agent-skills-review (no matching files)

---

📋 Findings (17)

RESTRICT

1. [Functional.Support] package.json:57 — @vtex/diagnostics-semconv jumps from ^1.1.2 to 5.5.2 — four major versions in a single bump. Semantic-convention packages export attribute-name constants that are routinely renamed or removed across majors, so telemetry attribute keys emitted by this library can silently change or fail to compile. goldenPathGetMigrationGuide(technology=@vtex/diagnostics-semconv, from=1.1.2, to=5.5.2) returned an empty guide, so no vetted migration path exists for this leap; the 12 non-lockfile files in this PR appear to be the hand-rolled adaptation and were not covered by any Golden Path validation.
   → Confirm with the diagnostics owners that 5.5.2 is the intended target for @vtex/api, and document the 1.x→5.x breaking changes (renamed/removed constants) in the PR description. If the goal is only a patch/minor uplift, retarget to the latest 1.x instead.
2. [Functional.Support] package.json:57 — @vtex/diagnostics-semconv jumps four major versions (^1.1.2 → 5.5.2) in a single change whose stated purpose is only to obtain two new cluster constants. Semantic-convention packages routinely rename or remove attribute constants across majors, and this repo consumes five pre-existing exports (ATTR_VTEX_ACCOUNT_NAME, ATTR_VTEX_IO_WORKSPACE_NAME/TYPE, ATTR_VTEX_IO_APP_ID, ATTR_VTEX_IO_APP_AUTHOR_TYPE) whose key values could silently change without breaking compilation — which would rename dimensions on already-published dashboards and alerts. goldenPathGetMigrationGuide returned no guide for this transition, so no migration path was validated. Additionally, @vtex/api is a published library and this is a runtime dependency: an exact pin (no caret, unlike every other @vtex/* entry except diagnostics-nodejs) can force duplicate installs for consumers that depend on the same package.
   → Document in the PR (or design.md) the diff of attribute values between 1.1.2 and 5.5.2 for the five constants already in use, confirming none of the emitted keys changed. Confirm the exact pin is deliberate given the package is a transitive runtime dependency of @vtex/api.
3. [Functional.Check] src/constants.test.ts:142 — The new assertions expect(AttributeKeys.VTEX_IO_CLUSTER_ID).toBe('vtex_io.cluster.id') validate the manual mock, not the real package. __mocks__/@vtex/diagnostics-semconv.ts sits at the project root and mocks a node_modules module, so Jest applies it automatically to every test file without any jest.mock() call (jest config in package.json sets no roots/modulePaths that would exclude it). These assertions therefore pass by construction from the two lines added to the mock and prove nothing about @vtex/diagnostics-semconv@5.5.2. This is the exact claim tasks.md 4.1/4.2 says was verified, so the suite gives false confidence: if 5.5.2 exported a different key string (e.g. vtex_io.k8s.cluster.id), tests stay green and production emits the wrong dimension name.
   → Add one assertion that imports the real package (bypassing the manual mock via jest.unmock('@vtex/diagnostics-semconv') or jest.requireActual) and checks that ATTR_VTEX_IO_CLUSTER_ID/ATTR_VTEX_IO_CLUSTER_ROLE exist and equal the expected keys — otherwise the mock and the dependency can drift undetected.
4. [Functional.Logic] src/service/telemetry/client.test.ts:113 — Vacuous assertion: expect(options.additionalAttrs).not.toHaveProperty('vtex_io.cluster.id') — Jest treats a dotted string as a nested property path (options.additionalAttrs.vtex_io.cluster.id), not as the flat key 'vtex_io.cluster.id'. Since the attribute is stored as a single flat key containing dots, this negative assertion can never fail and provides zero regression protection for the 'no cluster metadata' path.
   → Pass the key as an array to force literal-key matching: expect(options.additionalAttrs).not.toHaveProperty(['vtex_io.cluster.id']), or assert on keys directly, e.g. expect(Object.keys(options.additionalAttrs)).not.toContain('vtex_io.cluster.id').
5. [Functional.Logic] src/service/telemetry/client.test.ts:114 — Second occurrence of the same vacuous assertion: expect(options.additionalAttrs).not.toHaveProperty('vtex_io.cluster.role') is interpreted as the nested path additionalAttrs.vtex_io.cluster.role and therefore always passes, even if the flat key 'vtex_io.cluster.role' is present with a value.
   → Use the array form not.toHaveProperty(['vtex_io.cluster.role']), and consider asserting the whole object with toEqual so unexpected keys are caught: expect(options.additionalAttrs).toEqual({ 'vtex_io.app.id': ..., vendor: ..., version: ..., 'vtex_io.workspace.name': ..., 'vtex_io.workspace.type': ... }).
6. [Functional.Support] src/constants.ts:8 — The diff imports the new symbols ATTR_VTEX_IO_CLUSTER_ID and ATTR_VTEX_IO_CLUSTER_ROLE from @vtex/diagnostics-semconv, but package.json still declares "@vtex/diagnostics-semconv": "^1.1.2" and is not bumped in this PR. If these attributes were introduced after 1.1.2, any consumer (or a fresh install without the updated lockfile) resolving 1.1.2 gets undefined for both keys, producing resource attributes keyed by undefined instead of a compile/install failure.
   → Raise the minimum version of @vtex/diagnostics-semconv in package.json to the release that first exports ATTR_VTEX_IO_CLUSTER_ID/ATTR_VTEX_IO_CLUSTER_ROLE (and update yarn.lock in the same PR). If 1.1.2 already exports them, no change is needed — please confirm explicitly.

SUGGEST

1. [dep-gov.version-consistency] yarn.lock:1295 — After this change the lockfile still resolves a second, major-incompatible copy of the same package: the entry immediately above the hunk keeps @vtex/diagnostics-semconv at 0.1.0-beta.11 (pulled in transitively) while the direct dependency now resolves to 5.5.2. Two versions of a semantic-convention package coexisting in node_modules means different call sites can import different attribute-key constants, producing inconsistent telemetry attribute names at runtime — a failure mode that does not surface at build time.
   → Check which package still pins 0.1.0-beta.11 (yarn why @vtex/diagnostics-semconv) — most likely @vtex/diagnostics-nodejs@0.1.8-io, which is still on a 0.1.x line and may not be compatible with semconv 5.x. Either bump that dependency in step or add a resolutions entry to collapse both to a single version, as already done for @grpc/grpc-js.
2. [dep-gov.version-guard] package.json:57 — The version-history shape of this dependency is unusual enough to be worth a provenance check before merge: the lockfile records prior resolutions of 0.1.0-beta.11 and 1.1.2, and the sibling package @vtex/diagnostics-nodejs is still on the internal 0.1.8-io prerelease line, yet this bump lands on a public-registry 5.5.2. A version far ahead of an internal package's real release line is the signature of a dependency-confusion publish, and the new resolution points at registry.yarnpkg.com rather than an internal registry.
   → Verify on the registry that @vtex/diagnostics-semconv@5.5.2 was published by the VTEX org and matches the diagnostics team's release line before merging. If it is legitimate, no change is needed — record the confirmation in the PR.
3. [Evolvability.SupportedByLanguage] src/constants.ts:172 — CLUSTER_ID and CLUSTER_ROLE are cast with as string, but the design and spec explicitly define both as optional deployment metadata that is frequently absent (local runs, non-Kubernetes runtimes). The cast asserts a guarantee the runtime does not provide, so any future consumer that does CLUSTER_ID.toUpperCase() or passes it to a string-typed parameter compiles cleanly and throws or emits "undefined" at runtime. getClusterResourceAttributes happens to accept string | undefined, so the current call site is safe only by coincidence.
   → Type these as process.env.VTEX_CLUSTER_ID (inferred string | undefined) rather than as string, so the compiler enforces the optionality the spec requires. The pre-existing REGION cast follows the same anti-pattern but is out of scope here.
4. [Functional.Check] src/constants.test.ts:227 — expect(CLUSTER_ID).toBe(process.env.VTEX_CLUSTER_ID as string) is vacuous in CI: VTEX_CLUSTER_ID and VTEX_CLUSTER_ROLE are almost certainly unset there, so both sides evaluate to undefined and the assertion passes regardless of what the constant does. This is the only test covering the env-var → constant link (client.test.ts mocks ../../constants, and resourceAttributes.test.ts passes literals), so nothing in the suite actually proves the code reads VTEX_CLUSTER_ID rather than, say, VTEX_CLUSTER.
   → Set the env vars to known values and re-require the module (jest.resetModules() + require('./constants')) so the assertion compares a real configured value, or assert against an explicit literal.
5. [general.error-handling] src/service/telemetry/resourceAttributes.ts:12 — Missing cluster metadata is dropped completely silently. The design explicitly rejects unknown placeholders, which is reasonable, but it also means a production pod deployed without VTEX_CLUSTER_ID emits metrics and logs that are simply absent from every cluster-segmented dashboard, with no signal anywhere that the variable was never injected. The failure mode is invisible until someone notices a gap in a query — exactly the scenario the PR is trying to make debuggable.
   → When PRODUCTION is true and either value normalizes to undefined, emit a one-time console.warn at telemetry initialization naming the missing variable. Keep the attribute omitted as designed — only the diagnostic is added.
6. [general.broken-references] openspec/changes/add-k8s-cluster-telemetry-dimensions/design.md:42 — The design cites vtex/diagnostics#174 as the source of the vtex_io.cluster.id/vtex_io.cluster.role constants and asserts that 5.5.2 is "the first public version adopted here that contains both cluster constants". Neither claim can be confirmed from this diff, and both are load-bearing: the version choice in package.json rests on them.
   → Please double-check that vtex/diagnostics#174 is the correct reference and is reachable by readers of this repo, and confirm the 5.5.2 claim against the published changelog.
7. [general.broken-references] openspec/changes/add-k8s-cluster-telemetry-dimensions/proposal.md:25 — Second occurrence of the unverifiable vtex/diagnostics#174 reference, in the Impact section.
   → Verify the reference alongside the one in design.md; if the issue/PR number is wrong, both files need correcting.
8. [Evolvability.SupportedByLanguage] src/constants.ts:173 — CLUSTER_ID/CLUSTER_ROLE are declared as process.env.VTEX_CLUSTER_ID as string, asserting a non-optional string while the values are genuinely optional at runtime (the new getClusterResourceAttributes(clusterId?: string, clusterRole?: string) signature and its 'metadata unavailable' test both acknowledge this). The cast hides the undefined case from every other consumer of these exports.
   → Type them honestly as export const CLUSTER_ID: string | undefined = process.env.VTEX_CLUSTER_ID (same for CLUSTER_ROLE), so the compiler forces callers to handle absence rather than relying on getClusterResourceAttributes being the only reader.
9. [quality.new-logic-enforcement] src/service/telemetry/client.test.ts:38 — The new test file is the only coverage for client.ts, but it hardcodes DIAGNOSTICS_TELEMETRY_ENABLED: false inside the jest.mock factory, so the enabled branch (instrumentation registration, metricsClient.provider(), KoaInstrumentation, HostMetricsInstrumentation) and the catch path can never be exercised. Coverage for initializeTelemetryClients is therefore structurally capped at the disabled path.
   → Use jest.doMock/jest.isolateModules (or read the flag through a getter) so a third case can set DIAGNOSTICS_TELEMETRY_ENABLED: true and assert registerInstrumentations is called with the host-metrics instrumentation, plus a case where NewTelemetryClient rejects and the error is rethrown.
10. [quality.new-logic-enforcement] src/constants.test.ts:227 — expect(CLUSTER_ID).toBe(process.env.VTEX_CLUSTER_ID as string) compares the constant against the very expression that produced it, so it passes trivially when VTEX_CLUSTER_ID is unset (both sides are undefined) — which is the normal state in CI. The two added lines raise the line-coverage number for the new constants without verifying any behaviour.
    → Set the env vars to known values before importing the module (or via jest.resetModules() + process.env.VTEX_CLUSTER_ID = 'cluster-a') and assert the concrete value, so the test fails if the constant is wired to the wrong env var name.
11. [Evolvability.Textual] src/service/telemetry/client.test.ts:80 — The test is named 'shares configured cluster resource attributes across metrics and logs', but its assertions verify the opposite shape: that newMetricsClient and newLogsClient are called WITHOUT additionalAttrs. The actual invariant being tested is that cluster attributes are set once at the NewTelemetryClient resource level and deliberately not repeated per-signal; the name misleads a future reader into thinking per-client attributes are expected.
    → Rename to something like 'sets cluster resource attributes once on the telemetry resource, not per signal client', so the assertion block matches the stated intent.

---

DK Review v1.0.0 | To dismiss a finding: reply /dk-review dismiss <finding-id> [reason]

[dk-pr-review]

[Functional.Support] 🟡 RESTRICT

@vtex/diagnostics-semconv jumps from ^1.1.2 to 5.5.2 — four major versions in a single bump. Semantic-convention packages export attribute-name constants that are routinely renamed or removed across majors, so telemetry attribute keys emitted by this library can silently change or fail to compile. goldenPathGetMigrationGuide(technology=@vtex/diagnostics-semconv, from=1.1.2, to=5.5.2) returned an empty guide, so no vetted migration path exists for this leap; the 12 non-lockfile files in this PR appear to be the hand-rolled adaptation and were not covered by any Golden Path validation.

Action: Confirm with the diagnostics owners that 5.5.2 is the intended target for @vtex/api, and document the 1.x→5.x breaking changes (renamed/removed constants) in the PR description. If the goal is only a patch/minor uplift, retarget to the latest 1.x instead.
Source: goldenPathGetMigrationGuide(@vtex/diagnostics-semconv, 1.1.2 → 5.5.2) → {"title":"","steps":[],"estimatedEffort":"unknown"}

To dismiss: /dk-review dismiss 9f3c1a7e-52b4-4d18-8a6c-2e7d9b0f4c31 [reason]

[dk-pr-review]

[dep-gov.version-consistency] 🔵 SUGGEST

After this change the lockfile still resolves a second, major-incompatible copy of the same package: the entry immediately above the hunk keeps @vtex/diagnostics-semconv at 0.1.0-beta.11 (pulled in transitively) while the direct dependency now resolves to 5.5.2. Two versions of a semantic-convention package coexisting in node_modules means different call sites can import different attribute-key constants, producing inconsistent telemetry attribute names at runtime — a failure mode that does not surface at build time.

Action: Check which package still pins 0.1.0-beta.11 (yarn why @vtex/diagnostics-semconv) — most likely @vtex/diagnostics-nodejs@0.1.8-io, which is still on a 0.1.x line and may not be compatible with semconv 5.x. Either bump that dependency in step or add a resolutions entry to collapse both to a single version, as already done for @grpc/grpc-js.
Source: goldenPathGetRules(vtex/node-vtex-api, [package.json, yarn.lock]) → {"rules":[]} — no Golden Path rule backs this, so reported as SUGGEST

To dismiss: /dk-review dismiss c47a8d20-6b91-4f53-9e12-8d3a5c6b7f04 [reason]

[dk-pr-review]

[dep-gov.version-guard] 🔵 SUGGEST

The version-history shape of this dependency is unusual enough to be worth a provenance check before merge: the lockfile records prior resolutions of 0.1.0-beta.11 and 1.1.2, and the sibling package @vtex/diagnostics-nodejs is still on the internal 0.1.8-io prerelease line, yet this bump lands on a public-registry 5.5.2. A version far ahead of an internal package's real release line is the signature of a dependency-confusion publish, and the new resolution points at registry.yarnpkg.com rather than an internal registry.

Action: Verify on the registry that @vtex/diagnostics-semconv@5.5.2 was published by the VTEX org and matches the diagnostics team's release line before merging. If it is legitimate, no change is needed — record the confirmation in the PR.
Source: yarn.lock:1295 resolved https://registry.yarnpkg.com/@vtex/diagnostics-semconv/-/diagnostics-semconv-5.5.2.tgz; prior entries 0.1.0-beta.11 and 1.1.2

To dismiss: /dk-review dismiss 1e8b4f96-73d5-42a7-b0c8-9a2f6e5d3b18 [reason]

[dk-pr-review]

[Functional.Support] 🟡 RESTRICT

@vtex/diagnostics-semconv jumps four major versions (^1.1.2 → 5.5.2) in a single change whose stated purpose is only to obtain two new cluster constants. Semantic-convention packages routinely rename or remove attribute constants across majors, and this repo consumes five pre-existing exports (ATTR_VTEX_ACCOUNT_NAME, ATTR_VTEX_IO_WORKSPACE_NAME/TYPE, ATTR_VTEX_IO_APP_ID, ATTR_VTEX_IO_APP_AUTHOR_TYPE) whose key values could silently change without breaking compilation — which would rename dimensions on already-published dashboards and alerts. goldenPathGetMigrationGuide returned no guide for this transition, so no migration path was validated. Additionally, @vtex/api is a published library and this is a runtime dependency: an exact pin (no caret, unlike every other @vtex/* entry except diagnostics-nodejs) can force duplicate installs for consumers that depend on the same package.

Action: Document in the PR (or design.md) the diff of attribute values between 1.1.2 and 5.5.2 for the five constants already in use, confirming none of the emitted keys changed. Confirm the exact pin is deliberate given the package is a transitive runtime dependency of @vtex/api.

To dismiss: /dk-review dismiss b7f3c1a2-9d4e-4c8b-a5f1-6e2d0b93c714 [reason]

[dk-pr-review]

[Functional.Check] 🟡 RESTRICT

The new assertions expect(AttributeKeys.VTEX_IO_CLUSTER_ID).toBe('vtex_io.cluster.id') validate the manual mock, not the real package. __mocks__/@vtex/diagnostics-semconv.ts sits at the project root and mocks a node_modules module, so Jest applies it automatically to every test file without any jest.mock() call (jest config in package.json sets no roots/modulePaths that would exclude it). These assertions therefore pass by construction from the two lines added to the mock and prove nothing about @vtex/diagnostics-semconv@5.5.2. This is the exact claim tasks.md 4.1/4.2 says was verified, so the suite gives false confidence: if 5.5.2 exported a different key string (e.g. vtex_io.k8s.cluster.id), tests stay green and production emits the wrong dimension name.

Action: Add one assertion that imports the real package (bypassing the manual mock via jest.unmock('@vtex/diagnostics-semconv') or jest.requireActual) and checks that ATTR_VTEX_IO_CLUSTER_ID/ATTR_VTEX_IO_CLUSTER_ROLE exist and equal the expected keys — otherwise the mock and the dependency can drift undetected.

To dismiss: /dk-review dismiss 3a1e8f47-2c60-4b19-8d73-f0c9a5e21b8d [reason]

[dk-pr-review]

[Functional.Support] 🟡 RESTRICT

The diff imports the new symbols ATTR_VTEX_IO_CLUSTER_ID and ATTR_VTEX_IO_CLUSTER_ROLE from @vtex/diagnostics-semconv, but package.json still declares "@vtex/diagnostics-semconv": "^1.1.2" and is not bumped in this PR. If these attributes were introduced after 1.1.2, any consumer (or a fresh install without the updated lockfile) resolving 1.1.2 gets undefined for both keys, producing resource attributes keyed by undefined instead of a compile/install failure.

Action: Raise the minimum version of @vtex/diagnostics-semconv in package.json to the release that first exports ATTR_VTEX_IO_CLUSTER_ID/ATTR_VTEX_IO_CLUSTER_ROLE (and update yarn.lock in the same PR). If 1.1.2 already exports them, no change is needed — please confirm explicitly.

To dismiss: /dk-review dismiss 5c7e1a80-9f42-4b36-a0d8-72e4c6913bfa [reason]

[dk-pr-review]

[Evolvability.SupportedByLanguage] 🔵 SUGGEST

CLUSTER_ID/CLUSTER_ROLE are declared as process.env.VTEX_CLUSTER_ID as string, asserting a non-optional string while the values are genuinely optional at runtime (the new getClusterResourceAttributes(clusterId?: string, clusterRole?: string) signature and its 'metadata unavailable' test both acknowledge this). The cast hides the undefined case from every other consumer of these exports.

Action: Type them honestly as export const CLUSTER_ID: string | undefined = process.env.VTEX_CLUSTER_ID (same for CLUSTER_ROLE), so the compiler forces callers to handle absence rather than relying on getClusterResourceAttributes being the only reader.

To dismiss: /dk-review dismiss e4906b23-8d15-4c7a-b2f6-3a81d0e5947c [reason]

[dk-pr-review]

[quality.new-logic-enforcement] 🔵 SUGGEST

The new test file is the only coverage for client.ts, but it hardcodes DIAGNOSTICS_TELEMETRY_ENABLED: false inside the jest.mock factory, so the enabled branch (instrumentation registration, metricsClient.provider(), KoaInstrumentation, HostMetricsInstrumentation) and the catch path can never be exercised. Coverage for initializeTelemetryClients is therefore structurally capped at the disabled path.

Action: Use jest.doMock/jest.isolateModules (or read the flag through a getter) so a third case can set DIAGNOSTICS_TELEMETRY_ENABLED: true and assert registerInstrumentations is called with the host-metrics instrumentation, plus a case where NewTelemetryClient rejects and the error is rethrown.

To dismiss: /dk-review dismiss a2d738f5-6014-4e9b-8c3d-95b7e2f10a68 [reason]

[dk-pr-review]

[quality.new-logic-enforcement] 🔵 SUGGEST

expect(CLUSTER_ID).toBe(process.env.VTEX_CLUSTER_ID as string) compares the constant against the very expression that produced it, so it passes trivially when VTEX_CLUSTER_ID is unset (both sides are undefined) — which is the normal state in CI. The two added lines raise the line-coverage number for the new constants without verifying any behaviour.

Action: Set the env vars to known values before importing the module (or via jest.resetModules() + process.env.VTEX_CLUSTER_ID = 'cluster-a') and assert the concrete value, so the test fails if the constant is wired to the wrong env var name.

To dismiss: /dk-review dismiss 7b3f0c96-4a28-4d51-9e7c-08f6b1d3a542 [reason]

[dk-pr-review]

[Evolvability.Textual] 🔵 SUGGEST

The test is named 'shares configured cluster resource attributes across metrics and logs', but its assertions verify the opposite shape: that newMetricsClient and newLogsClient are called WITHOUT additionalAttrs. The actual invariant being tested is that cluster attributes are set once at the NewTelemetryClient resource level and deliberately not repeated per-signal; the name misleads a future reader into thinking per-client attributes are expected.

Action: Rename to something like 'sets cluster resource attributes once on the telemetry resource, not per signal client', so the assertion block matches the stated intent.

To dismiss: /dk-review dismiss c60a95e7-31b8-4f2d-8a74-b9e0c47d1f83 [reason]

[sonar-workflows]

Quality Gate passed

Issues

• 0 New Issues
• 0 Fixed Issues
• 0 Accepted Issues

Measures

• 0 Security Hotspots
• 93.10% Coverage (31.20% Estimated after merge)
• 0.00% Duplicated Code (0.90% Estimated after merge)

Project ID: node-vtex-api

View in SonarQube

[dk-pr-review]

DK Review — Audit Summary

Verdict: ⚠️ RESTRICTED

Severity	Count
BLOCK	0
RESTRICT	3
SUGGEST	13

Scenarios evaluated: dependency-governance, general-review, quality-ratchet
Scenarios skipped: pipeline-config (no matching files), agent-skills-review (no matching files)

---

📋 Findings (16)

RESTRICT

1. [Functional.Support] package.json:57 — @vtex/diagnostics-semconv jumps from ^1.1.2 to 5.5.2, skipping majors 2.x, 3.x and 4.x, while its companion SDK @vtex/diagnostics-nodejs stays pinned at 0.1.8-io (unchanged in this diff). Semantic-convention packages rename, relocate and remove attribute constants across majors, so a 4-major leap paired with an unchanged 0.x consumer SDK is a concrete compatibility risk: telemetry attributes emitted by this library may silently diverge from what the diagnostics runtime expects, or fail to compile against the new constant surface. goldenPathGetMigrationGuide(@vtex/diagnostics-semconv, 1.1.2 -> 5.5.2) returned no published migration path, so there is no VTEX-sanctioned upgrade procedure backing this bump.
   → Confirm that @vtex/diagnostics-nodejs@0.1.8-io is built against semconv 5.x (check its peer/direct dependency range) and bump it together with semconv if not. Document the breaking changes absorbed by this jump in the PR description, and verify that every semconv constant referenced in src/ still resolves under 5.5.2 — a successful tsc build plus an assertion on the emitted attribute keys is the minimum evidence.
2. [Functional.Support] package.json:57 — @vtex/diagnostics-semconv jumps four major versions (^1.1.2 → 5.5.2) solely to obtain the two new cluster constants, but nothing in the diff verifies that the already-consumed constants (ATTR_VTEX_ACCOUNT_NAME, ATTR_VTEX_IO_WORKSPACE_NAME, ATTR_VTEX_IO_WORKSPACE_TYPE, ATTR_VTEX_IO_APP_ID, ATTR_VTEX_IO_APP_AUTHOR_TYPE) still resolve to the same attribute keys. A semantic-convention rename across 2.x–5.x would silently rename production telemetry dimensions and break existing dashboards and alerts, with no compile-time or test-time signal because those constants are string values, not types.
   → Diff the exported key values between 1.1.2 and 5.5.2 (or read the package changelog) and record the result in design.md; add an assertion suite that imports the real package (bypassing the local __mocks__ entry, e.g. via jest.unmock/requireActual) and pins each consumed ATTR_* value.
3. [Evolvability.SolutionApproach] __mocks__/@vtex/diagnostics-semconv.ts:8 — The root-level __mocks__/@vtex/diagnostics-semconv.ts is automatically applied by Jest for this node_modules package (the repo's jest config sets no roots/modulePaths that would exclude it), so no test ever loads the real 5.5.2 package. Hand-adding ATTR_VTEX_IO_CLUSTER_ID/ATTR_VTEX_IO_CLUSTER_ROLE here makes src/constants.test.ts's expect(AttributeKeys.VTEX_IO_CLUSTER_ID).toBe('vtex_io.cluster.id') assert against this file, not against the dependency — the suite would stay green even if the published package never exported these constants, or exported different key strings.
   → Keep one test that uses jest.requireActual('@vtex/diagnostics-semconv') to assert the real exported key values, so the mock cannot mask a missing or renamed constant in the bumped dependency.

SUGGEST

1. [dep-gov.version-consistency] package.json:57 — The dependency is converted from a caret range (^1.1.2) to an exact pin (5.5.2), which deviates from the range style used by every other dependency in this manifest. Because @vtex/api is itself a published library, an exact pin in dependencies propagates to all consumers: any consumer that resolves a different 5.x of @vtex/diagnostics-semconv will end up with two copies of the package in its tree, and consumers can no longer pick up patch or security releases of semconv without a new @vtex/api release.
   → Use ^5.5.2 in dependencies so consumers can deduplicate and receive patch releases, and rely on yarn.lock to pin the exact version for this repo's own builds. If the exact pin is deliberate (e.g. a known-bad 5.x release), add a comment or resolutions entry recording why.
2. [dep-gov.version-consistency] yarn.lock:1295 — After this change the lockfile still resolves two disjoint versions of @vtex/diagnostics-semconv: the pre-existing 0.1.0-beta.11 entry (visible immediately above the changed hunk) and the new 5.5.2. The bump widens the gap between them from ~1 major to ~5, so the two copies now almost certainly export different attribute-constant sets. Whichever transitive dependency still pulls 0.1.0-beta.11 will emit telemetry under the old conventions while this package emits the 5.x conventions, producing inconsistent attribute names in the same trace.
   → Identify the dependent that requires @vtex/diagnostics-semconv@0.1.0-beta.11 (yarn why @vtex/diagnostics-semconv) and either upgrade it or add a resolutions entry aligning the whole tree on a single semconv major, alongside the existing @grpc/grpc-js resolution.
3. [general.error-handling] src/service/telemetry/client.ts:104 — When VTEX_CLUSTER_ID/VTEX_CLUSTER_ROLE are absent or whitespace-only in a production runtime, the dimensions are dropped completely silently. The failure mode is indistinguishable from a working deployment until someone notices missing cluster labels on dashboards, which is exactly the incident-isolation use case the change is meant to enable.
   → Emit a one-time console.warn (or a diagnostics log) at initialization when DIAGNOSTICS_TELEMETRY_ENABLED && PRODUCTION and either cluster attribute resolves to undefined, naming the missing environment variable.
4. [Evolvability.SupportedByLanguage] src/constants.ts:172 — CLUSTER_ID/CLUSTER_ROLE are cast with as string although the whole feature is built on these values being optional (getClusterResourceAttributes accepts string | undefined, the spec requires independent omission). The cast asserts a guarantee the runtime does not provide, so any future consumer that does CLUSTER_ID.toLowerCase() type-checks cleanly and throws at runtime outside Kubernetes.
   → Type these as string | undefined (drop the as string), matching the optional parameters of getClusterResourceAttributes; the existing REGION cast is prior art but not a reason to extend the pattern to genuinely optional values.
5. [Functional.Check] src/constants.test.ts:227 — expect(CLUSTER_ID).toBe(process.env.VTEX_CLUSTER_ID as string) is tautological: in CI neither variable is set, so the assertion reduces to expect(undefined).toBe(undefined) and passes regardless of whether constants.ts reads the correct variable name (a typo such as VTEX_CLUSTERID would still pass).
   → Set process.env.VTEX_CLUSTER_ID/VTEX_CLUSTER_ROLE to fixed values and re-require the module (jest.isolateModules), then assert the concrete expected strings.
6. [Functional.Check] src/service/telemetry/client.test.ts:38 — The suite mocks DIAGNOSTICS_TELEMETRY_ENABLED: false, so noop: true is passed to NewTelemetryClient and the enabled branch (instrumentation registration, host metrics) is never exercised — while tasks.md 3.2 claims the tests prove diagnostics metrics and logs share the cluster dimensions. With NewTelemetryClient, both exporter factories and getClusterResourceAttributes all mocked, the test only asserts the shape of an object literal, not that any emitted metric or log carries the resource attributes.
   → Add a case with DIAGNOSTICS_TELEMETRY_ENABLED: true and stop mocking ./resourceAttributes in at least one case, so the real mapping and the enabled initialization path are covered; or soften the claim in tasks.md 3.2 to match what the tests actually verify.
7. [general.broken-references] openspec/changes/add-k8s-cluster-telemetry-dimensions/design.md:42 — The design cites vtex/diagnostics#174 as the source of the vtex_io.cluster.id/vtex_io.cluster.role constants, and claims 5.5.2 is "the first public version ... that contains both cluster constants". Neither the PR reference nor the version claim can be confirmed from this diff.
   → Please double-check that vtex/diagnostics#174 is the correct reference (a full URL would be more durable than a bare number) and that 5.5.2 is indeed the first version exporting both constants.
8. [general.broken-references] openspec/changes/add-k8s-cluster-telemetry-dimensions/proposal.md:25 — The Impact section references "the cluster attributes introduced by vtex/diagnostics#174"; this external reference cannot be verified from the diff.
   → Confirm the issue/PR number resolves and consider linking the full URL so readers of the archived change can follow it.
9. [general.documentation-gap] openspec/changes/add-k8s-cluster-telemetry-dimensions/proposal.md:27 — The change establishes a new deployment/runtime contract (VTEX_CLUSTER_ID, VTEX_CLUSTER_ROLE must be injected by the platform) but documents it only inside an openspec change folder, which is a point-in-time proposal that gets archived. Operators and platform maintainers looking for the environment variables this library consumes have no durable place to find them, and the newly exported getClusterResourceAttributes has no doc comment explaining the omit-on-empty semantics.
   → Document the two environment variables in the repository's long-lived docs (README or the telemetry docs) alongside VTEX_REGION, and add a short TSDoc comment on getClusterResourceAttributes stating that empty/whitespace values are omitted independently.
10. [Functional.Support] src/constants.ts:9 — The new imports ATTR_VTEX_IO_CLUSTER_ID / ATTR_VTEX_IO_CLUSTER_ROLE come from @vtex/diagnostics-semconv, which is declared in package.json as ^1.1.2. If these symbols were added in a version newer than 1.1.2 and the dependency floor was not raised in this PR, a consumer of @vtex/api resolving 1.1.2 gets undefined for both symbols. Because AttributeKeys.VTEX_IO_CLUSTER_ID is then used as a computed object key in resourceAttributes.ts, the failure is silent: the emitted resource attribute key becomes the literal string "undefined" instead of throwing.
    → Raise the @vtex/diagnostics-semconv range floor to the first version that actually exports ATTR_VTEX_IO_CLUSTER_ID and ATTR_VTEX_IO_CLUSTER_ROLE (e.g. ^1.2.0), so the requirement is enforced at install time rather than degrading silently at runtime.
11. [quality.new-logic-enforcement] src/constants.test.ts:227 — The new assertions expect(CLUSTER_ID).toBe(process.env.VTEX_CLUSTER_ID as string) and the CLUSTER_ROLE equivalent are tautological: both sides read the same environment variable, so when VTEX_CLUSTER_ID / VTEX_CLUSTER_ROLE are unset (the normal case in CI) both sides are undefined and the test passes without verifying anything. This raises line coverage for the two new constants without adding real verification, which weakens the coverage ratchet rather than satisfying it.
    → Set the env vars explicitly before re-importing the module (e.g. process.env.VTEX_CLUSTER_ID = 'cluster-a' + jest.resetModules()) and assert against the literal expected value, so the test fails if the constant is wired to the wrong variable name.
12. [Evolvability.SupportedByLanguage] src/constants.ts:173 — export const CLUSTER_ID = process.env.VTEX_CLUSTER_ID as string asserts a non-optional string for a value the PR itself treats as optional — getClusterResourceAttributes(clusterId?: string, ...) accepts undefined and resourceAttributes.test.ts explicitly covers the missing/empty cases. The cast hides the real nullability from every other consumer of this constant, so a future caller doing CLUSTER_ID.toLowerCase() compiles cleanly and throws at runtime.
    → Type it as export const CLUSTER_ID = process.env.VTEX_CLUSTER_ID (inferred string | undefined) so the optionality the new code already handles is visible to the type checker.
13. [Evolvability.Organizational] src/service/telemetry/client.ts:104 — When VTEX_CLUSTER_ID / VTEX_CLUSTER_ROLE are absent, getClusterResourceAttributes returns {} and the cluster dimensions are dropped from every trace, metric, and log with no signal anywhere. Since telemetry is already emitting a startup line here (Telemetry enabled for app: ...), an operator debugging why cluster-scoped dashboards are empty has no way to distinguish 'env var not injected' from 'collector dropped the attribute'.
    → Inside the DIAGNOSTICS_TELEMETRY_ENABLED branch, emit a one-time warning when the resolved cluster attribute object is empty, naming the missing env vars.

---

DK Review v1.0.0 | To dismiss a finding: reply /dk-review dismiss <finding-id> [reason]