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.

Sign up for GitHub

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

Update Otel Invalid Telemetry Calculation #4369

Merged
merged 4 commits into from
Jul 8, 2024
Merged

Update Otel Invalid Telemetry Calculation #4369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
45c1006
update otel invalid telemetry calculation
khanayan123 Jun 3, 2024
eee08f2
fix lint
khanayan123 Jun 3, 2024
2d6993f
update tag names
khanayan123 Jun 24, 2024
502b03c
Merge branch 'master' into khanayan123/update-otel-invalid-telemetry-…
khanayan123 Jun 24, 2024
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
123 changes: 103 additions & 20 deletions integration-tests/opentelemetry.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ describe('opentelemetry', () => {
assert.strictEqual(payload.request_type, 'generate-metrics')

const metrics = payload.payload

assert.strictEqual(metrics.namespace, 'tracers')

const otelHiding = metrics.series.filter(({ metric }) => metric === 'otel.env.hiding')
Expand All @@ -107,7 +106,7 @@ describe('opentelemetry', () => {
}, true)
})

it('should capture telemetry if both DD and OTEL env vars are set ', () => {
it('should capture telemetry if both DD and OTEL env vars are set', () => {
proc = fork(join(cwd, 'opentelemetry/basic.js'), {
cwd,
env: {
Expand All @@ -121,15 +120,15 @@ describe('opentelemetry', () => {
OTEL_LOG_LEVEL: 'debug',
DD_TRACE_SAMPLE_RATE: '0.5',
OTEL_TRACES_SAMPLER: 'traceidratio',
OTEL_TRACES_SAMPLER_ARG: '0.1',
OTEL_TRACES_SAMPLER_ARG: '1.0',
Copy link

Choose a reason for hiding this comment

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

If I'm understanding this correctly, we're incrementing the otelHiding.length in the test because now we consider the OTEL_TRACES_SAMPLER_ARG config as another config that's being overlooked by its corresponding Datadog config, DD_TRACE_SAMPLE_RATE?

Copy link
Collaborator Author

@khanayan123 khanayan123 Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, since they both map to DD_TRACE_SAMPLE_RATE it makes more sense to increment them as seperate pairs of tags. since a user could have inputted both an invalid OTEL_TRACES_SAMPLER value & and an invalid OTEL_TRACES_SAMPLER_ARG value or either or. This makes it more granular.

DD_TRACE_ENABLED: 'true',
OTEL_TRACES_EXPORTER: 'none',
DD_RUNTIME_METRICS_ENABLED: 'true',
OTEL_METRICS_EXPORTER: 'none',
DD_TAGS: 'foo:bar,baz:qux',
OTEL_RESOURCE_ATTRIBUTES: 'foo=bar1,baz=qux1',
DD_TRACE_PROPAGATION_STYLE: 'datadog',
OTEL_PROPAGATORS: 'datadog,tracecontext',
OTEL_RESOURCE_ATTRIBUTES: 'foo+bar13baz+qux1',
Copy link

Choose a reason for hiding this comment

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

If I'm understanding this correctly, we're no longer considering this as "invalid" because we don't have a set of recognized values, so that's why the otelInvalid.length went down?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes precisely, I think that should also be the expected behavior cross tracers.

DD_TRACE_PROPAGATION_STYLE: 'datadog, tracecontext',
OTEL_PROPAGATORS: 'datadog, tracecontext',
OTEL_LOGS_EXPORTER: 'none',
OTEL_SDK_DISABLED: 'false'
}
Expand All @@ -144,58 +143,142 @@ describe('opentelemetry', () => {

const otelHiding = metrics.series.filter(({ metric }) => metric === 'otel.env.hiding')
const otelInvalid = metrics.series.filter(({ metric }) => metric === 'otel.env.invalid')

assert.strictEqual(otelHiding.length, 8)
assert.strictEqual(otelInvalid.length, 1)
assert.strictEqual(otelHiding.length, 9)
assert.strictEqual(otelInvalid.length, 0)

assert.deepStrictEqual(otelHiding[0].tags, [
'DD_TRACE_LOG_LEVEL', 'OTEL_LOG_LEVEL',
'config.datadog:DD_TRACE_LOG_LEVEL', 'config.opentelemetry:OTEL_LOG_LEVEL',
`version:${process.version}`
])
assert.deepStrictEqual(otelHiding[1].tags, [
'DD_TRACE_PROPAGATION_STYLE', 'OTEL_PROPAGATORS',
'config.datadog:DD_TRACE_PROPAGATION_STYLE', 'config.opentelemetry:OTEL_PROPAGATORS',
`version:${process.version}`
])
assert.deepStrictEqual(otelHiding[2].tags, [
'DD_SERVICE', 'OTEL_SERVICE_NAME',
'config.datadog:DD_SERVICE', 'config.opentelemetry:OTEL_SERVICE_NAME',
`version:${process.version}`
])

assert.deepStrictEqual(otelHiding[3].tags, [
'DD_TRACE_SAMPLE_RATE', 'OTEL_TRACES_SAMPLER',
'OTEL_TRACES_SAMPLER_ARG', `version:${process.version}`
'config.datadog:DD_TRACE_SAMPLE_RATE', 'config.opentelemetry:OTEL_TRACES_SAMPLER', `version:${process.version}`
])

assert.deepStrictEqual(otelHiding[4].tags, [
'DD_TRACE_ENABLED', 'OTEL_TRACES_EXPORTER',
'config.datadog:DD_TRACE_SAMPLE_RATE', 'config.opentelemetry:OTEL_TRACES_SAMPLER_ARG',
`version:${process.version}`
])

assert.deepStrictEqual(otelHiding[5].tags, [
'DD_RUNTIME_METRICS_ENABLED', 'OTEL_METRICS_EXPORTER',
'config.datadog:DD_TRACE_ENABLED', 'config.opentelemetry:OTEL_TRACES_EXPORTER',
`version:${process.version}`
])

assert.deepStrictEqual(otelHiding[6].tags, [
'DD_TAGS', 'OTEL_RESOURCE_ATTRIBUTES',
'config.datadog:DD_RUNTIME_METRICS_ENABLED', 'config.opentelemetry:OTEL_METRICS_EXPORTER',
`version:${process.version}`
])

assert.deepStrictEqual(otelHiding[7].tags, [
'DD_TRACE_OTEL_ENABLED', 'OTEL_SDK_DISABLED',
'config.datadog:DD_TAGS', 'config.opentelemetry:OTEL_RESOURCE_ATTRIBUTES',
`version:${process.version}`
])

assert.deepStrictEqual(otelHiding[8].tags, [
'config.datadog:DD_TRACE_OTEL_ENABLED', 'config.opentelemetry:OTEL_SDK_DISABLED',
`version:${process.version}`
])

for (const metric of otelHiding) {
assert.strictEqual(metric.points[0][1], 1)
}
}, true)
})

it('should capture telemetry when OTEL env vars are invalid', () => {
proc = fork(join(cwd, 'opentelemetry/basic.js'), {
cwd,
env: {
DD_TRACE_AGENT_PORT: agent.port,
DD_TRACE_OTEL_ENABLED: 1,
DD_TELEMETRY_HEARTBEAT_INTERVAL: 1,
TIMEOUT: 1500,
OTEL_SERVICE_NAME: 'otel_service',
OTEL_LOG_LEVEL: 'foo',
OTEL_TRACES_SAMPLER: 'foo',
OTEL_TRACES_SAMPLER_ARG: 'foo',
OTEL_TRACES_EXPORTER: 'foo',
OTEL_METRICS_EXPORTER: 'foo',
OTEL_RESOURCE_ATTRIBUTES: 'foo',
OTEL_PROPAGATORS: 'foo',
OTEL_LOGS_EXPORTER: 'foo',
OTEL_SDK_DISABLED: 'foo'
}
})

return check(agent, proc, timeout, ({ payload }) => {
assert.strictEqual(payload.request_type, 'generate-metrics')

const metrics = payload.payload

assert.strictEqual(metrics.namespace, 'tracers')

const otelHiding = metrics.series.filter(({ metric }) => metric === 'otel.env.hiding')
const otelInvalid = metrics.series.filter(({ metric }) => metric === 'otel.env.invalid')

assert.deepStrictEqual(otelInvalid[0].points[0][1], 1)
assert.strictEqual(otelHiding.length, 1)
assert.strictEqual(otelInvalid.length, 8)

assert.deepStrictEqual(otelHiding[0].tags, [
'config.datadog:DD_TRACE_OTEL_ENABLED', 'config.opentelemetry:OTEL_SDK_DISABLED',
`version:${process.version}`
])

assert.deepStrictEqual(otelInvalid[0].tags, [
'OTEL_LOGS_EXPORTER',
'config.datadog:DD_TRACE_LOG_LEVEL', 'config.opentelemetry:OTEL_LOG_LEVEL',
`version:${process.version}`
])

assert.deepStrictEqual(otelInvalid[1].tags, [
'config.datadog:DD_TRACE_SAMPLE_RATE',
'config.opentelemetry:OTEL_TRACES_SAMPLER',
`version:${process.version}`
])

assert.deepStrictEqual(otelInvalid[2].tags, [
'config.datadog:DD_TRACE_SAMPLE_RATE',
'config.opentelemetry:OTEL_TRACES_SAMPLER_ARG',
`version:${process.version}`
])
assert.deepStrictEqual(otelInvalid[3].tags, [
'config.datadog:DD_TRACE_ENABLED', 'config.opentelemetry:OTEL_TRACES_EXPORTER',
`version:${process.version}`
])

assert.deepStrictEqual(otelInvalid[4].tags, [
'config.datadog:DD_RUNTIME_METRICS_ENABLED',
'config.opentelemetry:OTEL_METRICS_EXPORTER',
`version:${process.version}`
])

assert.deepStrictEqual(otelInvalid[5].tags, [
'config.datadog:DD_TRACE_OTEL_ENABLED', 'config.opentelemetry:OTEL_SDK_DISABLED',
`version:${process.version}`
])

assert.deepStrictEqual(otelInvalid[6].tags, [
'config.opentelemetry:OTEL_LOGS_EXPORTER',
`version:${process.version}`
])

assert.deepStrictEqual(otelInvalid[7].tags, [
'config.datadog:DD_TRACE_PROPAGATION_STYLE',
'config.opentelemetry:OTEL_PROPAGATORS',
`version:${process.version}`
])

for (const metric of otelInvalid) {
assert.strictEqual(metric.points[0][1], 1)
}
}, true)
})

Expand Down
126 changes: 88 additions & 38 deletions packages/dd-trace/src/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,50 +26,104 @@ const telemetryCounters = {
'otel.env.invalid': {}
}

function getCounter (event, ddVar, otelVar, otelTracesSamplerArg) {
function getCounter (event, ddVar, otelVar) {
const counters = telemetryCounters[event]
const tags = []
const ddVarPrefix = 'config.datadog:'
const otelVarPrefix = 'config.opentelemetry:'
if (ddVar) {
ddVar = ddVarPrefix + ddVar
tags.push(ddVar)
}
if (otelVar) {
otelVar = otelVarPrefix + otelVar
tags.push(otelVar)
}

if (ddVar) tags.push(ddVar)
if (otelVar) tags.push(otelVar)
if (otelTracesSamplerArg) tags.push(otelTracesSamplerArg)

if (!(ddVar in counters)) counters[ddVar] = {}
if (!(otelVar in counters)) counters[otelVar] = {}

const counter = tracerMetrics.count(event, tags)
counters[ddVar][otelVar] = counter
counters[otelVar][ddVar] = counter
return counter
}

const otelDdEnvMapping = {
DD_TRACE_LOG_LEVEL: 'OTEL_LOG_LEVEL',
DD_TRACE_PROPAGATION_STYLE: 'OTEL_PROPAGATORS',
DD_SERVICE: 'OTEL_SERVICE_NAME',
DD_TRACE_SAMPLE_RATE: 'OTEL_TRACES_SAMPLER',
DD_TRACE_ENABLED: 'OTEL_TRACES_EXPORTER',
DD_RUNTIME_METRICS_ENABLED: 'OTEL_METRICS_EXPORTER',
DD_TAGS: 'OTEL_RESOURCE_ATTRIBUTES',
DD_TRACE_OTEL_ENABLED: 'OTEL_SDK_DISABLED'
OTEL_LOG_LEVEL: 'DD_TRACE_LOG_LEVEL',
OTEL_PROPAGATORS: 'DD_TRACE_PROPAGATION_STYLE',
OTEL_SERVICE_NAME: 'DD_SERVICE',
OTEL_TRACES_SAMPLER: 'DD_TRACE_SAMPLE_RATE',
OTEL_TRACES_SAMPLER_ARG: 'DD_TRACE_SAMPLE_RATE',
OTEL_TRACES_EXPORTER: 'DD_TRACE_ENABLED',
OTEL_METRICS_EXPORTER: 'DD_RUNTIME_METRICS_ENABLED',
OTEL_RESOURCE_ATTRIBUTES: 'DD_TAGS',
OTEL_SDK_DISABLED: 'DD_TRACE_OTEL_ENABLED',
OTEL_LOGS_EXPORTER: undefined
}

const otelInvalidEnv = ['OTEL_LOGS_EXPORTER']
const VALID_PROPAGATION_STYLES = new Set(['datadog', 'tracecontext', 'b3', 'b3 single header', 'none'])

function checkIfBothOtelAndDdEnvVarSet () {
for (const [ddVar, otelVar] of Object.entries(otelDdEnvMapping)) {
if (process.env[ddVar] && process.env[otelVar]) {
log.warn(`both ${ddVar} and ${otelVar} environment variables are set`)
getCounter('otel.env.hiding', ddVar, otelVar,
otelVar === 'OTEL_TRACES_SAMPLER' &&
process.env.OTEL_TRACES_SAMPLER_ARG
? 'OTEL_TRACES_SAMPLER_ARG'
: undefined).inc()
const VALID_LOG_LEVELS = new Set(['debug', 'info', 'warn', 'error'])

function getFromOtelSamplerMap (otelTracesSampler, otelTracesSamplerArg) {
const OTEL_TRACES_SAMPLER_MAPPING = {
always_on: '1.0',
always_off: '0.0',
traceidratio: otelTracesSamplerArg,
parentbased_always_on: '1.0',
parentbased_always_off: '0.0',
parentbased_traceidratio: otelTracesSamplerArg
}
return OTEL_TRACES_SAMPLER_MAPPING[otelTracesSampler]
}

function validateOtelPropagators (propagators) {
if (!process.env.PROPAGATION_STYLE_EXTRACT &&
!process.env.PROPAGATION_STYLE_INJECT &&
!process.env.DD_TRACE_PROPAGATION_STYLE &&
process.env.OTEL_PROPAGATORS) {
for (const style in propagators) {
if (!VALID_PROPAGATION_STYLES.has(style)) {
log.warn('unexpected value for OTEL_PROPAGATORS environment variable')
getCounter('otel.env.invalid', 'DD_TRACE_PROPAGATION_STYLE', 'OTEL_PROPAGATORS').inc()
}
}
}
}

for (const otelVar of otelInvalidEnv) {
if (process.env[otelVar]) {
log.warn(`${otelVar} is not supported by the Datadog SDK`)
getCounter('otel.env.invalid', otelVar).inc()
function validateEnvVarType (envVar) {
const value = process.env[envVar]
switch (envVar) {
case 'OTEL_LOG_LEVEL':
return VALID_LOG_LEVELS.has(value)
case 'OTEL_PROPAGATORS':
case 'OTEL_RESOURCE_ATTRIBUTES':
case 'OTEL_SERVICE_NAME':
return typeof value === 'string'
case 'OTEL_TRACES_SAMPLER':
return getFromOtelSamplerMap(value, process.env.OTEL_TRACES_SAMPLER_ARG) !== undefined
case 'OTEL_TRACES_SAMPLER_ARG':
return !isNaN(parseFloat(value))
case 'OTEL_SDK_DISABLED':
return value.toLowerCase() === 'true' || value.toLowerCase() === 'false'
case 'OTEL_TRACES_EXPORTER':
case 'OTEL_METRICS_EXPORTER':
case 'OTEL_LOGS_EXPORTER':
return value.toLowerCase() === 'none'
default:
return false
}
}

function checkIfBothOtelAndDdEnvVarSet () {
for (const [otelEnvVar, ddEnvVar] of Object.entries(otelDdEnvMapping)) {
if (ddEnvVar && process.env[ddEnvVar] && process.env[otelEnvVar]) {
log.warn(`both ${ddEnvVar} and ${otelEnvVar} environment variables are set`)
getCounter('otel.env.hiding', ddEnvVar, otelEnvVar).inc()
}

if (process.env[otelEnvVar] && !validateEnvVarType(otelEnvVar)) {
log.warn(`unexpected value for ${otelEnvVar} environment variable`)
getCounter('otel.env.invalid', ddEnvVar, otelEnvVar).inc()
}
}
}
Expand Down Expand Up @@ -235,6 +289,9 @@ class Config {
options.tracePropagationStyle,
defaultPropagationStyle
)

validateOtelPropagators(PROPAGATION_STYLE_INJECT)

const DD_TRACE_PROPAGATION_EXTRACT_FIRST = coalesce(
process.env.DD_TRACE_PROPAGATION_EXTRACT_FIRST,
false
Expand Down Expand Up @@ -715,15 +772,8 @@ class Config {
: undefined
this._setBoolean(env, 'runtimeMetrics', DD_RUNTIME_METRICS_ENABLED ||
otelSetRuntimeMetrics)
const OTEL_TRACES_SAMPLER_MAPPING = {
always_on: '1.0',
always_off: '0.0',
traceidratio: OTEL_TRACES_SAMPLER_ARG,
parentbased_always_on: '1.0',
parentbased_always_off: '0.0',
parentbased_traceidratio: OTEL_TRACES_SAMPLER_ARG
}
this._setUnit(env, 'sampleRate', DD_TRACE_SAMPLE_RATE || OTEL_TRACES_SAMPLER_MAPPING[OTEL_TRACES_SAMPLER])
this._setUnit(env, 'sampleRate', DD_TRACE_SAMPLE_RATE ||
getFromOtelSamplerMap(OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG))
this._setValue(env, 'sampler.rateLimit', DD_TRACE_RATE_LIMIT)
this._setSamplingRule(env, 'sampler.rules', safeJsonParse(DD_TRACE_SAMPLING_RULES))
this._envUnprocessed['sampler.rules'] = DD_TRACE_SAMPLING_RULES
Expand Down
Loading