-
Notifications
You must be signed in to change notification settings - Fork 15
feat(telemetry): add cluster dimensions to diagnostics #669
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
Changes from all commits
fa6b5fc
d36aa22
22abf39
5726f4b
ef185ec
8bf20ef
820feda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-07-23 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| ## Context | ||
|
|
||
| The diagnostics telemetry stack is initialized once in `src/service/telemetry/client.ts`. `NewTelemetryClient` receives `additionalAttrs`, which become resource attributes shared by the metrics, logs, and traces clients created from that telemetry client. Today those attributes identify the IO app, vendor, version, and workspace, while signal-specific code adds request and operation dimensions. | ||
|
|
||
| Cluster metadata is deployment-level rather than request-level. Adding it independently at every metric and log call site would duplicate logic, miss automatic or future instrumentation, and consume the custom metric attribute budget enforced by `DiagnosticsMetrics`. | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
|
|
||
| - Export the authoritative Kubernetes cluster identifier/name as `vtex_io.cluster.id`. | ||
| - Export the cluster's platform role as `vtex_io.cluster.role`. | ||
| - Make both dimensions available on every diagnostics metric and log, including automatically instrumented telemetry. | ||
| - Preserve current behavior when either metadata value is unavailable. | ||
| - Keep cluster metadata low-cardinality and constant for the process lifetime. | ||
|
|
||
| **Non-Goals:** | ||
|
|
||
| - Deriving cluster identity from hostnames, pod names, request headers, or network calls. | ||
| - Adding pod, node, namespace, or workload dimensions. | ||
| - Renaming existing metrics, changing log messages, or modifying legacy console-based telemetry. | ||
| - Requiring cluster metadata when running locally or outside Kubernetes. | ||
|
|
||
| ## Decisions | ||
|
|
||
| ### Add cluster metadata as telemetry resource attributes | ||
|
|
||
| The implementation will extend the `additionalAttrs` passed to `NewTelemetryClient` with non-empty `vtex_io.cluster.id` and `vtex_io.cluster.role` values. This is the single initialization point shared by diagnostics metrics and logs and covers direct instruments, the `DiagnosticsMetrics` wrapper, host metrics, and logger calls. | ||
|
|
||
| Per-call instrumentation was rejected because the repository has several independent metric paths and automatic instrumentation. Updating every call site would be error-prone and would count the values against the seven custom attributes accepted by `DiagnosticsMetrics`. | ||
|
|
||
| Because the diagnostics clients share one OpenTelemetry resource, traces may also carry these attributes. That consistency is accepted as a side effect; metrics and logs are the required signals. | ||
|
|
||
| ### Treat runtime metadata as authoritative configuration | ||
|
|
||
| The cluster identifier/name will be read from `process.env.VTEX_CLUSTER_ID`, and the cluster role will be read from `process.env.VTEX_CLUSTER_ROLE`. Both values will be normalized by trimming whitespace. Empty, whitespace-only, or undefined values will be omitted independently. `VTEX_REGION` will not be used as a fallback because region and cluster identity have different semantics. | ||
|
|
||
| Fallback strings such as `unknown` were rejected because they create an artificial cluster that combines unrelated local or misconfigured workloads. | ||
|
|
||
| ### Use generated VTEX IO semantic-convention keys | ||
|
|
||
| `AttributeKeys.VTEX_IO_CLUSTER_ID` and `AttributeKeys.VTEX_IO_CLUSTER_ROLE` will reference `ATTR_VTEX_IO_CLUSTER_ID` and `ATTR_VTEX_IO_CLUSTER_ROLE` from `@vtex/diagnostics-semconv`, following the existing workspace and app attribute pattern. These generated constants resolve to the stable `vtex_io.cluster.id` and `vtex_io.cluster.role` keys introduced by `vtex/diagnostics#174`. | ||
|
|
||
| Hard-coded local attribute names were rejected because they would duplicate the semantic-convention package and could drift from the cross-language contract. | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **A runtime does not inject the cluster variables** → Treat both variables as optional and add tests around the resulting constants. | ||
| - **Resource attributes also appear on traces** → Accept this because the telemetry client shares a resource and consistent deployment identity is useful across signals. | ||
| - **Additional dimensions increase metric series count** → Cluster identity and role are bounded deployment metadata; do not add pod- or request-level values. | ||
| - **A deployment omits one value** → Emit the available dimension independently and omit only the missing one. | ||
| - **The semantic-convention upgrade introduces a major-version change** → Pin version `5.5.2` and validate the full build, lint, and test suites. | ||
| - **A caller emits a data-point attribute with the same key** → Treat `vtex_io.cluster.id` and `vtex_io.cluster.role` as platform resource dimensions and test the diagnostics payload shape at initialization. | ||
|
|
||
| ## Migration Plan | ||
|
|
||
| 1. Update `@vtex/diagnostics-semconv` to `5.5.2`, the first public version adopted here that contains both cluster constants. | ||
| 2. Validate that build, lint, and tests pass with the published package. | ||
| 3. Deploy without changing existing metric names or log schemas beyond the two optional dimensions. | ||
| 4. Verify emitted metrics and logs in one development cluster before broad rollout. | ||
| 5. Roll back by reverting the resource attributes; no stored-data migration is required. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| ## Why | ||
|
|
||
| Metrics and logs emitted through the diagnostics API cannot currently be grouped or filtered by the Kubernetes cluster that produced them. Adding stable cluster identity and role dimensions will make cross-cluster comparison, incident isolation, and dashboard segmentation possible. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - Read the Kubernetes cluster identifier/name from `VTEX_CLUSTER_ID` and cluster role from `VTEX_CLUSTER_ROLE`. | ||
| - Attach the values as the stable `vtex_io.cluster.id` and `vtex_io.cluster.role` semantic-convention dimensions from `@vtex/diagnostics-semconv`. | ||
| - Apply the dimensions centrally so built-in and application-provided telemetry receive the same metadata. | ||
| - Omit unavailable or empty cluster metadata rather than emitting misleading placeholder values. | ||
| - Add coverage for configured and missing cluster metadata. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
| - `cluster-telemetry-dimensions`: Defines how Kubernetes cluster identity and role are represented on diagnostics metrics and logs. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| None. | ||
|
|
||
| ## Impact | ||
|
|
||
| - Telemetry initialization in `src/service/telemetry/client.ts`. | ||
| - Diagnostics metric and log tests, constants for `VTEX_CLUSTER_ID` and `VTEX_CLUSTER_ROLE`, and adoption of the cluster attributes introduced by `vtex/diagnostics#174`. | ||
| - Metrics and logs gain two low-cardinality dimensions; dashboards and queries can adopt them without changing metric names or log messages. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [general.broken-references] 🔵 SUGGEST Second occurrence of the unverifiable Action: Verify the reference alongside the one in design.md; if the issue/PR number is wrong, both files need correcting. To dismiss: |
||
| - The deployment/runtime contract exposes the authoritative Kubernetes cluster identifier/name and role through `VTEX_CLUSTER_ID` and `VTEX_CLUSTER_ROLE`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: Diagnostics metrics include Kubernetes cluster dimensions | ||
| The system SHALL attach configured Kubernetes cluster metadata to every metric emitted through the diagnostics telemetry client. The Kubernetes cluster identifier or name SHALL be exported as `vtex_io.cluster.id`, and the cluster role SHALL be exported as `vtex_io.cluster.role`. | ||
|
|
||
| #### Scenario: Both cluster values are configured for metrics | ||
| - **WHEN** diagnostics telemetry initializes with non-empty cluster identifier and cluster role metadata | ||
| - **THEN** every diagnostics metric has `vtex_io.cluster.id` and `vtex_io.cluster.role` resource dimensions containing the configured values | ||
|
|
||
| #### Scenario: Automatically instrumented metric is emitted | ||
| - **WHEN** an automatic or built-in diagnostics metric is emitted after telemetry initialization | ||
| - **THEN** the metric has the same `vtex_io.cluster.id` and `vtex_io.cluster.role` resource dimensions as an application-provided metric | ||
|
|
||
| ### Requirement: Diagnostics logs include Kubernetes cluster dimensions | ||
| The system SHALL attach configured Kubernetes cluster metadata to every log emitted through the diagnostics telemetry client, using `vtex_io.cluster.id` for the Kubernetes cluster identifier or name and `vtex_io.cluster.role` for the cluster role. | ||
|
|
||
| #### Scenario: Both cluster values are configured for logs | ||
| - **WHEN** the diagnostics logger emits a log after telemetry initializes with non-empty cluster identifier and cluster role metadata | ||
| - **THEN** the log has `vtex_io.cluster.id` and `vtex_io.cluster.role` resource dimensions containing the configured values | ||
|
|
||
| #### Scenario: Logs and metrics use consistent cluster values | ||
| - **WHEN** a diagnostics metric and diagnostics log are emitted by the same process | ||
| - **THEN** both signals have identical `vtex_io.cluster.id` and `vtex_io.cluster.role` values | ||
|
|
||
| ### Requirement: Missing cluster metadata is omitted safely | ||
| The system SHALL normalize cluster metadata by trimming surrounding whitespace and SHALL omit a cluster dimension when its configured value is undefined, empty, or whitespace-only. Availability of one cluster value SHALL NOT depend on availability of the other. | ||
|
|
||
| #### Scenario: No cluster metadata is configured | ||
| - **WHEN** diagnostics telemetry initializes without a cluster identifier and cluster role | ||
| - **THEN** telemetry initialization succeeds and neither `vtex_io.cluster.id` nor `vtex_io.cluster.role` is emitted | ||
|
|
||
| #### Scenario: Only cluster identifier is configured | ||
| - **WHEN** diagnostics telemetry initializes with a non-empty cluster identifier and no cluster role | ||
| - **THEN** emitted diagnostics metrics and logs include `vtex_io.cluster.id` and omit `vtex_io.cluster.role` | ||
|
|
||
| #### Scenario: Only cluster role is configured | ||
| - **WHEN** diagnostics telemetry initializes with a non-empty cluster role and no cluster identifier | ||
| - **THEN** emitted diagnostics metrics and logs include `vtex_io.cluster.role` and omit `vtex_io.cluster.id` | ||
|
|
||
| #### Scenario: Cluster metadata contains surrounding whitespace | ||
| - **WHEN** configured cluster metadata contains leading or trailing whitespace | ||
| - **THEN** emitted cluster dimensions contain the trimmed values | ||
|
|
||
| ### Requirement: Cluster dimensions use deployment metadata | ||
| The system MUST obtain cluster identity from `process.env.VTEX_CLUSTER_ID` and cluster role from `process.env.VTEX_CLUSTER_ROLE` and SHALL keep those values constant for the lifetime of the telemetry client. The system SHALL NOT use `VTEX_REGION` as a fallback for cluster identity. The attribute keys SHALL come from `ATTR_VTEX_IO_CLUSTER_ID` and `ATTR_VTEX_IO_CLUSTER_ROLE` exported by `@vtex/diagnostics-semconv`. | ||
|
|
||
| #### Scenario: Telemetry client is initialized | ||
| - **WHEN** the process creates its diagnostics telemetry client | ||
| - **THEN** it reads cluster identity from `VTEX_CLUSTER_ID` and cluster role from `VTEX_CLUSTER_ROLE` and applies them as resource attributes | ||
| - **AND** it uses the generated VTEX IO semantic-convention keys rather than locally defined attribute-name strings | ||
|
|
||
| #### Scenario: Request data differs between telemetry calls | ||
| - **WHEN** metrics or logs are emitted for different accounts, workspaces, routes, or operations | ||
| - **THEN** their cluster dimensions remain the deployment-level values selected at telemetry initialization |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| ## 1. Runtime Metadata Configuration | ||
|
|
||
| - [x] 1.1 Add constants that read cluster identifier/name from `VTEX_CLUSTER_ID` and cluster role from `VTEX_CLUSTER_ROLE` | ||
| - [x] 1.2 Expose `ATTR_VTEX_IO_CLUSTER_ID` and `ATTR_VTEX_IO_CLUSTER_ROLE` through `AttributeKeys`, following the existing semantic-convention pattern | ||
|
|
||
| ## 2. Cluster Metadata Configuration | ||
|
|
||
| - [x] 2.1 Add a helper that maps the runtime cluster variables to `vtex_io.cluster.id` and `vtex_io.cluster.role` without using `VTEX_REGION` as a fallback | ||
| - [x] 2.2 Add normalization that trims configured values and omits undefined, empty, or whitespace-only metadata independently | ||
| - [x] 2.3 Add unit tests covering both values, either value alone, missing values, and whitespace normalization | ||
|
|
||
| ## 3. Diagnostics Telemetry Integration | ||
|
|
||
| - [x] 3.1 Add the normalized semantic-convention cluster values conditionally to `NewTelemetryClient` resource attributes | ||
| - [x] 3.2 Add telemetry initialization tests proving diagnostics metrics and logs share `vtex_io.cluster.id` and `vtex_io.cluster.role` without changing metric-call custom attributes | ||
| - [x] 3.3 Verify initialization remains successful when neither cluster value is available | ||
|
|
||
| ## 4. Validation | ||
|
|
||
| - [x] 4.1 Confirm the previous build and focused-test failures were limited to the then-unpublished `ATTR_VTEX_IO_CLUSTER_ID` and `ATTR_VTEX_IO_CLUSTER_ROLE` exports | ||
| - [x] 4.2 Update `@vtex/diagnostics-semconv` to `5.5.2` and run repository linting, type checking, and the broader test suite |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,7 +54,7 @@ | |
| "@types/koa": "^2.11.0", | ||
| "@types/koa-compose": "^3.2.3", | ||
| "@vtex/diagnostics-nodejs": "0.1.8-io", | ||
| "@vtex/diagnostics-semconv": "^1.1.2", | ||
| "@vtex/diagnostics-semconv": "5.5.2", | ||
| "@vtex/node-error-report": "^0.0.3", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Functional.Support] 🟡 RESTRICT
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. To dismiss: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 Action: Verify on the registry that To dismiss: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Functional.Support] 🟡 RESTRICT
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 To dismiss: |
||
| "@wry/equality": "^0.1.9", | ||
| "agentkeepalive": "^4.0.2", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,8 @@ import { | |
| MAX_WORKERS, | ||
| LINKED, | ||
| REGION, | ||
| CLUSTER_ID, | ||
| CLUSTER_ROLE, | ||
| PUBLIC_ENDPOINT, | ||
| APP, | ||
| NODE_ENV, | ||
|
|
@@ -130,11 +132,15 @@ describe('constants', () => { | |
| expect(AttributeKeys).toHaveProperty('VTEX_IO_WORKSPACE_TYPE') | ||
| expect(AttributeKeys).toHaveProperty('VTEX_IO_APP_ID') | ||
| expect(AttributeKeys).toHaveProperty('VTEX_IO_APP_AUTHOR_TYPE') | ||
| expect(AttributeKeys).toHaveProperty('VTEX_IO_CLUSTER_ID') | ||
| expect(AttributeKeys).toHaveProperty('VTEX_IO_CLUSTER_ROLE') | ||
|
|
||
| expect(typeof AttributeKeys.VTEX_IO_WORKSPACE_NAME).toBe('string') | ||
| expect(typeof AttributeKeys.VTEX_IO_WORKSPACE_TYPE).toBe('string') | ||
| expect(typeof AttributeKeys.VTEX_IO_APP_ID).toBe('string') | ||
| expect(typeof AttributeKeys.VTEX_IO_APP_AUTHOR_TYPE).toBe('string') | ||
| expect(AttributeKeys.VTEX_IO_CLUSTER_ID).toBe('vtex_io.cluster.id') | ||
| expect(AttributeKeys.VTEX_IO_CLUSTER_ROLE).toBe('vtex_io.cluster.role') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Functional.Check] 🟡 RESTRICT The new assertions Action: Add one assertion that imports the real package (bypassing the manual mock via To dismiss: |
||
| }) | ||
|
|
||
| test('should have non-empty string values', () => { | ||
|
|
@@ -218,6 +224,8 @@ describe('constants', () => { | |
|
|
||
| test('string environment constants should match their env vars', () => { | ||
| expect(REGION).toBe(process.env.VTEX_REGION as string) | ||
| expect(CLUSTER_ID).toBe(process.env.VTEX_CLUSTER_ID as string) | ||
| expect(CLUSTER_ROLE).toBe(process.env.VTEX_CLUSTER_ROLE as string) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Functional.Check] 🔵 SUGGEST
Action: Set the env vars to known values and re-require the module ( To dismiss: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [quality.new-logic-enforcement] 🔵 SUGGEST
Action: Set the env vars to known values before importing the module (or via To dismiss: |
||
| expect(NODE_ENV).toBe(process.env.NODE_ENV as string) | ||
| expect(ACCOUNT).toBe(process.env.VTEX_ACCOUNT as string) | ||
| expect(WORKSPACE).toBe(process.env.VTEX_WORKSPACE as string) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,9 @@ import { | |
| ATTR_VTEX_IO_WORKSPACE_NAME, | ||
| ATTR_VTEX_IO_WORKSPACE_TYPE, | ||
| ATTR_VTEX_IO_APP_ID, | ||
| ATTR_VTEX_IO_APP_AUTHOR_TYPE | ||
| ATTR_VTEX_IO_APP_AUTHOR_TYPE, | ||
| ATTR_VTEX_IO_CLUSTER_ID, | ||
| ATTR_VTEX_IO_CLUSTER_ROLE, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Functional.Support] 🟡 RESTRICT The diff imports the new symbols Action: Raise the minimum version of To dismiss: |
||
| } from '@vtex/diagnostics-semconv' | ||
|
|
||
| // tslint:disable-next-line | ||
|
|
@@ -62,6 +64,8 @@ export const AttributeKeys = { | |
| VTEX_IO_WORKSPACE_TYPE: ATTR_VTEX_IO_WORKSPACE_TYPE, | ||
| VTEX_IO_APP_ID: ATTR_VTEX_IO_APP_ID, | ||
| VTEX_IO_APP_AUTHOR_TYPE: ATTR_VTEX_IO_APP_AUTHOR_TYPE, | ||
| VTEX_IO_CLUSTER_ID: ATTR_VTEX_IO_CLUSTER_ID, | ||
| VTEX_IO_CLUSTER_ROLE: ATTR_VTEX_IO_CLUSTER_ROLE, | ||
| } | ||
|
|
||
| /** @deprecated Use HeaderKeys.CACHE_CONTROL instead */ | ||
|
|
@@ -165,6 +169,8 @@ export const MAX_WORKERS = 4 | |
|
|
||
| export const LINKED = !!process.env.VTEX_APP_LINK | ||
| export const REGION = process.env.VTEX_REGION as string | ||
| export const CLUSTER_ID = process.env.VTEX_CLUSTER_ID as string | ||
| export const CLUSTER_ROLE = process.env.VTEX_CLUSTER_ROLE as string | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Evolvability.SupportedByLanguage] 🔵 SUGGEST
Action: Type these as To dismiss: |
||
| export const PUBLIC_ENDPOINT = process.env.VTEX_PUBLIC_ENDPOINT || 'myvtex.com' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Evolvability.SupportedByLanguage] 🔵 SUGGEST
Action: Type them honestly as To dismiss: |
||
| export const APP = { | ||
| ID: process.env.VTEX_APP_ID as string, | ||
|
|
||
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.
[general.broken-references] 🔵 SUGGEST
The design cites
vtex/diagnostics#174as the source of thevtex_io.cluster.id/vtex_io.cluster.roleconstants and asserts that5.5.2is "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.Action: Please double-check that
vtex/diagnostics#174is the correct reference and is reachable by readers of this repo, and confirm the 5.5.2 claim against the published changelog.To dismiss:
/dk-review dismiss 5b8ac2f1-3049-4e7a-96d2-8ef07b1a4c65 [reason]