Skip to content

Commit f1eea8d

Browse files
amychisholm03amychisholm03
authored
fix: getHostnameSafe rewrite (newrelic#3132)
1 parent 7f28a29 commit f1eea8d

File tree

6 files changed

+29
-25
lines changed

6 files changed

+29
-25
lines changed

‎lib/collector/facts.js‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,7 @@ async function facts(agent, callback, { logger = defaultLogger } = {}) {
3131
systemInfo = systemInfo || Object.create(null)
3232
environment = environment || []
3333

34-
let hostname = agent.config.getHostnameSafe()
35-
if (agent.config.gcp_cloud_run.use_instance_as_host && process.env.K_SERVICE) {
36-
// K_SERVICE is the name of the service in GCP Cloud Run
37-
hostname = systemInfo.vendors?.gcp?.id
38-
}
34+
const hostname = agent.config.getHostnameSafe(systemInfo?.vendors?.gcp?.id)
3935
const results = {
4036
utilization: {
4137
metadata_version: 5,

‎lib/config/index.js‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -972,16 +972,25 @@ Config.prototype.getIPAddresses = function getIPAddresses() {
972972
return addresses
973973
}
974974

975-
function getHostnameSafe() {
975+
/**
976+
* Gets the system's host name. If that fails, it just returns ipv4/6
977+
* based on the user's process_host.ipv_preference setting.
978+
* @param {string} gcpId GCP metadata ID
979+
* @returns {string} host name
980+
*/
981+
function getHostnameSafe(gcpId = null) {
976982
let _hostname
977983
const config = this
978984
this.getHostnameSafe = function getCachedHostname() {
979985
return _hostname
980986
}
981987
try {
982-
if (config.heroku.use_dyno_names) {
983-
const dynoName = process.env.DYNO
984-
_hostname = dynoName || os.hostname()
988+
// Check for any special hostname scenarios.
989+
// If not applicable, use the os.hostname()
990+
if (config.heroku.use_dyno_names && process.env.DYNO) {
991+
_hostname = process.env.DYNO || os.hostname()
992+
} else if (config.gcp_cloud_run.use_instance_as_host && process.env.K_SERVICE && gcpId) {
993+
_hostname = gcpId
985994
} else {
986995
_hostname = os.hostname()
987996
}

‎lib/otel/attr-reconciler.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class AttributeReconciler {
2323

2424
resolveHost(hostname) {
2525
if (urltils.isLocalhost(hostname)) {
26-
return this.#agent.config.getHostnameSafe(hostname)
26+
return this.#agent.config.getHostnameSafe()
2727
}
2828
return hostname
2929
}

‎lib/shim/datastore-shim.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,7 @@ function _normalizeInstanceInformation(parameters, dsTracerConf, config) {
696696
}
697697
if (hasOwnProperty(parameters, 'host')) {
698698
if (parameters.host && urltils.isLocalhost(parameters.host)) {
699-
parameters.host = config.getHostnameSafe(parameters.host)
699+
parameters.host = config.getHostnameSafe()
700700
}
701701

702702
// Config's default name of a host is `UNKNOWN_BOX`.

‎test/unit/collector/facts.test.js‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,6 @@ test('host facts', async (t) => {
829829

830830
ctx.nr.agent = helper.loadMockedAgent(structuredClone(DISABLE_ALL_DETECTIONS))
831831
ctx.nr.agent.config.utilization = null
832-
ctx.nr.agent.config.getHostnameSafe = () => 'localhost'
833832
})
834833

835834
t.afterEach((ctx) => {
@@ -855,7 +854,7 @@ test('host facts', async (t) => {
855854
}
856855
})
857856

858-
await t.test('should use GCP instance ID as hostname when K_SERVICE is set', (t, end) => {
857+
await t.test('should be GCP id when K_SERVICE is set', (t, end) => {
859858
const { agent, facts } = t.nr
860859

861860
agent.config.gcp_cloud_run = { use_instance_as_host: true }
@@ -867,25 +866,25 @@ test('host facts', async (t) => {
867866
})
868867
})
869868

870-
await t.test('should use getHostnameSafe as hostname when K_SERVICE is not present', (t, end) => {
869+
await t.test('should not be GCP id when K_SERVICE is not present', (t, end) => {
871870
const { agent, facts } = t.nr
872871

873872
agent.config.gcp_cloud_run = { use_instance_as_host: true }
874873

875874
facts(agent, (result) => {
876-
assert.equal(result.host, 'localhost', 'Hostname should be set to GCP instance ID')
875+
assert.equal(result.host, os.hostname(), 'Hostname should not be set to GCP instance ID')
877876
end()
878877
})
879878
})
880879

881-
await t.test('should use getHostnameSafe when K_SERVICE is set but gcp_cloud_run.use_instance_as_host is false', (t, end) => {
880+
await t.test('should not be GCP id when K_SERVICE is set but gcp_cloud_run.use_instance_as_host is false', (t, end) => {
882881
const { agent, facts } = t.nr
883882

884883
agent.config.gcp_cloud_run = { use_instance_as_host: false }
885884
process.env.K_SERVICE = 'mock-service'
886885

887886
facts(agent, (result) => {
888-
assert.equal(result.host, 'localhost', 'Hostname should be set to GCP instance ID')
887+
assert.equal(result.host, os.hostname(), 'Hostname should not be set to GCP instance ID')
889888
end()
890889
})
891890
})

‎test/versioned/otel-bridge/span.test.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ test('client span(db) is bridge accordingly(statement test)', (t, end) => {
405405
[ATTR_SERVER_PORT]: 5436,
406406
[ATTR_SERVER_ADDRESS]: '127.0.0.1'
407407
}
408-
const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
408+
const expectedHost = agent.config.getHostnameSafe()
409409
helper.runInTransaction(agent, (tx) => {
410410
tx.name = 'db-test'
411411
tracer.startActiveSpan('db-test', { kind: otel.SpanKind.CLIENT, attributes }, (span) => {
@@ -456,7 +456,7 @@ test('client span(db) is bridged accordingly(operation test)', (t, end) => {
456456
[ATTR_SERVER_PORT]: 5436,
457457
[ATTR_SERVER_ADDRESS]: '127.0.0.1'
458458
}
459-
const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
459+
const expectedHost = agent.config.getHostnameSafe()
460460
helper.runInTransaction(agent, (tx) => {
461461
tx.name = 'db-test'
462462
tracer.startActiveSpan('db-test', { kind: otel.SpanKind.CLIENT, attributes }, (span) => {
@@ -504,7 +504,7 @@ test('client span(db) 1.17 is bridged accordingly(operation test)', (t, end) =>
504504
[ATTR_NET_PEER_PORT]: 5436,
505505
[ATTR_NET_PEER_NAME]: '127.0.0.1'
506506
}
507-
const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
507+
const expectedHost = agent.config.getHostnameSafe()
508508
helper.runInTransaction(agent, (tx) => {
509509
tx.name = 'db-test'
510510
tracer.startActiveSpan('db-test', { kind: otel.SpanKind.CLIENT, attributes }, (span) => {
@@ -555,7 +555,7 @@ test('client span(db) 1.17 mongodb is bridged accordingly(operation test)', (t,
555555
[ATTR_NET_PEER_PORT]: 5436,
556556
[ATTR_NET_PEER_NAME]: '127.0.0.1'
557557
}
558-
const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
558+
const expectedHost = agent.config.getHostnameSafe()
559559
helper.runInTransaction(agent, (tx) => {
560560
tx.name = 'db-test'
561561
tracer.startActiveSpan('db-test', { kind: otel.SpanKind.CLIENT, attributes }, (span) => {
@@ -779,7 +779,7 @@ test('producer span(legacy) is bridged accordingly', (t, end) => {
779779
helper.runInTransaction(agent, (tx) => {
780780
tx.name = 'prod-test'
781781

782-
const expectedHost = agent.config.getHostnameSafe('localhost')
782+
const expectedHost = agent.config.getHostnameSafe()
783783
tracer.startActiveSpan('prod-test', { kind: otel.SpanKind.PRODUCER, attributes }, (span) => {
784784
const segment = agent.tracer.getSegment()
785785
assert.equal(tx.traceId, span.spanContext().traceId)
@@ -826,7 +826,7 @@ test('producer span is bridged accordingly', (t, end) => {
826826
helper.runInTransaction(agent, (tx) => {
827827
tx.name = 'prod-test'
828828

829-
const expectedHost = agent.config.getHostnameSafe('localhost')
829+
const expectedHost = agent.config.getHostnameSafe()
830830
tracer.startActiveSpan('prod-test', { kind: otel.SpanKind.PRODUCER, attributes }, (span) => {
831831
const segment = agent.tracer.getSegment()
832832
assert.equal(tx.traceId, span.spanContext().traceId)
@@ -861,7 +861,7 @@ test('producer span is bridged accordingly', (t, end) => {
861861

862862
test('consumer span is bridged correctly', (t, end) => {
863863
const { agent, tracer } = t.nr
864-
const expectedHost = agent.config.getHostnameSafe('localhost')
864+
const expectedHost = agent.config.getHostnameSafe()
865865
const attributes = {
866866
[ATTR_MESSAGING_SYSTEM]: 'kafka',
867867
[ATTR_SERVER_ADDRESS]: '127.0.0.1',
@@ -914,7 +914,7 @@ test('consumer span is bridged correctly', (t, end) => {
914914

915915
test('messaging consumer skips high security attributes', (t, end) => {
916916
const { agent, tracer } = t.nr
917-
const expectedHost = agent.config.getHostnameSafe('localhost')
917+
const expectedHost = agent.config.getHostnameSafe()
918918
const attributes = {
919919
[ATTR_MESSAGING_SYSTEM]: 'kafka',
920920
[ATTR_SERVER_ADDRESS]: '127.0.0.1',

0 commit comments

Comments
 (0)