Skip to content
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

feat: Added http timeslice metrics from otel #2924

Merged
merged 4 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions lib/otel/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ module.exports = {
*/
ATTR_RPC_SYSTEM: 'rpc.system',

/**
* Server domain name, IP address, or Unix domain socket.
*
* @example example.com
* @example 10.1.2.80
* @example /tmp/my.sock
*/
ATTR_SERVER_ADDRESS: 'server.address',

/**
* Logical name of the local service being instrumented.
*/
Expand All @@ -178,6 +187,13 @@ module.exports = {
*/
ATTR_URL_PATH: 'url.path',

/**
* The scheme value for the URL.
*
* @example https
*/
ATTR_URL_SCHEME: 'url.scheme',

/* !!! Miscellaneous !!! */
/**
* Database system names.
Expand Down
9 changes: 7 additions & 2 deletions lib/otel/segments/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
'use strict'

const Transaction = require('../../transaction')
const httpRecorder = require('../../metrics/recorders/http')
const urltils = require('../../util/urltils')
const url = require('url')
const url = require('node:url')

const DESTINATION = Transaction.DESTINATIONS.TRANS_COMMON
const {
ATTR_HTTP_METHOD,
ATTR_HTTP_REQUEST_METHOD,
ATTR_HTTP_ROUTE,
ATTR_HTTP_URL,
ATTR_RPC_METHOD,
Expand All @@ -23,7 +25,7 @@ module.exports = function createServerSegment(agent, otelSpan) {
const transaction = new Transaction(agent)
transaction.type = 'web'
const rpcSystem = otelSpan.attributes[ATTR_RPC_SYSTEM]
const httpMethod = otelSpan.attributes[ATTR_HTTP_METHOD]
const httpMethod = otelSpan.attributes[ATTR_HTTP_METHOD] ?? otelSpan.attributes[ATTR_HTTP_REQUEST_METHOD]
let segment
if (rpcSystem) {
segment = rpcSegment({ agent, otelSpan, transaction, rpcSystem })
Expand All @@ -46,6 +48,7 @@ function rpcSegment({ agent, otelSpan, transaction, rpcSystem }) {
transaction.url = name
const segment = agent.tracer.createSegment({
name,
recorder: httpRecorder,
parent: transaction.trace.root,
transaction
})
Expand All @@ -69,6 +72,7 @@ function httpSegment({ agent, otelSpan, transaction, httpMethod }) {
transaction.trace.attributes.addAttribute(DESTINATION, 'request.method', httpMethod)
return agent.tracer.createSegment({
name,
recorder: httpRecorder,
parent: transaction.trace.root,
transaction
})
Expand All @@ -79,6 +83,7 @@ function genericHttpSegment({ agent, transaction }) {
transaction.name = name
return agent.tracer.createSegment({
name,
recorder: httpRecorder,
parent: transaction.trace.root,
transaction
})
Expand Down
4 changes: 3 additions & 1 deletion lib/otel/span-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ const {
ATTR_DB_NAME,
ATTR_DB_STATEMENT,
ATTR_DB_SYSTEM,
ATTR_HTTP_HOST,
ATTR_NET_PEER_NAME,
ATTR_NET_PEER_PORT,
ATTR_SERVER_ADDRESS,
} = require('./constants')

module.exports = class NrSpanProcessor {
Expand Down Expand Up @@ -60,7 +62,7 @@ module.exports = class NrSpanProcessor {
let sanitized = value
if (key === ATTR_NET_PEER_PORT) {
key = 'port_path_or_id'
} else if (prop === ATTR_NET_PEER_NAME) {
} else if (prop === ATTR_NET_PEER_NAME || prop === ATTR_SERVER_ADDRESS || prop === ATTR_HTTP_HOST) {
key = 'host'
if (urltils.isLocalhost(sanitized)) {
sanitized = this.agent.config.getHostnameSafe(sanitized)
Expand Down
188 changes: 176 additions & 12 deletions test/versioned/otel-bridge/span.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,33 @@
*/

'use strict'

const assert = require('node:assert')
const test = require('node:test')
const helper = require('../../lib/agent_helper')
const otel = require('@opentelemetry/api')
const { hrTimeToMilliseconds } = require('@opentelemetry/core')

const helper = require('../../lib/agent_helper')
const { otelSynthesis } = require('../../../lib/symbols')
const { SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD, SEMATTRS_DB_NAME, SEMATTRS_DB_STATEMENT, SEMATTRS_DB_SYSTEM, SEMATTRS_NET_PEER_PORT, SEMATTRS_NET_PEER_NAME, DbSystemValues } = require('@opentelemetry/semantic-conventions')

const {
ATTR_DB_NAME,
ATTR_DB_STATEMENT,
ATTR_DB_SYSTEM,
ATTR_HTTP_HOST,
ATTR_HTTP_METHOD,
ATTR_HTTP_REQUEST_METHOD,
ATTR_HTTP_ROUTE,
ATTR_NET_PEER_NAME,
ATTR_NET_PEER_PORT,
ATTR_RPC_METHOD,
ATTR_RPC_SERVICE,
ATTR_RPC_SYSTEM,
ATTR_SERVER_ADDRESS,
ATTR_URL_PATH,
ATTR_URL_SCHEME,
DB_SYSTEM_VALUES
} = require('../../../lib/otel/constants.js')

test.beforeEach((ctx) => {
const agent = helper.instrumentMockedAgent({
Expand Down Expand Up @@ -85,7 +105,7 @@ test('Otel http external span test', (t, end) => {
const { agent, tracer } = t.nr
helper.runInTransaction(agent, (tx) => {
tx.name = 'http-external-test'
tracer.startActiveSpan('http-outbound', { kind: otel.SpanKind.CLIENT, attributes: { [SEMATTRS_HTTP_HOST]: 'newrelic.com', [SEMATTRS_HTTP_METHOD]: 'GET' } }, (span) => {
tracer.startActiveSpan('http-outbound', { kind: otel.SpanKind.CLIENT, attributes: { [ATTR_HTTP_HOST]: 'newrelic.com', [ATTR_HTTP_METHOD]: 'GET' } }, (span) => {
const segment = agent.tracer.getSegment()
assert.equal(segment.name, 'External/newrelic.com')
span.end()
Expand All @@ -107,11 +127,11 @@ test('Otel http external span test', (t, end) => {
test('Otel db client span statement test', (t, end) => {
const { agent, tracer } = t.nr
const attributes = {
[SEMATTRS_DB_NAME]: 'test-db',
[SEMATTRS_DB_SYSTEM]: 'postgresql',
[SEMATTRS_DB_STATEMENT]: "select foo from test where foo = 'bar';",
[SEMATTRS_NET_PEER_PORT]: 5436,
[SEMATTRS_NET_PEER_NAME]: '127.0.0.1'
[ATTR_DB_NAME]: 'test-db',
[ATTR_DB_SYSTEM]: 'postgresql',
[ATTR_DB_STATEMENT]: "select foo from test where foo = 'bar';",
[ATTR_NET_PEER_PORT]: 5436,
[ATTR_NET_PEER_NAME]: '127.0.0.1'
}
const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
helper.runInTransaction(agent, (tx) => {
Expand Down Expand Up @@ -152,10 +172,10 @@ test('Otel db client span statement test', (t, end) => {
test('Otel db client span operation test', (t, end) => {
const { agent, tracer } = t.nr
const attributes = {
[SEMATTRS_DB_SYSTEM]: DbSystemValues.REDIS,
[SEMATTRS_DB_STATEMENT]: 'hset has random random',
[SEMATTRS_NET_PEER_PORT]: 5436,
[SEMATTRS_NET_PEER_NAME]: '127.0.0.1'
[ATTR_DB_SYSTEM]: DB_SYSTEM_VALUES.REDIS,
[ATTR_DB_STATEMENT]: 'hset has random random',
[ATTR_NET_PEER_PORT]: 5436,
[ATTR_NET_PEER_NAME]: '127.0.0.1'
}
const expectedHost = agent.config.getHostnameSafe('127.0.0.1')
helper.runInTransaction(agent, (tx) => {
Expand Down Expand Up @@ -189,3 +209,147 @@ test('Otel db client span operation test', (t, end) => {
})
})
})

test('http metrics are bridged correctly', (t, end) => {
const { agent, tracer } = t.nr

// Required span attributes for incoming HTTP server spans as defined by:
// https://opentelemetry.io/docs/specs/semconv/http/http-spans/#http-server-semantic-conventions
const attributes = {
[ATTR_URL_SCHEME]: 'http',
[ATTR_SERVER_ADDRESS]: 'newrelic.com',
[ATTR_HTTP_REQUEST_METHOD]: 'GET',
[ATTR_URL_PATH]: '/foo/bar',
[ATTR_HTTP_ROUTE]: '/foo/:param'
}

tracer.startActiveSpan('http-test', { kind: otel.SpanKind.SERVER, attributes }, (span) => {
const tx = agent.getTransaction()
const segment = agent.tracer.getSegment()
assert.equal(segment.name, 'WebTransaction/Nodejs/GET//foo/:param')
span.end()

const duration = hrTimeToMilliseconds(span.duration)
assert.equal(duration, segment.getDurationInMillis())
tx.end()

const attrs = segment.getAttributes()
assert.equal(attrs.host, 'newrelic.com')
assert.equal(attrs['http.request.method'], 'GET')
assert.equal(attrs['http.route'], '/foo/:param')
assert.equal(attrs['url.path'], '/foo/bar')
assert.equal(attrs['url.scheme'], 'http')
assert.equal(attrs.nr_exclusive_duration_millis, duration)

const unscopedMetrics = tx.metrics.unscoped
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me if there should be any scoped metrics. When I step through, there are not any.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes there are a lot of metrics getting created, some aren't complete because it's not propagating context from upstream if it exists(which I will add in my ticket) but here are some that need asserted:

Apdex, Apdex/<partialName> which is currently null(I have fixed this), HttpDispatcher, WebTransactionTotalTime, WebTransactionTotalTime/(i have fixed in my PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are present in the unscoped metrics. I can add assertions for them. But I get zero metrics in tx.metrics.scoped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry, yea there are no scoped metrics in this recorder. you can tell by looking at the recorder, the 2nd arg would be the scope and all are set to null

const expectedMetrics = [
'HttpDispatcher',
'WebTransaction',
'WebTransaction/Nodejs/GET//foo/:param',
'WebTransactionTotalTime',
'WebTransactionTotalTime/null',
segment.name
]
for (const expectedMetric of expectedMetrics) {
assert.equal(unscopedMetrics[expectedMetric].callCount, 1, `${expectedMetric} has correct callCount`)
}
assert.equal(unscopedMetrics.Apdex.apdexT, 0.1)
assert.equal(unscopedMetrics['Apdex/null'].apdexT, 0.1)

end()
})
})

test('rpc server metrics are bridged correctly', (t, end) => {
const { agent, tracer } = t.nr

// Required span attributes for incoming HTTP server spans as defined by:
// https://opentelemetry.io/docs/specs/semconv/rpc/rpc-spans/#client-attributes
const attributes = {
[ATTR_RPC_SYSTEM]: 'foo',
[ATTR_RPC_METHOD]: 'getData',
[ATTR_RPC_SERVICE]: 'test.service',
[ATTR_SERVER_ADDRESS]: 'newrelic.com',
[ATTR_URL_PATH]: '/foo/bar'
}

tracer.startActiveSpan('http-test', { kind: otel.SpanKind.SERVER, attributes }, (span) => {
const tx = agent.getTransaction()
const segment = agent.tracer.getSegment()
assert.equal(segment.name, 'WebTransaction/WebFrameworkUri/foo/test.service.getData')
span.end()

const duration = hrTimeToMilliseconds(span.duration)
assert.equal(duration, segment.getDurationInMillis())
tx.end()

const attrs = segment.getAttributes()
assert.equal(attrs.host, 'newrelic.com')
assert.equal(attrs['rpc.system'], 'foo')
assert.equal(attrs['rpc.method'], 'getData')
assert.equal(attrs['rpc.service'], 'test.service')
assert.equal(attrs['url.path'], '/foo/bar')
assert.equal(attrs.nr_exclusive_duration_millis, duration)

const unscopedMetrics = tx.metrics.unscoped
const expectedMetrics = [
'HttpDispatcher',
'WebTransaction',
'WebTransaction/WebFrameworkUri/foo/test.service.getData',
'WebTransactionTotalTime',
'WebTransactionTotalTime/null',
segment.name
]
for (const expectedMetric of expectedMetrics) {
assert.equal(unscopedMetrics[expectedMetric].callCount, 1, `${expectedMetric} has correct callCount`)
}
assert.equal(unscopedMetrics.Apdex.apdexT, 0.1)
assert.equal(unscopedMetrics['Apdex/null'].apdexT, 0.1)

end()
})
})

test('fallback metrics are bridged correctly', (t, end) => {
const { agent, tracer } = t.nr

const attributes = {
[ATTR_URL_SCHEME]: 'gopher',
[ATTR_SERVER_ADDRESS]: 'newrelic.com',
[ATTR_URL_PATH]: '/foo/bar',
}

tracer.startActiveSpan('http-test', { kind: otel.SpanKind.SERVER, attributes }, (span) => {
const tx = agent.getTransaction()
const segment = agent.tracer.getSegment()
assert.equal(segment.name, 'WebTransaction/NormalizedUri/*')
span.end()

const duration = hrTimeToMilliseconds(span.duration)
assert.equal(duration, segment.getDurationInMillis())
tx.end()

const attrs = segment.getAttributes()
assert.equal(attrs.host, 'newrelic.com')
assert.equal(attrs['url.path'], '/foo/bar')
assert.equal(attrs['url.scheme'], 'gopher')
assert.equal(attrs.nr_exclusive_duration_millis, duration)

const unscopedMetrics = tx.metrics.unscoped
const expectedMetrics = [
'HttpDispatcher',
'WebTransaction',
'WebTransaction/NormalizedUri/*',
'WebTransactionTotalTime',
'WebTransactionTotalTime/null',
segment.name
]
for (const expectedMetric of expectedMetrics) {
assert.equal(unscopedMetrics[expectedMetric].callCount, 1, `${expectedMetric} has correct callCount`)
}
assert.equal(unscopedMetrics.Apdex.apdexT, 0.1)
assert.equal(unscopedMetrics['Apdex/null'].apdexT, 0.1)

end()
})
})
Loading