Skip to content

Commit fce4d92

Browse files
committed
fix log messages to be memoized and add tests for memoization
1 parent 8185395 commit fce4d92

File tree

4 files changed

+111
-24
lines changed

4 files changed

+111
-24
lines changed

packages/dd-trace/src/encode/0.4.js

+17-9
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const { Chunk, MsgpackEncoder } = require('../msgpack')
55
const log = require('../log')
66
const { isTrue } = require('../util')
77
const coalesce = require('koalas')
8+
const { memoize } = require('../log/utils')
89

910
const SOFT_LIMIT = 8 * 1024 * 1024 // 8MB
1011

@@ -339,13 +340,22 @@ class AgentEncoder {
339340
}
340341
}
341342

343+
const memoizedLogDebug = memoize((key, message) => {
344+
log.debug(message)
345+
// return something to store in memoize cache
346+
return true
347+
})
348+
342349
function formatSpanEvents (span) {
343350
for (const spanEvent of span.span_events) {
344351
if (spanEvent.attributes) {
345352
for (const [key, value] of Object.entries(spanEvent.attributes)) {
346353
const newValue = convertSpanEventAttributeValues(key, value)
347-
if (newValue !== undefined) spanEvent.attributes[key] = newValue
348-
else delete spanEvent.attributes[key] // delete from attributes if undefined
354+
if (newValue !== undefined) {
355+
spanEvent.attributes[key] = newValue
356+
} else {
357+
delete spanEvent.attributes[key] // delete from attributes if undefined
358+
}
349359
}
350360
if (Object.entries(spanEvent.attributes).length === 0) {
351361
delete spanEvent.attributes
@@ -388,20 +398,18 @@ function convertSpanEventAttributeValues (key, value, depth = 0) {
388398
array_value: convertedArray
389399
}
390400
} else {
391-
// If all elements were unsupported, return undefined
401+
// If all elements were unsupported, return undefined
392402
return undefined
393403
}
394404
} else {
395-
log.warn(
396-
`Encountered nested array data type for span event v0.4 encoding.
397-
Skipping encoding key: ${key}: with value: ${typeof value}.`
405+
memoizedLogDebug(key, 'Encountered nested array data type for span event v0.4 encoding. ' +
406+
`Skipping encoding key: ${key}: with value: ${typeof value}.`
398407
)
399408
return undefined
400409
}
401410
} else {
402-
log.warn(
403-
`Encountered unsupported data type for span event v0.4 encoding, key: ${key}: with value: ${typeof value}.
404-
Skipping encoding of pair.`
411+
memoizedLogDebug(key, 'Encountered unsupported data type for span event v0.4 encoding, key: ' +
412+
`${key}: with value: ${typeof value}. Skipping encoding of pair.`
405413
)
406414
return undefined
407415
}

packages/dd-trace/src/log/index.js

+1-13
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,7 @@ const { isTrue } = require('../util')
66
const { traceChannel, debugChannel, infoChannel, warnChannel, errorChannel } = require('./channels')
77
const logWriter = require('./writer')
88
const { Log } = require('./log')
9-
10-
const memoize = func => {
11-
const cache = {}
12-
const memoized = function (key) {
13-
if (!cache[key]) {
14-
cache[key] = func.apply(this, arguments)
15-
}
16-
17-
return cache[key]
18-
}
19-
20-
return memoized
21-
}
9+
const { memoize } = require('./utils')
2210

2311
const config = {
2412
enabled: false,

packages/dd-trace/src/log/utils.js

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
'use strict'
2+
3+
const memoize = func => {
4+
const cache = {}
5+
const memoized = function (key) {
6+
if (!cache[key]) {
7+
cache[key] = func.apply(this, arguments)
8+
}
9+
10+
return cache[key]
11+
}
12+
13+
return memoized
14+
}
15+
16+
module.exports = { memoize }

packages/dd-trace/test/encode/0.4.spec.js

+77-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ require('../setup/tap')
55
const { expect } = require('chai')
66
const msgpack = require('@msgpack/msgpack')
77
const id = require('../../src/id')
8+
const proxyquire = require('proxyquire')
9+
const sinon = require('sinon')
810

911
function randString (length) {
1012
return Array.from({ length }, () => {
@@ -23,7 +25,7 @@ describe('encode', () => {
2325
logger = {
2426
debug: sinon.stub()
2527
}
26-
const { AgentEncoder } = proxyquire('../src/encode/0.4', {
28+
const { AgentEncoder } = proxyquire('../../src/encode/0.4', {
2729
'../log': logger
2830
})
2931
writer = { flush: sinon.spy() }
@@ -474,8 +476,15 @@ describe('encode', () => {
474476
})
475477

476478
describe('with configuration', () => {
479+
let logger
480+
477481
beforeEach(() => {
478-
const { AgentEncoder } = proxyquire('../src/encode/0.4', {
482+
// Create a sinon spy for log.debug
483+
logger = {
484+
debug: sinon.spy()
485+
}
486+
487+
const { AgentEncoder } = proxyquire('../../src/encode/0.4', {
479488
'../log': logger
480489
})
481490
writer = { flush: sinon.spy(), _config: { trace: { nativeSpanEvents: true } } }
@@ -565,5 +574,71 @@ describe('encode', () => {
565574

566575
expect(trace[0].span_events).to.deep.equal(formattedTopLevelEvent)
567576
})
577+
578+
it('should call log.debug only once for the same unsupported key', () => {
579+
const topLevelEvents = [
580+
{
581+
name: 'Event 1',
582+
time_unix_nano: 1000000,
583+
attributes: { unsupported_key: { some: 'object' }, other_key: 'valid' }
584+
},
585+
{
586+
name: 'Event 2',
587+
time_unix_nano: 2000000,
588+
attributes: { unsupported_key: { another: 'object' } }
589+
},
590+
{
591+
name: 'Event 3',
592+
time_unix_nano: 3000000,
593+
attributes: { unsupported_key: { yet: 'another object' } }
594+
},
595+
{
596+
name: 'Event 4',
597+
time_unix_nano: 4000000,
598+
attributes: { unsupported_key: { different: 'structure' } }
599+
}
600+
]
601+
602+
data[0].span_events = topLevelEvents
603+
604+
encoder.encode(data)
605+
606+
// Assert that log.debug was called only once for 'unsupported_key'
607+
sinon.assert.calledOnce(logger.debug)
608+
sinon.assert.calledWith(
609+
logger.debug,
610+
sinon.match(/Encountered unsupported data type for span event v0\.4 encoding, key: unsupported_key/)
611+
)
612+
})
613+
614+
it('should call log.debug once per unique unsupported key', () => {
615+
const topLevelEvents = [
616+
{
617+
name: 'Event 1',
618+
time_unix_nano: 1000000,
619+
attributes: { unsupported_key1: { some: 'object' }, unsupported_key2: { another: 'object' } }
620+
},
621+
{
622+
name: 'Event 2',
623+
time_unix_nano: 2000000,
624+
attributes: { unsupported_key1: { different: 'structure' }, unsupported_key3: { more: 'objects' } }
625+
},
626+
{
627+
name: 'Event 3',
628+
time_unix_nano: 3000000,
629+
attributes: { unsupported_key2: { yet: 'another object' }, unsupported_key3: { extra: 'data' } }
630+
}
631+
]
632+
633+
data[0].span_events = topLevelEvents
634+
635+
encoder.encode(data)
636+
637+
// Assert that log.debug was called once for each unique unsupported key
638+
expect(logger.debug.callCount).to.equal(3)
639+
expect(logger.debug.getCall(0).args[0]).to.match(/unsupported_key1/)
640+
expect(logger.debug.getCall(1).args[0]).to.match(/unsupported_key2/)
641+
expect(logger.debug.getCall(2).args[0]).to.match(/unsupported_key3/)
642+
})
568643
})
569644
})

0 commit comments

Comments
 (0)