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 9 commits into
Conversation
- 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.
SummaryThe following content is AI-generated and provides a summary of the pull request: Add
|
![hyperspace-insights[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
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
| } | ||
|
|
||
| // use http endpoint by default (proto exporter) | ||
| credentials.url = credentials.otlp.http |
There was a problem hiding this comment.
Logic Error: When credentials.otlp.http is falsy but credentials.otlp.grpc exists, credentials.url is set to undefined (the falsy http value), silently misconfiguring the exporter. The URL should fall back to the gRPC endpoint when http is absent.
| credentials.url = credentials.otlp.http | |
| credentials.url = credentials.otlp.http ?? credentials.otlp.grpc | |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| // The certificate must be provided separately via BTP Certificate Service | ||
| // For now, we just set the URL and log a warning about mTLS | ||
| LOG._warn && LOG.warn('CaaS requires mTLS authentication. Make sure the SAP-signed certificate is configured.') | ||
| } |
There was a problem hiding this comment.
Bug: augmentCaaSCreds mutates the original credentials object from the VCAP binding (sets _augmented, url) as a side effect. If the credentials object is reused elsewhere, or getCredsForCaaS() is called again (e.g., both tracing and metrics paths), the first call marks _augmented = true and the second call skips re-augmentation - which is fine, but the mutation of a shared object can be surprising. Returning a new augmented object (like augmentCLCreds does) or at minimum being consistent with the other augment pattern is safer.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This file should not be committed to the feature branch.
- Pass httpAgentOptions to OTLP exporter config as agentOptions - Add debug logging to verify cert is being passed - Remove monkey-patching of https.request (didn't work due to OTEL instrumentation)
The OTLP HTTP exporter expects 'httpAgentOptions' in the config, not 'agentOptions'. This was causing the cert/key to be ignored.
When config.url is provided, OTLP exporter uses it as-is without appending the signal resource path. CaaS binding provides base URL without path, so we need to append /v1/traces and /v1/metrics.
2 participants
Note: CaaS requires mTLS authentication with SAP-signed certificates. The certificate must be obtained separately via BTP Certificate Service.