Skip to content

Commit afbb68c

Browse files
ida613BridgeAR
andauthored
fix: [OTLP] detect http/https protocol from parsedUrl.protocol (#9028)
* detect http/https protocol from prasedUrl.protocol * privatize the attribute this.transport * fix ci failures * fall back on https * Update packages/dd-trace/src/opentelemetry/otlp/otlp_http_exporter_base.js Co-authored-by: Ruben Bridgewater <ruben@bridgewater.de> * added telemetry test cases --------- Co-authored-by: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 16a785f commit afbb68c

2 files changed

Lines changed: 59 additions & 5 deletions

File tree

packages/dd-trace/src/opentelemetry/otlp/otlp_http_exporter_base.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
'use strict'
22

3-
const http = require('http')
4-
const { URL } = require('url')
3+
const http = require('node:http')
4+
const https = require('node:https')
5+
const { URL } = require('node:url')
56
const log = require('../../log')
67
const telemetryMetrics = require('../../telemetry/metrics')
78

@@ -16,6 +17,8 @@ const tracerMetrics = telemetryMetrics.manager.namespace('tracers')
1617
* @class OtlpHttpExporterBase
1718
*/
1819
class OtlpHttpExporterBase {
20+
#transport = https
21+
1922
/**
2023
* Creates a new OtlpHttpExporterBase instance.
2124
*
@@ -45,7 +48,7 @@ class OtlpHttpExporterBase {
4548
this.setUrl(url)
4649

4750
this.telemetryTags = [
48-
'protocol:http',
51+
`protocol:${this.#transport === https ? 'https' : 'http'}`,
4952
`encoding:${isJson ? 'json' : 'protobuf'}`,
5053
]
5154
}
@@ -81,7 +84,7 @@ class OtlpHttpExporterBase {
8184
},
8285
}
8386

84-
const req = http.request(options, (res) => {
87+
const req = this.#transport.request(options, (res) => {
8588
let data = ''
8689

8790
res.on('data', (chunk) => {
@@ -124,6 +127,10 @@ class OtlpHttpExporterBase {
124127
this.options.hostname = parsedUrl.hostname
125128
this.options.port = parsedUrl.port
126129
this.options.path = parsedUrl.pathname + parsedUrl.search
130+
this.#transport = parsedUrl.protocol === 'http:' ? http : https
131+
if (this.telemetryTags !== undefined) {
132+
this.telemetryTags[0] = `protocol:${this.#transport === https ? 'https' : 'http'}`
133+
}
127134
}
128135

129136
/**

packages/dd-trace/test/opentelemetry/traces.spec.js

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
'use strict'
22

33
const assert = require('assert')
4-
const http = require('http')
4+
const http = require('node:http')
5+
const https = require('node:https')
56

67
const { describe, it, beforeEach, afterEach } = require('mocha')
78
const sinon = require('sinon')
@@ -844,6 +845,18 @@ describe('OpenTelemetry Traces', () => {
844845
})
845846

846847
describe('Telemetry Metrics', () => {
848+
it('sets protocol:http tag for http:// endpoint', () => {
849+
const exporter = new OtlpHttpTraceExporter('http://collector.example/v1/traces', {}, 1000, {})
850+
851+
assert.ok(exporter.telemetryTags.includes('protocol:http'))
852+
})
853+
854+
it('sets protocol:https tag for https:// endpoint', () => {
855+
const exporter = new OtlpHttpTraceExporter('https://collector.example/v1/traces', {}, 1000, {})
856+
857+
assert.ok(exporter.telemetryTags.includes('protocol:https'))
858+
})
859+
847860
it('tracks telemetry metrics for exported traces', () => {
848861
const telemetryMetrics = {
849862
manager: { namespace: sinon.stub().returns({ count: sinon.stub().returns({ inc: sinon.spy() }) }) },
@@ -887,5 +900,39 @@ describe('OpenTelemetry Traces', () => {
887900

888901
assert.strictEqual(exporter.options.path, '/v1/traces?token=abc')
889902
})
903+
904+
it('selects http transport for http:// URLs', () => {
905+
const exporter = new OtlpHttpTraceExporter('http://collector.example/v1/traces', {}, 1000, {})
906+
const mockReq = { write: () => {}, end: () => {}, on: () => mockReq, once: () => mockReq }
907+
const httpStub = sinon.stub(http, 'request').returns(mockReq)
908+
sinon.stub(https, 'request').returns(mockReq)
909+
910+
exporter.sendPayload(Buffer.from('{}'), () => {})
911+
912+
assert.ok(httpStub.calledOnce, 'http.request should have been called')
913+
})
914+
915+
it('selects https transport for https:// URLs', () => {
916+
const exporter = new OtlpHttpTraceExporter('https://collector.example/v1/traces', {}, 1000, {})
917+
const mockReq = { write: () => {}, end: () => {}, on: () => mockReq, once: () => mockReq }
918+
sinon.stub(http, 'request').returns(mockReq)
919+
const httpsStub = sinon.stub(https, 'request').returns(mockReq)
920+
921+
exporter.sendPayload(Buffer.from('{}'), () => {})
922+
923+
assert.ok(httpsStub.calledOnce, 'https.request should have been called')
924+
})
925+
926+
it('switches transport when setUrl is called with a different scheme', () => {
927+
const exporter = new OtlpHttpTraceExporter('http://collector.example/v1/traces', {}, 1000, {})
928+
const mockReq = { write: () => {}, end: () => {}, on: () => mockReq, once: () => mockReq }
929+
sinon.stub(http, 'request').returns(mockReq)
930+
const httpsStub = sinon.stub(https, 'request').returns(mockReq)
931+
932+
exporter.setUrl('https://secure-collector.example/v1/traces')
933+
exporter.sendPayload(Buffer.from('{}'), () => {})
934+
935+
assert.ok(httpsStub.calledOnce, 'https.request should have been called after switching to https')
936+
})
890937
})
891938
})

0 commit comments

Comments
 (0)