-
Notifications
You must be signed in to change notification settings - Fork 12
Add telemetry-to-caas kind for CaaS (Collector as a Service) support #436
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
base: main
Are you sure you want to change the base?
Changes from all commits
08837fe
79eaadc
12223ce
2da163c
46b0129
a10eb6b
41f70d5
6de322b
10140df
d592028
7aca1b4
a3d0f5e
00c635d
e4dbdf6
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 | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ const { | |||||||
| getCredsForDTAsUPS, | ||||||||
| getCredsForCLSAsUPS, | ||||||||
| augmentCLCreds, | ||||||||
| augmentCaaSCreds, | ||||||||
| hasDependency, | ||||||||
| _require | ||||||||
| } = require('../utils') | ||||||||
|
|
@@ -126,6 +127,18 @@ function _getExporter() { | |||||||
| config.credentials ??= credentials.credentials | ||||||||
| } | ||||||||
|
|
||||||||
| if (kind.match(/to-caas$/)) { | ||||||||
| if (!credentials) throw new Error('No CaaS credentials found. Make sure the app is bound to a caas-service instance.') | ||||||||
| augmentCaaSCreds(credentials) | ||||||||
| // Set OTLP env vars to binding credentials (prevents stale env values from taking precedence) | ||||||||
| process.env.OTEL_EXPORTER_OTLP_ENDPOINT = credentials.baseUrl | ||||||||
|
Contributor
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. Bug:
Suggested change
Double-check suggestion before committing. Edit this comment for amendments. Please provide feedback on the review comment by checking the appropriate box:
|
||||||||
| // Append /v1/traces to base URL (OTLP exporter expects full URL when config.url is provided) | ||||||||
| config.url = credentials.baseUrl + '/v1/traces' | ||||||||
| if (credentials.httpAgentOptions) { | ||||||||
| config.httpAgentOptions = credentials.httpAgentOptions | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| const exporter = new tracingExporterModule[tracingExporter.class](config) | ||||||||
| LOG._debug && LOG.debug('Using trace exporter:', exporter) | ||||||||
|
|
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -137,6 +137,53 @@ function getCredsForCLSAsUPS() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function getCredsForCaaSMtls() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!process.env.VCAP_SERVICES) return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const vcap = JSON.parse(process.env.VCAP_SERVICES) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Look for user-provided service with name matching configured pattern | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { mtls_service_pattern } = cds.env.requires.telemetry | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!mtls_service_pattern) return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
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. Logic Error:
Suggested change
Double-check suggestion before committing. Edit this comment for amendments. Please provide feedback on the review comment by checking the appropriate box:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const pattern = new RegExp(mtls_service_pattern, 'i') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
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. Bug:
Suggested change
Double-check suggestion before committing. Edit this comment for amendments. Please provide feedback on the review comment by checking the appropriate box:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mtlsCreds = vcap['user-provided']?.find(b => b.name.match(pattern)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (mtlsCreds) return mtlsCreds.credentials | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function augmentCaaSCreds(credentials) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (credentials._augmented) return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| credentials._augmented = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // check for otlp http endpoint | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!credentials.otlp?.http) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('No OTLP HTTP endpoint found in CaaS binding. Make sure the CaaS instance is properly configured.') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Store the base URL - path will be added per signal type (traces: /v1/traces, metrics: /v1/metrics) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| credentials.baseUrl = credentials.otlp.http | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Also set url for backwards compatibility (without path - exporter will append it) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| credentials.url = credentials.otlp.http | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
vkozyura marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check for mTLS credentials (cert + key) in user-provided service | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const mtlsCreds = getCredsForCaaSMtls() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (mtlsCreds && mtlsCreds.cert && mtlsCreds.key) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cert = Buffer.from(mtlsCreds.cert, 'base64').toString('utf-8') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const key = Buffer.from(mtlsCreds.key, 'base64').toString('utf-8') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Store the mTLS options for the exporter's httpAgentOptions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // The OTLP HTTP exporter will create an https.Agent with these options | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| credentials.httpAgentOptions = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cert: cert, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key: key, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| keepAlive: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOG._error && LOG.error('Failed to configure CaaS mTLS:', err.message) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+168
to
+181
Contributor
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. Bug:
Suggested change
Double-check suggestion before committing. Edit this comment for amendments. Please provide feedback on the review comment by checking the appropriate box:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOG._warn && LOG.warn('CaaS requires mTLS authentication. No mTLS credentials found. Bind a user-provided service with cert and key (base64 encoded).') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
vkozyura marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function augmentCLCreds(credentials) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (credentials._augmented) return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| credentials._augmented = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -203,6 +250,7 @@ module.exports = { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getCredsForDTAsUPS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getCredsForCLSAsUPS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| augmentCLCreds, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| augmentCaaSCreds, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasDependency, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _hrnow, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _require | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| const cds = require('@sap/cds') | ||
|
|
||
| // Mock VCAP_SERVICES for CaaS | ||
| const MOCK_CAAS_VCAP = { | ||
| 'caas-service': [{ | ||
| name: 'test-caas', | ||
| credentials: { | ||
| otlp: { | ||
| http: 'https://caas.example.com/otlp', | ||
| grpc: 'grpc://caas.example.com:4317' | ||
| } | ||
| } | ||
| }], | ||
| 'user-provided': [{ | ||
| name: 'caas-mtls-creds', | ||
| credentials: { | ||
| cert: Buffer.from('-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----').toString('base64'), | ||
| key: Buffer.from('-----BEGIN PRIVATE KEY-----\ntest\n-----END PRIVATE KEY-----').toString('base64') | ||
| }, | ||
| tags: [] | ||
| }] | ||
| } | ||
|
|
||
| const MOCK_CAAS_VCAP_NO_MTLS = { | ||
| 'caas-service': [{ | ||
| name: 'test-caas', | ||
| credentials: { | ||
| otlp: { | ||
| http: 'https://caas.example.com/otlp', | ||
| grpc: 'grpc://caas.example.com:4317' | ||
| } | ||
| } | ||
| }] | ||
| } | ||
|
|
||
| describe('augmentCaaSCreds', () => { | ||
| let originalVcap | ||
|
|
||
| beforeAll(() => { | ||
| originalVcap = process.env.VCAP_SERVICES | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| if (originalVcap) process.env.VCAP_SERVICES = originalVcap | ||
| else delete process.env.VCAP_SERVICES | ||
| }) | ||
|
|
||
| beforeEach(() => { | ||
| cds.env.requires = cds.env.requires || {} | ||
| cds.env.requires.telemetry = { mtls_service_pattern: 'caas-mtls|caas-cert' } | ||
| delete require.cache[require.resolve('../lib/utils')] | ||
| }) | ||
|
|
||
| test('sets baseUrl and url from otlp.http', () => { | ||
| process.env.VCAP_SERVICES = JSON.stringify(MOCK_CAAS_VCAP) | ||
| delete require.cache[require.resolve('../lib/utils')] | ||
| const { augmentCaaSCreds } = require('../lib/utils') | ||
|
|
||
| const credentials = { | ||
| otlp: { | ||
| http: 'https://caas.example.com/otlp', | ||
| grpc: 'grpc://caas.example.com:4317' | ||
| } | ||
| } | ||
|
|
||
| augmentCaaSCreds(credentials) | ||
|
|
||
| expect(credentials.baseUrl).toBe('https://caas.example.com/otlp') | ||
| expect(credentials.url).toBe('https://caas.example.com/otlp') | ||
| }) | ||
|
|
||
| test('sets httpAgentOptions when mTLS credentials found', () => { | ||
| process.env.VCAP_SERVICES = JSON.stringify(MOCK_CAAS_VCAP) | ||
| delete require.cache[require.resolve('../lib/utils')] | ||
| const { augmentCaaSCreds } = require('../lib/utils') | ||
|
|
||
| const credentials = { | ||
| otlp: { http: 'https://caas.example.com/otlp' } | ||
| } | ||
|
|
||
| augmentCaaSCreds(credentials) | ||
|
|
||
| expect(credentials.httpAgentOptions).toBeDefined() | ||
| expect(credentials.httpAgentOptions.cert).toContain('BEGIN CERTIFICATE') | ||
| expect(credentials.httpAgentOptions.key).toContain('BEGIN PRIVATE KEY') | ||
| expect(credentials.httpAgentOptions.keepAlive).toBe(true) | ||
| }) | ||
|
|
||
| test('throws when no OTLP endpoints', () => { | ||
| process.env.VCAP_SERVICES = JSON.stringify(MOCK_CAAS_VCAP) | ||
| delete require.cache[require.resolve('../lib/utils')] | ||
| const { augmentCaaSCreds } = require('../lib/utils') | ||
|
|
||
| expect(() => augmentCaaSCreds({})).toThrow('No OTLP HTTP endpoint found') | ||
| }) | ||
|
|
||
| test('does not augment twice', () => { | ||
| process.env.VCAP_SERVICES = JSON.stringify(MOCK_CAAS_VCAP) | ||
| delete require.cache[require.resolve('../lib/utils')] | ||
| const { augmentCaaSCreds } = require('../lib/utils') | ||
|
|
||
| const credentials = { | ||
| otlp: { http: 'https://caas.example.com/otlp' } | ||
| } | ||
|
|
||
| augmentCaaSCreds(credentials) | ||
| const originalBaseUrl = credentials.baseUrl | ||
|
|
||
| credentials.otlp.http = 'https://different.com' | ||
| augmentCaaSCreds(credentials) | ||
|
|
||
| expect(credentials.baseUrl).toBe(originalBaseUrl) | ||
| }) | ||
|
|
||
| test('no httpAgentOptions when mTLS credentials not found', () => { | ||
| process.env.VCAP_SERVICES = JSON.stringify(MOCK_CAAS_VCAP_NO_MTLS) | ||
| delete require.cache[require.resolve('../lib/utils')] | ||
| const { augmentCaaSCreds } = require('../lib/utils') | ||
|
|
||
| const credentials = { | ||
| otlp: { http: 'https://caas.example.com/otlp' } | ||
| } | ||
|
|
||
| augmentCaaSCreds(credentials) | ||
|
|
||
| expect(credentials.httpAgentOptions).toBeUndefined() | ||
| }) | ||
| }) |
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.
Bug: Same process-wide env var mutation as in
tracing/index.js—OTEL_EXPORTER_OTLP_ENDPOINTis set to the base URL without a signal-specific path, which conflicts with the value set by the tracing exporter (or vice-versa depending on initialisation order). UseOTEL_EXPORTER_OTLP_METRICS_ENDPOINTinstead.Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box: