Skip to content

Commit 2e337bd

Browse files
watsonwatson
authored
refactor(logging): use printf-style formatting (#7409)
Replace string concatenation and callback-style logging with printf-style formatting (%d, %s placeholders) for better performance and consistency. Merge multi-line concatenated strings into single strings with eslint max-len disables where needed to avoid unnecessary string operations.
1 parent 627ba54 commit 2e337bd

File tree

32 files changed

+296
-87
lines changed

32 files changed

+296
-87
lines changed

‎eslint-rules/eslint-log-printf-style.mjs‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
export default {
2+
meta: {
3+
type: 'problem',
4+
docs: {
5+
description:
6+
'Enforce printf-style formatting in log calls instead of string concatenation, template literals, or ' +
7+
'callbacks. Callback-style logging (e.g., log.debug(() => ...)) is ONLY allowed for expensive formatting ' +
8+
'operations where you need to avoid the overhead when the log level is disabled. In those rare cases, ' +
9+
'disable this rule with: // eslint-disable-next-line eslint-rules/eslint-log-printf-style',
10+
},
11+
messages: {
12+
useFormat: 'Use printf-style formatting (e.g., log.{{method}}("message %s", value)) instead of {{badPattern}}. ' +
13+
'Only use callback-style for expensive operations and disable this rule with a comment.',
14+
},
15+
schema: [],
16+
},
17+
create (context) {
18+
const LOG_METHODS = ['trace', 'debug', 'info', 'warn', 'error', 'errorWithoutTelemetry']
19+
20+
const isLogCall = (node) => {
21+
return node.type === 'CallExpression' &&
22+
node.callee.type === 'MemberExpression' &&
23+
node.callee.object.name === 'log' &&
24+
LOG_METHODS.includes(node.callee.property.name)
25+
}
26+
27+
const hasBinaryStringConcat = (node) => {
28+
if (node.type !== 'BinaryExpression' || node.operator !== '+') {
29+
return false
30+
}
31+
32+
// Check if either side is a string literal or another concatenation
33+
const leftIsString = node.left.type === 'Literal' && typeof node.left.value === 'string'
34+
const rightIsString = node.right.type === 'Literal' && typeof node.right.value === 'string'
35+
const leftIsConcat = hasBinaryStringConcat(node.left)
36+
const rightIsConcat = hasBinaryStringConcat(node.right)
37+
38+
return leftIsString || rightIsString || leftIsConcat || rightIsConcat
39+
}
40+
41+
return {
42+
CallExpression (node) {
43+
if (!isLogCall(node)) return
44+
if (node.arguments.length === 0) return
45+
46+
const firstArg = node.arguments[0]
47+
const methodName = node.callee.property.name
48+
49+
// Check for callback-style logging (arrow functions or function expressions)
50+
// NOTE: Callback-style is only acceptable for expensive formatting operations
51+
// where you need to avoid overhead when the log level is disabled.
52+
// Use: // eslint-disable-next-line eslint-rules/eslint-log-printf-style
53+
if (firstArg.type === 'ArrowFunctionExpression' || firstArg.type === 'FunctionExpression') {
54+
context.report({
55+
node: firstArg,
56+
messageId: 'useFormat',
57+
data: {
58+
method: methodName,
59+
badPattern: 'callback-style logging',
60+
},
61+
})
62+
return
63+
}
64+
65+
// Check for template literals with expressions
66+
if (firstArg.type === 'TemplateLiteral' && firstArg.expressions.length > 0) {
67+
context.report({
68+
node: firstArg,
69+
messageId: 'useFormat',
70+
data: {
71+
method: methodName,
72+
badPattern: 'template literals',
73+
},
74+
})
75+
return
76+
}
77+
78+
// Check for string concatenation with +
79+
if (hasBinaryStringConcat(firstArg)) {
80+
context.report({
81+
node: firstArg,
82+
messageId: 'useFormat',
83+
data: {
84+
method: methodName,
85+
badPattern: 'string concatenation',
86+
},
87+
})
88+
}
89+
},
90+
}
91+
},
92+
}

‎eslint-rules/eslint-log-printf-style.test.mjs‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { RuleTester } from 'eslint'
2+
import rule from './eslint-log-printf-style.mjs'
3+
4+
const ruleTester = new RuleTester({
5+
languageOptions: {
6+
ecmaVersion: 2022,
7+
sourceType: 'module',
8+
},
9+
})
10+
11+
ruleTester.run('eslint-log-printf-style', rule, {
12+
valid: [
13+
// Printf-style formatting
14+
{ code: 'log.debug("message %s", value)' },
15+
{ code: 'log.info("count: %d", count)' },
16+
{ code: 'log.warn("Error: %s with code %d", message, code)' },
17+
{ code: 'log.error("Failed with error %s", err)' },
18+
19+
// Simple string literals (no interpolation)
20+
{ code: 'log.debug("simple message")' },
21+
{ code: 'log.info("no variables here")' },
22+
23+
// Template literals without expressions
24+
{ code: 'log.debug(`simple template`)' },
25+
26+
// Not a log call
27+
{ code: 'console.log(`template ${value}`)' }, // eslint-disable-line no-template-curly-in-string
28+
{ code: 'foo.bar("test" + value)' },
29+
],
30+
31+
invalid: [
32+
// Callback-style logging
33+
{
34+
code: 'log.debug(() => "foo")',
35+
errors: [{
36+
messageId: 'useFormat',
37+
data: { method: 'debug', badPattern: 'callback-style logging' },
38+
}],
39+
},
40+
{
41+
code: 'log.info(() => `message ${value}`)', // eslint-disable-line no-template-curly-in-string
42+
errors: [{
43+
messageId: 'useFormat',
44+
data: { method: 'info', badPattern: 'callback-style logging' },
45+
}],
46+
},
47+
{
48+
code: 'log.warn(function() { return "message" })',
49+
errors: [{
50+
messageId: 'useFormat',
51+
data: { method: 'warn', badPattern: 'callback-style logging' },
52+
}],
53+
},
54+
55+
// Template literals with expressions
56+
{
57+
code: 'log.debug(`message ${value}`)', // eslint-disable-line no-template-curly-in-string
58+
errors: [{
59+
messageId: 'useFormat',
60+
data: { method: 'debug', badPattern: 'template literals' },
61+
}],
62+
},
63+
{
64+
// eslint-disable-next-line no-template-curly-in-string
65+
code: 'log.warn(`Error: ${err.message} with code ${err.code}`)',
66+
errors: [{
67+
messageId: 'useFormat',
68+
data: { method: 'warn', badPattern: 'template literals' },
69+
}],
70+
},
71+
72+
// String concatenation
73+
{
74+
code: 'log.error("Error: " + message)',
75+
errors: [{
76+
messageId: 'useFormat',
77+
data: { method: 'error', badPattern: 'string concatenation' },
78+
}],
79+
},
80+
{
81+
code: 'log.info("Count is " + count + " items")',
82+
errors: [{
83+
messageId: 'useFormat',
84+
data: { method: 'info', badPattern: 'string concatenation' },
85+
}],
86+
},
87+
{
88+
code: 'log.warn("Metric queue exceeded limit (max: " + max + "). Dropping " + dropped + " measurements.")',
89+
errors: [{
90+
messageId: 'useFormat',
91+
data: { method: 'warn', badPattern: 'string concatenation' },
92+
}],
93+
},
94+
],
95+
})

‎eslint.config.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import globals from 'globals'
1313
import eslintProcessEnv from './eslint-rules/eslint-process-env.mjs'
1414
import eslintEnvAliases from './eslint-rules/eslint-env-aliases.mjs'
1515
import eslintSafeTypeOfObject from './eslint-rules/eslint-safe-typeof-object.mjs'
16+
import eslintLogPrintfStyle from './eslint-rules/eslint-log-printf-style.mjs'
1617

1718
const { dependencies } = JSON.parse(readFileSync('./vendor/package.json', 'utf8'))
1819

@@ -410,6 +411,7 @@ export default [
410411
'eslint-process-env': eslintProcessEnv,
411412
'eslint-env-aliases': eslintEnvAliases,
412413
'eslint-safe-typeof-object': eslintSafeTypeOfObject,
414+
'eslint-log-printf-style': eslintLogPrintfStyle,
413415
},
414416
},
415417
n: eslintPluginN,
@@ -419,6 +421,7 @@ export default [
419421
'eslint-rules/eslint-process-env': 'error',
420422
'eslint-rules/eslint-env-aliases': 'error',
421423
'eslint-rules/eslint-safe-typeof-object': 'error',
424+
'eslint-rules/eslint-log-printf-style': 'error',
422425
'n/no-restricted-require': ['error', [
423426
{
424427
name: 'diagnostics_channel',

‎packages/datadog-instrumentations/src/confluentinc-kafka-javascript.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,10 @@ function instrumentKafkaJS (kafkaJS) {
234234
// This approach is implemented by other tracers as well.
235235
if (err.name === 'KafkaJSError' && err.type === 'ERR_UNKNOWN') {
236236
disabledHeaderWeakSet.add(producer)
237-
log.error('Kafka Broker responded with UNKNOWN_SERVER_ERROR (-1). ' +
238-
'Please look at broker logs for more information. ' +
239-
'Tracer message header injection for Kafka is disabled.')
237+
log.error(
238+
// eslint-disable-next-line @stylistic/max-len
239+
'Kafka Broker responded with UNKNOWN_SERVER_ERROR (-1). Please look at broker logs for more information. Tracer message header injection for Kafka is disabled.'
240+
)
240241
}
241242
ctx.error = err
242243
channels.producerError.publish(ctx)

‎packages/datadog-instrumentations/src/jest.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -776,9 +776,7 @@ function getTestEnvironment (pkg, jestVersion) {
776776
function applySuiteSkipping (originalTests, rootDir, frameworkVersion) {
777777
const jestSuitesToRun = getJestSuitesToRun(skippableSuites, originalTests, rootDir || process.cwd())
778778
hasFilteredSkippableSuites = true
779-
log.debug(
780-
() => `${jestSuitesToRun.suitesToRun.length} out of ${originalTests.length} suites are going to run.`
781-
)
779+
log.debug('%d out of %d suites are going to run.', jestSuitesToRun.suitesToRun.length, originalTests.length)
782780
hasUnskippableSuites = jestSuitesToRun.hasUnskippableSuites
783781
hasForcedToRunSuites = jestSuitesToRun.hasForcedToRunSuites
784782

‎packages/datadog-instrumentations/src/kafkajs.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ addHook({ name: 'kafkajs', file: 'src/index.js', versions: ['>=1.4'] }, (BaseKaf
9393
// This approach is implemented by other tracers as well.
9494
if (err.name === 'KafkaJSProtocolError' && err.type === 'UNKNOWN') {
9595
disabledHeaderWeakSet.add(producer)
96-
log.error('Kafka Broker responded with UNKNOWN_SERVER_ERROR (-1). ' +
97-
'Please look at broker logs for more information. ' +
98-
'Tracer message header injection for Kafka is disabled.')
96+
log.error(
97+
// eslint-disable-next-line @stylistic/max-len
98+
'Kafka Broker responded with UNKNOWN_SERVER_ERROR (-1). Please look at broker logs for more information. Tracer message header injection for Kafka is disabled.'
99+
)
99100
}
100101
producerErrorCh.publish(err)
101102
}

‎packages/datadog-instrumentations/src/mocha/main.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,7 @@ function getExecutionConfiguration (runner, isParallel, frameworkVersion, onFini
220220

221221
isSuitesSkipped = suitesToRun.length !== runner.suite.suites.length
222222

223-
log.debug(
224-
() => `${suitesToRun.length} out of ${runner.suite.suites.length} suites are going to run.`
225-
)
223+
log.debug('%d out of %d suites are going to run.', suitesToRun.length, runner.suite.suites.length)
226224

227225
runner.suite.suites = suitesToRun
228226

‎packages/datadog-plugin-google-cloud-pubsub/src/pubsub-push-subscription.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ class GoogleCloudPubsubPushSubscriptionPlugin extends TracingPlugin {
5353
}
5454

5555
log.warn(
56-
'[PubSub] No x-goog-pubsub-* headers detected. pubsub.push.receive spans will not be created. ' +
57-
'Add --push-no-wrapper-write-metadata to your subscription.'
56+
// eslint-disable-next-line @stylistic/max-len
57+
'[PubSub] No x-goog-pubsub-* headers detected. pubsub.push.receive spans will not be created. Add --push-no-wrapper-write-metadata to your subscription.'
5858
)
5959
return false
6060
}

‎packages/datadog-plugin-http/src/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class HttpPlugin extends CompositePlugin {
2323
plugins['pubsub-push-subscription'] = PushSubscriptionPlugin
2424
log.debug('Loaded GCP Pub/Sub Push Subscription plugin for HTTP requests')
2525
} catch (e) {
26-
log.debug(`Failed to load GCP Pub/Sub Push Subscription plugin: ${e.message}`)
26+
log.debug('Failed to load GCP Pub/Sub Push Subscription plugin:', e)
2727
}
2828
}
2929

‎packages/dd-trace/src/ci-visibility/exporters/agentless/coverage-writer.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class Writer extends BaseWriter {
4242
options.headers['X-Datadog-EVP-Subdomain'] = 'citestcov-intake'
4343
}
4444

45+
// eslint-disable-next-line eslint-rules/eslint-log-printf-style
4546
log.debug(() => `Request to the intake: ${safeJSONStringify(options)}`)
4647

4748
const startRequestTime = Date.now()

0 commit comments

Comments
 (0)