Add telemetry-to-caas kind for CaaS (Collector as a Service) support - #436
Add telemetry-to-caas kind for CaaS (Collector as a Service) support#436vkozyura wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
The PR is generally well-structured, but has one logic bug: when only a gRPC OTLP endpoint is present in the CaaS binding, credentials.url is set to undefined (the falsy http value), silently breaking the exporter. Please address the flagged issues before merging.
PR Bot Information
Version: 1.26.5
- Event Trigger:
pull_request.opened - LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
be7942dc-b11a-457c-a83d-2aafac5eb2c3 - File Content Strategy: Full file content
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
already solved on develop: telemetry/lib/logging/index.js Line 163 in 48c9934 |
DONE
|
moved to be required-on-demand as in the instruction |
- Add telemetry-to-caas kind definition in package.json
- Add ZTI (Zero Trust Identity) support with SPIRE SVID certificate handling
- Add RotatingCertAgent for automatic certificate rotation via cds.emit('svid')
- Add wrapExporterWithBuffer for buffering until certs available
- Add createCaaSExporter helper for consistent CaaS exporter creation
- Handle CaaS in tracing, metrics, and logging exporters
- Add comprehensive tests for CaaS functionality
Rebased onto develop branch with:
- vitest migration (tests updated from jest to vitest)
- logProcessorArg() preserved for sdk-logs 0.221 compatibility
- Re-entrancy guard preserved for logging (#482)
Note: CaaS requires mTLS authentication with SAP-signed certificates.
The certificate must be obtained via BTP Certificate Service or ZTI.
When CDS_REQUIRES_TELEMETRY_USE_ZTI is true (or not set), certsAvailable() now loads from SVID files and ignores any static x509 env vars. Static x509 certs are only used when USE_ZTI is explicitly set to 'false'. This prevents stale/expired static certs from taking precedence over fresh ZTI SVID certificates.
Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
| ``` | ||
|
|
||
|
|
||
| ### Span Processor |
| - name: my-zti-instance | ||
| ``` | ||
|
|
||
| 2. **Bind CaaS service** to your app: |
There was a problem hiding this comment.
this step is needed with or without zti
| - name: my-caas-instance | ||
| ``` | ||
|
|
||
| 3. **After first deployment**, extract the certificate identity from the SVID and rebind CaaS with it: |
| "logging": { | ||
| "exporter": { | ||
| "module": "@opentelemetry/exporter-logs-otlp-proto", | ||
| "class": "OTLPLogExporter" | ||
| } | ||
| } |
There was a problem hiding this comment.
| "logging": { | |
| "exporter": { | |
| "module": "@opentelemetry/exporter-logs-otlp-proto", | |
| "class": "OTLPLogExporter" | |
| } | |
| } |
no logging by default
| SEMRESATTRS_SERVICE_INSTANCE_ID: ATTR_SERVICE_INSTANCE_ID | ||
| } = require('@opentelemetry/semantic-conventions') | ||
|
|
||
| const MAX_BUFFER_SIZE = 10 |
There was a problem hiding this comment.
| const MAX_BUFFER_SIZE = 10 | |
| const MAX_BUFFER_SIZE = Number(process.env.TELEMETRY_MAX_BUFFER_SIZE ?? 10) |
| return null | ||
| } | ||
|
|
||
| const svidDir = process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR || SVID_DIR |
There was a problem hiding this comment.
| const svidDir = process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR || SVID_DIR | |
| const svidDir = process.env.TELEMETRY_ZTI_DIR || SVID_DIR |
| vi.resetModules() | ||
| // Clear env vars that affect ZTI detection | ||
| delete process.env.VCAP_SERVICES | ||
| delete process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR |
There was a problem hiding this comment.
| delete process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR | |
| delete process.env.TELEMETRY_ZTI_DIR |
| delete process.env.VCAP_SERVICES | ||
| delete process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR | ||
| // Disable ZTI to test static x509 cert path | ||
| process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI = 'false' |
There was a problem hiding this comment.
| process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI = 'false' | |
| process.env.TELEMETRY_USE_ZTI = 'false' |
| }) | ||
|
|
||
| afterEach(() => { | ||
| delete process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI |
There was a problem hiding this comment.
| delete process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI | |
| delete process.env.TELEMETRY_USE_ZTI |
| } | ||
|
|
||
| function isZTIEnabled() { | ||
| return process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI !== 'false' |
There was a problem hiding this comment.
| return process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI !== 'false' | |
| return process.env.TELEMETRY_USE_ZTI !== 'false' |
| parameters: | ||
| config: | ||
| app-identifier: my-app | ||
| svid-store: | ||
| file: | ||
| name: caas-svid |
There was a problem hiding this comment.
do we really need to provide this? aren't there defaults?
| To explicitly disable ZTI (e.g., for testing), set: | ||
| ```bash | ||
| CDS_REQUIRES_TELEMETRY_USE_ZTI=false | ||
| ``` | ||
|
|
There was a problem hiding this comment.
| To explicitly disable ZTI (e.g., for testing), set: | |
| ```bash | |
| CDS_REQUIRES_TELEMETRY_USE_ZTI=false | |
| ``` |
don't document
Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
Note: CaaS requires mTLS authentication with SAP-signed certificates. The certificate must be obtained separately via BTP Certificate Service.