Skip to content

Commit b0a3e6d

Browse files
mrickardmrickard
authored
chore: Added otel compliant server.address, server.port, and http.request.method to external http spans (#2169)
1 parent 7ddf2c9 commit b0a3e6d

File tree

9 files changed

+81
-21
lines changed

9 files changed

+81
-21
lines changed

‎lib/instrumentation/core/http-outbound.js

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const http = require('http')
1717
const synthetics = require('../../synthetics')
1818

1919
const NAMES = require('../../metrics/names')
20+
const { DESTINATIONS } = require('../../config/attribute-filter')
2021

2122
const DEFAULT_HOST = 'localhost'
2223
const DEFAULT_HTTP_PORT = 80
@@ -88,14 +89,15 @@ function parseOpts(opts) {
8889
module.exports = function instrumentOutbound(agent, opts, makeRequest) {
8990
opts = parseOpts(opts)
9091
const defaultPort = getDefaultPort(opts)
91-
let hostname = getDefaultHostName(opts)
92+
const host = getDefaultHostName(opts)
9293
const port = getPort(opts, defaultPort)
9394

94-
if (!hostname || port < 1) {
95-
logger.warn('Invalid host name (%s) or port (%s) for outbound request.', hostname, port)
95+
if (!host || port < 1) {
96+
logger.warn('Invalid host name (%s) or port (%s) for outbound request.', host, port)
9697
return makeRequest(opts)
9798
}
9899

100+
let hostname = host
99101
if (port && port !== defaultPort) {
100102
hostname += ':' + port
101103
}
@@ -118,7 +120,7 @@ module.exports = function instrumentOutbound(agent, opts, makeRequest) {
118120
recordExternal(hostname, 'http'),
119121
parent,
120122
false,
121-
instrumentRequest.bind(null, agent, opts, makeRequest, hostname)
123+
instrumentRequest.bind(null, { agent, opts, makeRequest, host, port, hostname })
122124
)
123125
}
124126

@@ -127,14 +129,17 @@ module.exports = function instrumentOutbound(agent, opts, makeRequest) {
127129
* Instruments the request.emit to properly handle the response of the
128130
* outbound http request
129131
*
130-
* @param {Agent} agent New Relic agent
131-
* @param {string|object} opts a url string or HTTP request options
132-
* @param {Function} makeRequest function to make request
133-
* @param {string} hostname host of outbound request
132+
* @param {object} params to function
133+
* @param {Agent} params.agent New Relic agent
134+
* @param {string|object} params.opts a url string or HTTP request options
135+
* @param {Function} params.makeRequest function to make request
136+
* @param {string} params.hostname host + port of outbound request
137+
* @param {string} params.host host of outbound request
138+
* @param {string} params.port port of outbound request
134139
* @param {TraceSegment} segment outbound http segment
135140
* @returns {http.IncomingMessage} request actual http outbound request
136141
*/
137-
function instrumentRequest(agent, opts, makeRequest, hostname, segment) {
142+
function instrumentRequest({ agent, opts, makeRequest, host, port, hostname }, segment) {
138143
const transaction = segment.transaction
139144
const outboundHeaders = Object.create(null)
140145

@@ -144,7 +149,7 @@ function instrumentRequest(agent, opts, makeRequest, hostname, segment) {
144149
maybeAddDtCatHeaders(agent, transaction, outboundHeaders, opts?.headers)
145150
opts.headers = assignOutgoingHeaders(opts.headers, outboundHeaders)
146151

147-
const request = applySegment(opts, makeRequest, hostname, segment)
152+
const request = applySegment({ opts, makeRequest, hostname, host, port, segment })
148153

149154
instrumentRequestEmit(agent, hostname, segment, request)
150155

@@ -205,13 +210,16 @@ function assignOutgoingHeaders(currentHeaders, outboundHeaders) {
205210
/**
206211
* Starts the http outbound segment and attaches relevant attributes to the segment/span.
207212
*
208-
* @param {string|object} opts a url string or HTTP request options
209-
* @param {Function} makeRequest function to make request
210-
* @param {string} hostname host of outbound request
211-
* @param {TraceSegment} segment outbound http segment
213+
* @param {object} params to function
214+
* @param {string|object} params.opts a url string or HTTP request options
215+
* @param {Function} params.makeRequest function to make request
216+
* @param {string} params.host host of outbound request
217+
* @param {string} params.port port of outbound request
218+
* @param {string} params.hostname host + port of outbound request
219+
* @param {TraceSegment} params.segment outbound http segment
212220
* @returns {http.IncomingMessage} request actual http outbound request
213221
*/
214-
function applySegment(opts, makeRequest, hostname, segment) {
222+
function applySegment({ opts, makeRequest, host, port, hostname, segment }) {
215223
segment.start()
216224
const request = makeRequest(opts)
217225
const parsed = urltils.scrubAndParseParameters(request.path)
@@ -227,6 +235,8 @@ function applySegment(opts, makeRequest, hostname, segment) {
227235
segment.addSpanAttribute(`request.parameters.${key}`, parsed.parameters[key])
228236
}
229237
}
238+
segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'host', host)
239+
segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'port', port)
230240
segment.addAttribute('url', `${proto}//${hostname}${parsed.path}`)
231241
segment.addAttribute('procedure', opts.method || 'GET')
232242
return request

‎lib/instrumentation/undici.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const symbols = require('../symbols')
1313
const { executionAsyncResource } = require('async_hooks')
1414
const diagnosticsChannel = require('diagnostics_channel')
1515
const synthetics = require('../synthetics')
16+
const { DESTINATIONS } = require('../config/attribute-filter')
1617

1718
const channels = [
1819
{ channel: diagnosticsChannel.channel('undici:request:create'), hook: requestCreateHook },
@@ -128,6 +129,8 @@ function createExternalSegment({ shim, request, parentSegment }) {
128129
url.searchParams.forEach((value, key) => {
129130
segment.addSpanAttribute(`request.parameters.${key}`, value)
130131
})
132+
segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'host', url.hostname)
133+
segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'port', url.port)
131134
segment.addAttribute('procedure', request.method || 'GET')
132135
request[symbols.segment] = segment
133136
}

‎lib/spans/span-event.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class SpanEvent {
8282
* category of the segment.
8383
*
8484
* @param {TraceSegment} segment - The segment to turn into a span event.
85-
* @param {?string} [parentId=null] - The ID of the segment's parent.
85+
* @param {?string} [parentId] - The ID of the segment's parent.
8686
* @param isRoot
8787
* @returns {SpanEvent} The constructed event.
8888
*/
@@ -194,8 +194,19 @@ class HttpSpanEvent extends SpanEvent {
194194
attributes.url = null
195195
}
196196

197+
if (attributes.host) {
198+
this.addAttribute('server.address', attributes.host)
199+
attributes.host = null
200+
}
201+
202+
if (attributes.port) {
203+
this.addAttribute('server.port', attributes.port, true)
204+
attributes.port = null
205+
}
206+
197207
if (attributes.procedure) {
198208
this.addAttribute('http.method', attributes.procedure)
209+
this.addAttribute('http.request.method', attributes.procedure)
199210
attributes.procedure = null
200211
}
201212
}

‎lib/spans/streaming-span-event.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class StreamingSpanEvent {
6464
*
6565
* @param {string} key Name of the attribute to be stored.
6666
* @param {string|boolean|number} value Value of the attribute to be stored.
67-
* @param {boolean} [truncateExempt=false] Set to true if attribute should not be truncated.
67+
* @param {boolean} [truncateExempt] Set to true if attribute should not be truncated.
6868
*/
6969
addCustomAttribute(key, value, truncateExempt = false) {
7070
const shouldKeep = this._checkFilter(key)
@@ -79,7 +79,7 @@ class StreamingSpanEvent {
7979
*
8080
* @param {string} key Name of the attribute to be stored.
8181
* @param {string|boolean|number} value Value of the attribute to be stored.
82-
* @param {boolean} [truncateExempt=false] Set to true if attribute should not be truncated.
82+
* @param {boolean} [truncateExempt] Set to true if attribute should not be truncated.
8383
*/
8484
addAgentAttribute(key, value, truncateExempt = false) {
8585
const shouldKeep = this._checkFilter(key)
@@ -197,8 +197,19 @@ class StreamingHttpSpanEvent extends StreamingSpanEvent {
197197
agentAttributes.url = null
198198
}
199199

200+
if (agentAttributes.host) {
201+
this.addAgentAttribute('server.address', agentAttributes.host)
202+
agentAttributes.host = null
203+
}
204+
205+
if (agentAttributes.port) {
206+
this.addAgentAttribute('server.port', agentAttributes.port, true)
207+
agentAttributes.port = null
208+
}
209+
200210
if (agentAttributes.procedure) {
201211
this.addAgentAttribute('http.method', agentAttributes.procedure)
212+
this.addAgentAttribute('http.request.method', agentAttributes.procedure)
202213
agentAttributes.procedure = null
203214
}
204215
}

‎test/integration/instrumentation/http-outbound.tap.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,11 @@ tap.test('external requests', function (t) {
182182
res.resume()
183183
res.on('end', () => {
184184
const segment = tx.trace.root.children[0]
185-
t.equal(segment.getAttributes().url, 'http://example.com/')
186-
t.equal(segment.getAttributes().procedure, 'GET')
185+
const attrs = segment.getAttributes()
186+
t.same(attrs, {
187+
url: 'http://example.com/',
188+
procedure: 'GET'
189+
})
187190
t.equal(reqSegment, segment, 'should expose external')
188191
t.end()
189192
})

‎test/unit/instrumentation/http/outbound.test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ tap.test('instrumentOutbound', (t) => {
127127
t.same(
128128
transaction.trace.root.children[0].attributes.get(DESTINATIONS.SPAN_EVENT),
129129
{
130+
'host': HOSTNAME,
131+
'port': PORT,
130132
'url': `http://${HOSTNAME}:${PORT}/asdf`,
131133
'procedure': 'GET',
132134
'request.parameters.a': 'b',

‎test/unit/spans/span-event.test.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,20 @@ tap.test('fromSegment()', (t) => {
164164

165165
// Should have (most) http properties.
166166
t.equal(attributes['http.url'], 'https://example.com/')
167+
t.equal(attributes['server.address'], 'example.com')
168+
t.equal(attributes['server.port'], 443)
167169
t.ok(attributes['http.method'])
170+
t.ok(attributes['http.request.method'])
168171
t.equal(attributes['http.statusCode'], 200)
169172
t.equal(attributes['http.statusText'], 'OK')
170173

174+
// should nullify mapped properties
175+
t.notOk(attributes.library)
176+
t.notOk(attributes.url)
177+
t.notOk(attributes.host)
178+
t.notOk(attributes.port)
179+
t.notOk(attributes.procedure)
180+
171181
// Should have no datastore properties.
172182
const hasOwnAttribute = Object.hasOwnProperty.bind(attributes)
173183
t.notOk(hasOwnAttribute('db.statement'))
@@ -251,7 +261,9 @@ tap.test('fromSegment()', (t) => {
251261
// Should have not http properties.
252262
const hasOwnAttribute = Object.hasOwnProperty.bind(attributes)
253263
t.notOk(hasOwnAttribute('http.url'))
264+
t.notOk(hasOwnAttribute('server.address'))
254265
t.notOk(hasOwnAttribute('http.method'))
266+
t.notOk(hasOwnAttribute('http.request.method'))
255267

256268
// Should have (most) datastore properties.
257269
t.ok(attributes['db.instance'])

‎test/unit/spans/streaming-span-event.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,10 @@ tap.test('fromSegment()', (t) => {
156156

157157
// Should have (most) http properties.
158158
t.same(agentAttributes['http.url'], { [STRING_TYPE]: 'https://example.com/' })
159+
t.same(agentAttributes['server.address'], { [STRING_TYPE]: 'example.com' })
160+
t.same(agentAttributes['server.port'], { [INT_TYPE]: 443 })
159161
t.ok(agentAttributes['http.method'])
162+
t.ok(agentAttributes['http.request.method'])
160163
t.same(agentAttributes['http.statusCode'], { [INT_TYPE]: 200 })
161164
t.same(agentAttributes['http.statusText'], { [STRING_TYPE]: 'OK' })
162165

@@ -246,7 +249,9 @@ tap.test('fromSegment()', (t) => {
246249
// Should have not http properties.
247250
const hasOwnAttribute = Object.hasOwnProperty.bind(agentAttributes)
248251
t.notOk(hasOwnAttribute('http.url'))
252+
t.notOk(hasOwnAttribute('server.address'))
249253
t.notOk(hasOwnAttribute('http.method'))
254+
t.notOk(hasOwnAttribute('http.request.method'))
250255

251256
// Should have (most) datastore properties.
252257
t.ok(agentAttributes['db.instance'])

‎test/versioned/undici/requests.tap.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ tap.test('Undici request tests', (t) => {
2222
let server
2323
let REQUEST_URL
2424
let HOST
25+
let PORT
2526

2627
function createServer() {
2728
server = http.createServer((req, res) => {
@@ -45,6 +46,7 @@ tap.test('Undici request tests', (t) => {
4546

4647
server.listen(0)
4748
const { port } = server.address()
49+
PORT = port
4850
HOST = `localhost:${port}`
4951
REQUEST_URL = `http://${HOST}`
5052
return server
@@ -146,7 +148,8 @@ tap.test('Undici request tests', (t) => {
146148
t.equal(spanAttrs['http.statusText'], 'OK')
147149
t.equal(spanAttrs['request.parameters.a'], 'b')
148150
t.equal(spanAttrs['request.parameters.c'], 'd')
149-
151+
t.equal(spanAttrs.host, 'localhost')
152+
t.equal(spanAttrs.port, `${PORT}`)
150153
tx.end()
151154
t.end()
152155
})

0 commit comments

Comments
 (0)