Skip to content

Commit

Permalink
feat: Added segment synthesizer and provided ability to convert http …
Browse files Browse the repository at this point in the history
…client otel spans to external http trace segments (#2745)
  • Loading branch information
bizob2828 committed Nov 19, 2024
1 parent 18a5f2b commit cac0902
Show file tree
Hide file tree
Showing 13 changed files with 476 additions and 85 deletions.
236 changes: 223 additions & 13 deletions THIRD_PARTY_NOTICES.md

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions lib/metrics/recorders/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ function recordQueryMetrics(segment, scope, transaction) {

if (this.raw) {
transaction.agent.queries.add({
segment,
segment,
transaction,
type: this.type.toLowerCase(),
query: this.raw,
type: this.type.toLowerCase(),
query: this.raw,
trace: this.trace
})
}
Expand Down
6 changes: 6 additions & 0 deletions lib/otel/rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class Rule {
#name
#spanKinds
#requiredAttributes
#type
#mappings

/**
Expand All @@ -63,6 +64,7 @@ class Rule {
}

this.#name = input.name
this.#type = input.type
this.#spanKinds = input.matcher.required_span_kinds?.map((v) => v.toLowerCase()) ?? []
this.#requiredAttributes = input.matcher.required_attribute_keys ?? []
this.#mappings = input.target.attribute_mappings ?? []
Expand All @@ -72,6 +74,10 @@ class Rule {
return this.#name
}

get type() {
return this.#type
}

get isServerRule() {
return this.#spanKinds.includes(Rule.OTEL_SPAN_KIND_SERVER)
}
Expand Down
5 changes: 5 additions & 0 deletions lib/otel/rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@
},
{
"name": "OtelDbClientRedis1_24",
"type": "db",
"matcher": {
"required_span_kinds": [
"client"
Expand Down Expand Up @@ -267,6 +268,7 @@
},
{
"name": "OtelDbClient1_24",
"type": "db",
"matcher": {
"required_span_kinds": [
"client"
Expand Down Expand Up @@ -302,6 +304,7 @@
},
{
"name": "OtelHttpClient1_23",
"type": "external",
"matcher": {
"required_metric_names": [
"http.client.request.duration"
Expand Down Expand Up @@ -338,6 +341,7 @@
},
{
"name": "OtelHttpClient1_20",
"type": "external",
"matcher": {
"required_metric_names": [
"http.client.duration"
Expand Down Expand Up @@ -410,6 +414,7 @@
},
{
"name": "FallbackClient",
"type": "external",
"matcher": {
"required_metric_names": [
"rpc.client.duration",
Expand Down
50 changes: 50 additions & 0 deletions lib/otel/segment-synthesis.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright 2024 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'
const { RulesEngine } = require('./rules')
const defaultLogger = require('../logger').child({ component: 'segment-synthesizer' })
const NAMES = require('../metrics/names')
const { SEMATTRS_HTTP_HOST } = require('@opentelemetry/semantic-conventions')

class SegmentSynthesizer {
constructor(agent, { logger = defaultLogger } = {}) {
this.agent = agent
this.logger = logger
this.engine = new RulesEngine()
}

synthesize(otelSpan) {
const rule = this.engine.test(otelSpan)
if (!rule?.type) {
this.logger.debug(
'Cannot match a rule to span name: %s, kind %s',
otelSpan?.name,
otelSpan?.kind
)
return
}

if (rule?.type === 'external') {
return this.createExternalSegment(otelSpan)
}
this.logger.debug('Found type: %s, no synthesize rule currently built', rule.type)
}

// TODO: should we move these to somewhere else and use in the places
// where external segments are created in our agent
createExternalSegment(otelSpan) {
const context = this.agent.tracer.getContext()
const host = otelSpan.attributes[SEMATTRS_HTTP_HOST] || 'Unknown'
const name = NAMES.EXTERNAL.PREFIX + host
return this.agent.tracer.createSegment({
name,
parent: context.segment,
transaction: context.transaction
})
}
}

module.exports = SegmentSynthesizer
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@
"@grpc/proto-loader": "^0.7.5",
"@newrelic/security-agent": "^2.0.0",
"@opentelemetry/api": "^1.9.0",
"@opentelemetry/semantic-conventions": "^1.27.0",
"@tyriar/fibonacci-heap": "^2.0.7",
"concat-stream": "^2.0.0",
"https-proxy-agent": "^7.0.1",
Expand Down
102 changes: 102 additions & 0 deletions test/unit/lib/otel/segment-synthesizer.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright 2024 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

'use strict'
const test = require('node:test')
const assert = require('node:assert')

const helper = require('../../../lib/agent_helper')
const { ROOT_CONTEXT, SpanKind, TraceFlags } = require('@opentelemetry/api')
const { BasicTracerProvider, Span } = require('@opentelemetry/sdk-trace-base')
const SegmentSynthesizer = require('../../../../lib/otel/segment-synthesis')
const {
SEMATTRS_DB_SYSTEM,
SEMATTRS_HTTP_HOST,
SEMATTRS_HTTP_METHOD
} = require('@opentelemetry/semantic-conventions')
const createMockLogger = require('../../mocks/logger')

test.beforeEach((ctx) => {
const loggerMock = createMockLogger()
const agent = helper.loadMockedAgent()
const synthesizer = new SegmentSynthesizer(agent, { logger: loggerMock })
const tracer = new BasicTracerProvider().getTracer('default')
const parentId = '5c1c63257de34c67'
ctx.nr = {
agent,
loggerMock,
parentId,
synthesizer,
tracer
}
})

test.afterEach((ctx) => {
helper.unloadAgent(ctx.nr.agent)
})

test('should create http external segment from otel http client span', (t, end) => {
const { agent, synthesizer, parentId, tracer } = t.nr
helper.runInTransaction(agent, (tx) => {
const spanContext = {
traceId: tx.trace.id,
spanId: tx.trace.root.id,
traceFlags: TraceFlags.SAMPLED
}
const span = new Span(tracer, ROOT_CONTEXT, 'test-span', spanContext, SpanKind.CLIENT, parentId)
span.setAttribute(SEMATTRS_HTTP_METHOD, 'GET')
span.setAttribute(SEMATTRS_HTTP_HOST, 'newrelic.com')
const segment = synthesizer.synthesize(span)
assert.equal(segment.name, 'External/newrelic.com')
assert.equal(segment.parentId, tx.trace.root.id)
tx.end()
end()
})
})

test('should log warning if a rule does have a synthesis for the given type', (t, end) => {
const { agent, synthesizer, loggerMock, parentId, tracer } = t.nr

helper.runInTransaction(agent, (tx) => {
const spanContext = {
traceId: tx.trace.id,
spanId: tx.trace.root.id,
traceFlags: TraceFlags.SAMPLED
}
const span = new Span(tracer, ROOT_CONTEXT, 'test-span', spanContext, SpanKind.CLIENT, parentId)
span.setAttribute(SEMATTRS_DB_SYSTEM, 'postgres')
const segment = synthesizer.synthesize(span)
assert.ok(!segment)
assert.deepEqual(loggerMock.debug.args[0], [
'Found type: %s, no synthesize rule currently built',
'db'
])
tx.end()
end()
})
})

test('should log warning span does not match a rule', (t, end) => {
const { agent, synthesizer, loggerMock, parentId, tracer } = t.nr

helper.runInTransaction(agent, (tx) => {
const spanContext = {
traceId: tx.trace.id,
spanId: tx.trace.root.id,
traceFlags: TraceFlags.SAMPLED
}

const span = new Span(tracer, ROOT_CONTEXT, 'test-span', spanContext, 'bogus', parentId)
const segment = synthesizer.synthesize(span)
assert.ok(!segment)
assert.deepEqual(loggerMock.debug.args[0], [
'Cannot match a rule to span name: %s, kind %s',
'test-span',
'bogus'
])
tx.end()
end()
})
})
6 changes: 3 additions & 3 deletions test/versioned/aws-sdk-v3/client-dynamodb.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ test('DynamoDB', async (t) => {
}
tx.end()
const root = tx.trace.root
const segments = common.checkAWSAttributes({
trace: tx.trace,
segment: root,
const segments = common.checkAWSAttributes({
trace: tx.trace,
segment: root,
pattern: common.DATASTORE_PATTERN
})

Expand Down
16 changes: 8 additions & 8 deletions test/versioned/koa/code-level-metrics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,12 @@ test('vanilla koa, no router', async (t) => {
{
segments: [
{
segment: one,
segment: one,
name: 'one',
filepath: 'code-level-metrics.test.js'
},
{
segment: two,
segment: two,
name: 'two',
filepath: 'code-level-metrics.test.js'
}
Expand Down Expand Up @@ -168,17 +168,17 @@ test('using koa-router', async (t) => {
{
segments: [
{
segment: dispatch,
segment: dispatch,
name: 'dispatch',
filepath: 'koa-router/lib/router.js'
},
{
segment: appLevel,
segment: appLevel,
name: 'appLevelMiddleware',
filepath: 'code-level-metrics.test.js'
},
{
segment: secondMw,
segment: secondMw,
name: 'secondMiddleware',
filepath: 'code-level-metrics.test.js'
}
Expand Down Expand Up @@ -239,17 +239,17 @@ test('using @koa/router', async (t) => {
{
segments: [
{
segment: dispatch,
segment: dispatch,
name: 'dispatch',
filepath: '@koa/router/lib/router.js'
},
{
segment: appLevel,
segment: appLevel,
name: 'appLevelMiddleware',
filepath: 'code-level-metrics.test.js'
},
{
segment: secondMw,
segment: secondMw,
name: 'secondMiddleware',
filepath: 'code-level-metrics.test.js'
}
Expand Down
6 changes: 5 additions & 1 deletion test/versioned/memcached/memcached.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,11 @@ test('memcached instrumentation', { timeout: 5000 }, async function (t) {
assert.ok(!err)
transaction.end()
checkParams(firstSegment, 'server1', '1111')
checkParams(transaction.trace.getParent(agent.tracer.getSegment().parentId), 'server2', '2222')
checkParams(
transaction.trace.getParent(agent.tracer.getSegment().parentId),
'server2',
'2222'
)
end()
})
})
Expand Down
3 changes: 2 additions & 1 deletion test/versioned/nextjs/attributes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ test('Next.js', async (t) => {
enabled,
skipFull: true
})
})
}
)

await t.test('should not add CLM attrs to static page segment', async (t) => {
agent.config.code_level_metrics = { enabled }
Expand Down
6 changes: 3 additions & 3 deletions test/versioned/pg/pg.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ module.exports = function runTests(name, clientFactory) {
assert.ok(agent.getTransaction(), 'transaction should still still be visible')
assert.equal(selectResults.rows[0][COL], colVal, 'Postgres client should still work')
transaction.end()
verify(assert, transaction)
verify(assert, transaction)
end()
} catch (err) {
assert.ifError(err)
Expand Down Expand Up @@ -465,7 +465,7 @@ module.exports = function runTests(name, clientFactory) {

transaction.end()
pool.end()
verify(plan, transaction)
verify(plan, transaction)
})
})
})
Expand Down Expand Up @@ -514,7 +514,7 @@ module.exports = function runTests(name, clientFactory) {
}

done(true)
verify(plan, transaction)
verify(plan, transaction)
})
})
})
Expand Down
Loading

0 comments on commit cac0902

Please sign in to comment.