Skip to content

Add telemetry-to-caas kind for CaaS (Collector as a Service) support - #436

Open
vkozyura wants to merge 10 commits into
developfrom
feature/caas-support
Open

Add telemetry-to-caas kind for CaaS (Collector as a Service) support#436
vkozyura wants to merge 10 commits into
developfrom
feature/caas-support

Conversation

@vkozyura

Copy link
Copy Markdown
Contributor
  • Add telemetry-to-caas kind definition in package.json
  • Add getCredsForCaaS() to extract credentials from caas-service binding
  • Add augmentCaaSCreds() to configure OTLP endpoint URL
  • Handle CaaS in tracing and metrics exporters

Note: CaaS requires mTLS authentication with SAP-signed certificates. The certificate must be obtained separately via BTP Certificate Service.

@hyperspace-pr-bot hyperspace-pr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread lib/utils.js Outdated
Comment thread lib/utils.js
Comment thread lib/tracing/index.js Outdated
Comment thread lib/utils.js Fixed
Comment thread lib/utils.js Fixed
Comment thread lib/utils.js Fixed
@sjvans

This comment was marked as outdated.

@sjvans

This comment was marked as outdated.

@sjvans

sjvans commented Sep 3, 2026

Copy link
Copy Markdown
Contributor
  • The BatchLogRecordProcessor({ exporter }) / SimpleLogRecordProcessor({ exporter }) change is correct for @opentelemetry/sdk-logs@0.221 but touches all kinds, not just CaaS — worth splitting into its own PR.

Added @opentelemetry/sdk-logs: >=0.221 as optional peerDependency — npm will warn if users have an older incompatible version, but won't require it if logging isn't used

already solved on develop:

? new BatchLogRecordProcessor(logProcessorArg(exporter))

@vkozyura

vkozyura commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

the SVID watcher should emit an event on cds (e.g., cds.emit('svid', { ... })) that the self-refreshing agent and the exporter can react on.

FS-backed if we want crash durability

crash durability is not the point, minimized memory usage is

for now #436 (comment). we still can discuss emitting an event as future work. bli`?

there's no rush, we can spend the time now.

DONE

  1. Refactored certificate handling to use an event-based approach with a single RotatingCertAgent class for both ZTI and static x509 paths

  2. SVID Watcher watches for file changes, updates cds.env.requires.telemetry.x509, and emits cds.emit('svid', { cert, key })

  3. Unified RotatingCertAgent — both paths use the same agent class that:

  • Sources initial certs from cds.env.requires.telemetry.x509
  • Listens for cds.on('svid') events to rotate certificates
  • For ZTI: rotation is automatic (SVID watcher emits events)
  • For static x509: rotation is possible if user emits cds.emit('svid') after updating cds.env. Otherwise app should restart

@vkozyura

vkozyura commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

"zti.js only extends https.Agent to hold certificates — it doesn't make any HTTP requests itself

it may or may not make a difference, still we should follow the instruction

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.
Comment thread README.md Outdated
Comment thread README.md Outdated
vkozyura and others added 4 commits September 7, 2026 15:57
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>
Comment thread README.md Outdated
Comment thread README.md
```


### Span Processor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't delete this please ;)

Comment thread README.md Outdated
- name: my-zti-instance
```

2. **Bind CaaS service** to your app:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this step is needed with or without zti

Comment thread README.md Outdated
- name: my-caas-instance
```

3. **After first deployment**, extract the certificate identity from the SVID and rebind CaaS with it:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really? that's a no go!

Comment thread package.json
Comment on lines +197 to +202
"logging": {
"exporter": {
"module": "@opentelemetry/exporter-logs-otlp-proto",
"class": "OTLPLogExporter"
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"logging": {
"exporter": {
"module": "@opentelemetry/exporter-logs-otlp-proto",
"class": "OTLPLogExporter"
}
}

no logging by default

Comment thread lib/utils.js
SEMRESATTRS_SERVICE_INSTANCE_ID: ATTR_SERVICE_INSTANCE_ID
} = require('@opentelemetry/semantic-conventions')

const MAX_BUFFER_SIZE = 10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const MAX_BUFFER_SIZE = 10
const MAX_BUFFER_SIZE = Number(process.env.TELEMETRY_MAX_BUFFER_SIZE ?? 10)

Comment thread lib/zti.js
return null
}

const svidDir = process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR || SVID_DIR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const svidDir = process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR || SVID_DIR
const svidDir = process.env.TELEMETRY_ZTI_DIR || SVID_DIR

Comment thread test/buffer.test.js
vi.resetModules()
// Clear env vars that affect ZTI detection
delete process.env.VCAP_SERVICES
delete process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
delete process.env.CDS_REQUIRES_TELEMETRY_ZTI_DIR
delete process.env.TELEMETRY_ZTI_DIR

Comment thread test/buffer.test.js
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'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI = 'false'
process.env.TELEMETRY_USE_ZTI = 'false'

Comment thread test/buffer.test.js
})

afterEach(() => {
delete process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
delete process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI
delete process.env.TELEMETRY_USE_ZTI

Comment thread lib/zti.js
}

function isZTIEnabled() {
return process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI !== 'false'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return process.env.CDS_REQUIRES_TELEMETRY_USE_ZTI !== 'false'
return process.env.TELEMETRY_USE_ZTI !== 'false'

Comment thread README.md
Comment on lines +374 to +379
parameters:
config:
app-identifier: my-app
svid-store:
file:
name: caas-svid

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need to provide this? aren't there defaults?

Comment thread README.md
Comment on lines +407 to +411
To explicitly disable ZTI (e.g., for testing), set:
```bash
CDS_REQUIRES_TELEMETRY_USE_ZTI=false
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants