Skip to content

Commit f49e6ff

Browse files
authored
Remove legacyStorage in favor of namespaced storages (#5206)
1 parent cde9361 commit f49e6ff

File tree

123 files changed

+480
-442
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

123 files changed

+480
-442
lines changed

packages/datadog-core/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use strict'
22

3-
const storage = require('./src/storage')
3+
const { storage } = require('./src/storage')
44

55
module.exports = { storage }

packages/datadog-core/src/storage.js

+68-38
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,54 @@
22

33
const { AsyncLocalStorage } = require('async_hooks')
44

5-
/// This is exactly the same as AsyncLocalStorage, with the exception that it
6-
/// uses a WeakMap to store the store object. This is because ALS stores the
7-
/// store object as a property of the resource object, which causes all sorts
8-
/// of problems with logging and memory. We substitute the `store` object with
9-
/// a "handle" object, which is used as a key in a WeakMap, where the values
10-
/// are the real store objects.
5+
/**
6+
* This is exactly the same as AsyncLocalStorage, with the exception that it
7+
* uses a WeakMap to store the store object. This is because ALS stores the
8+
* store object as a property of the resource object, which causes all sorts
9+
* of problems with logging and memory. We substitute the `store` object with
10+
* a "handle" object, which is used as a key in a WeakMap, where the values
11+
* are the real store objects.
12+
*
13+
* @template T
14+
*/
1115
class DatadogStorage extends AsyncLocalStorage {
16+
/**
17+
*
18+
* @param store {DatadogStorage}
19+
*/
1220
enterWith (store) {
1321
const handle = {}
1422
stores.set(handle, store)
1523
super.enterWith(handle)
1624
}
1725

18-
// This is method is a passthrough to the real `getStore()`, so that, when we
19-
// need it, we can use the handle rather than our mapped store.
20-
// TODO: Refactor the Scope class to use a span-only store and remove this.
21-
// It's only here because stores are currently used for a bunch of things,
22-
// and we don't want to hold on to all of them in spans
23-
// (see opentracing/span.js). Using a namespaced storage for spans would
24-
// solve this.
26+
/**
27+
* This is method is a passthrough to the real `getStore()`, so that, when we
28+
* need it, we can use the handle rather than our mapped store.
29+
*
30+
* It's only here because stores are currently used for a bunch of things,
31+
* and we don't want to hold on to all of them in spans
32+
* (see opentracing/span.js). Using a namespaced storage for spans would
33+
* solve this.
34+
*
35+
* TODO: Refactor the Scope class to use a span-only store and remove this.
36+
*
37+
* @returns {{}}
38+
*/
2539
getHandle () {
2640
return super.getStore()
2741
}
2842

29-
// Here, we replicate the behavior of the original `getStore()` method by
30-
// passing in the handle, which we retrieve by calling it on super. Handles
31-
// retrieved through `getHandle()` can also be passed in to be used as the
32-
// key. This is useful if you've stashed a handle somewhere and want to
33-
// retrieve the store with it.
43+
/**
44+
* Here, we replicate the behavior of the original `getStore()` method by
45+
* passing in the handle, which we retrieve by calling it on super. Handles
46+
* retrieved through `getHandle()` can also be passed in to be used as the
47+
* key. This is useful if you've stashed a handle somewhere and want to
48+
* retrieve the store with it.
49+
*
50+
* @param handle {{}}
51+
* @returns {T | undefined}
52+
*/
3453
getStore (handle) {
3554
if (!handle) {
3655
handle = super.getStore()
@@ -39,11 +58,19 @@ class DatadogStorage extends AsyncLocalStorage {
3958
return stores.get(handle)
4059
}
4160

42-
// Here, we replicate the behavior of the original `run()` method. We ensure
43-
// that our `enterWith()` is called internally, so that the handle to the
44-
// store is set. As an optimization, we use super for getStore and enterWith
45-
// when dealing with the parent store, so that we don't have to access the
46-
// WeakMap.
61+
/**
62+
* Here, we replicate the behavior of the original `run()` method. We ensure
63+
* that our `enterWith()` is called internally, so that the handle to the
64+
* store is set. As an optimization, we use super for getStore and enterWith
65+
* when dealing with the parent store, so that we don't have to access the
66+
* WeakMap.
67+
* @template R
68+
* @template TArgs extends any[]
69+
* @param store {DatadogStorage}
70+
* @param fn {() => R}
71+
* @param args {TArgs}
72+
* @returns {void}
73+
*/
4774
run (store, fn, ...args) {
4875
const prior = super.getStore()
4976
this.enterWith(store)
@@ -55,28 +82,31 @@ class DatadogStorage extends AsyncLocalStorage {
5582
}
5683
}
5784

58-
// This is the map from handles to real stores, used in the class above.
85+
/**
86+
* This is the map from handles to real stores, used in the class above.
87+
* @template T
88+
* @type {WeakMap<WeakKey, T>}
89+
*/
5990
const stores = new WeakMap()
6091

61-
// For convenience, we use the `storage` function as a registry of namespaces
62-
// corresponding to DatadogStorage instances. This lets us have separate
63-
// storages for separate purposes.
92+
/**
93+
* For convenience, we use the `storage` function as a registry of namespaces
94+
* corresponding to DatadogStorage instances. This lets us have separate
95+
* storages for separate purposes.
96+
* @type {Map<string, DatadogStorage>}
97+
*/
6498
const storages = Object.create(null)
99+
100+
/**
101+
*
102+
* @param namespace {string} the namespace to use
103+
* @returns {DatadogStorage}
104+
*/
65105
function storage (namespace) {
66106
if (!storages[namespace]) {
67107
storages[namespace] = new DatadogStorage()
68108
}
69109
return storages[namespace]
70110
}
71111

72-
// Namespaces are a new concept, so for existing internal code that does not
73-
// use namespaces, we have a "legacy" storage object.
74-
const legacyStorage = new DatadogStorage()
75-
storage.disable = legacyStorage.disable.bind(legacyStorage)
76-
storage.enterWith = legacyStorage.enterWith.bind(legacyStorage)
77-
storage.exit = legacyStorage.exit.bind(legacyStorage)
78-
storage.getHandle = legacyStorage.getHandle.bind(legacyStorage)
79-
storage.getStore = legacyStorage.getStore.bind(legacyStorage)
80-
storage.run = legacyStorage.run.bind(legacyStorage)
81-
82-
module.exports = storage
112+
module.exports = { storage }

packages/datadog-core/test/storage.spec.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@ describe('storage', () => {
1616
})
1717

1818
afterEach(() => {
19-
testStorage.enterWith(undefined)
19+
testStorage('legacy').enterWith(undefined)
2020
testStorage2.enterWith(undefined)
2121
})
2222

2323
it('should enter a store', done => {
2424
const store = 'foo'
2525

26-
testStorage.enterWith(store)
26+
testStorage('legacy').enterWith(store)
2727

2828
setImmediate(() => {
29-
expect(testStorage.getStore()).to.equal(store)
29+
expect(testStorage('legacy').getStore()).to.equal(store)
3030
done()
3131
})
3232
})
@@ -35,11 +35,11 @@ describe('storage', () => {
3535
const store = 'foo'
3636
const store2 = 'bar'
3737

38-
testStorage.enterWith(store)
38+
testStorage('legacy').enterWith(store)
3939
testStorage2.enterWith(store2)
4040

4141
setImmediate(() => {
42-
expect(testStorage.getStore()).to.equal(store)
42+
expect(testStorage('legacy').getStore()).to.equal(store)
4343
expect(testStorage2.getStore()).to.equal(store2)
4444
done()
4545
})
@@ -52,7 +52,7 @@ describe('storage', () => {
5252
it('should not have its store referenced by the underlying async resource', () => {
5353
const resource = executionAsyncResource()
5454

55-
testStorage.enterWith({ internal: 'internal' })
55+
testStorage('legacy').enterWith({ internal: 'internal' })
5656

5757
for (const sym of Object.getOwnPropertySymbols(resource)) {
5858
if (sym.toString() === 'Symbol(kResourceStore)' && resource[sym]) {

packages/datadog-instrumentations/test/body-parser.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ withVersions('body-parser', 'body-parser', version => {
7777
let payload
7878

7979
function handler (data) {
80-
store = storage.getStore()
80+
store = storage('legacy').getStore()
8181
payload = data
8282
}
8383
bodyParserReadCh.subscribe(handler)

packages/datadog-instrumentations/test/generic-pool.spec.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ describe('Instrumentation', () => {
2727
it('should run the acquire() callback in context where acquire() was called', done => {
2828
const store = 'store'
2929

30-
storage.run(store, () => {
30+
storage('legacy').run(store, () => {
3131
// eslint-disable-next-line n/handle-callback-err
3232
pool.acquire((err, resource) => {
3333
pool.release(resource)
34-
expect(storage.getStore()).to.equal(store)
34+
expect(storage('legacy').getStore()).to.equal(store)
3535
done()
3636
})
3737
})
@@ -56,20 +56,20 @@ describe('Instrumentation', () => {
5656
const store = 'store'
5757
const store2 = 'store2'
5858

59-
storage.run(store, () => {
59+
storage('legacy').run(store, () => {
6060
pool.acquire()
6161
.then(resource => {
6262
pool.release(resource)
63-
expect(storage.getStore()).to.equal(store)
63+
expect(storage('legacy').getStore()).to.equal(store)
6464
})
6565
.catch(done)
6666
})
6767

68-
storage.run(store2, () => {
68+
storage('legacy').run(store2, () => {
6969
pool.acquire()
7070
.then(resource => {
7171
pool.release(resource)
72-
expect(storage.getStore()).to.equal(store2)
72+
expect(storage('legacy').getStore()).to.equal(store2)
7373
done()
7474
})
7575
.catch(done)

packages/datadog-instrumentations/test/helpers/instrument.spec.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ const { AsyncResource } = require('../../src/helpers/instrument')
1010
describe('helpers/instrument', () => {
1111
describe('AsyncResource', () => {
1212
it('should bind statically', () => {
13-
storage.run('test1', () => {
13+
storage('legacy').run('test1', () => {
1414
const tested = AsyncResource.bind(() => {
15-
expect(storage.getStore()).to.equal('test1')
15+
expect(storage('legacy').getStore()).to.equal('test1')
1616
})
1717

18-
storage.run('test2', () => {
18+
storage('legacy').run('test2', () => {
1919
tested()
2020
})
2121
})
@@ -34,12 +34,12 @@ describe('helpers/instrument', () => {
3434
})
3535

3636
it('should bind a specific instance', () => {
37-
storage.run('test1', () => {
37+
storage('legacy').run('test1', () => {
3838
const asyncResource = new AsyncResource('test')
3939

40-
storage.run('test2', () => {
40+
storage('legacy').run('test2', () => {
4141
const tested = asyncResource.bind((a, b, c) => {
42-
expect(storage.getStore()).to.equal('test1')
42+
expect(storage('legacy').getStore()).to.equal('test1')
4343
expect(test.asyncResource).to.equal(asyncResource)
4444
expect(test).to.have.length(3)
4545
})

packages/datadog-instrumentations/test/helpers/promise.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,18 @@ module.exports = (name, factory, versionRange) => {
3232

3333
let promise = new Promise((resolve, reject) => {
3434
setImmediate(() => {
35-
storage.run('promise', () => {
35+
storage('legacy').run('promise', () => {
3636
resolve()
3737
})
3838
})
3939
})
4040

41-
storage.run(store, () => {
41+
storage('legacy').run(store, () => {
4242
for (let i = 0; i < promise.then.length; i++) {
4343
const args = new Array(i + 1)
4444

4545
args[i] = () => {
46-
expect(storage.getStore()).to.equal(store)
46+
expect(storage('legacy').getStore()).to.equal(store)
4747
}
4848

4949
promise = promise.then.apply(promise, args)
@@ -54,31 +54,31 @@ module.exports = (name, factory, versionRange) => {
5454
})
5555

5656
it('should run the catch() callback in the context where catch() was called', () => {
57-
const store = storage.getStore()
57+
const store = storage('legacy').getStore()
5858

5959
let promise = new Promise((resolve, reject) => {
6060
setImmediate(() => {
61-
storage.run('promise', () => {
61+
storage('legacy').run('promise', () => {
6262
reject(new Error())
6363
})
6464
})
6565
})
6666

67-
storage.run(store, () => {
67+
storage('legacy').run(store, () => {
6868
promise = promise
6969
.catch(err => {
7070
throw err
7171
})
7272
.catch(() => {
73-
expect(storage.getStore()).to.equal(store)
73+
expect(storage('legacy').getStore()).to.equal(store)
7474
})
7575
})
7676

7777
return promise
7878
})
7979

8080
it('should allow to run without a scope if not available when calling then()', () => {
81-
storage.run(null, () => {
81+
storage('legacy').run(null, () => {
8282
const promise = new Promise((resolve, reject) => {
8383
setImmediate(() => {
8484
resolve()
@@ -87,7 +87,7 @@ module.exports = (name, factory, versionRange) => {
8787

8888
return promise
8989
.then(() => {
90-
expect(storage.getStore()).to.be.null
90+
expect(storage('legacy').getStore()).to.be.null
9191
})
9292
})
9393
})

packages/datadog-instrumentations/test/knex.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ describe('Instrumentation', () => {
2424
afterEach(() => client.destroy())
2525

2626
it('should propagate context', () =>
27-
storage.run(store, () =>
27+
storage('legacy').run(store, () =>
2828
client.raw('PRAGMA user_version')
2929
.finally(() => {
30-
expect(storage.getStore()).to.equal(store)
30+
expect(storage('legacy').getStore()).to.equal(store)
3131
})
3232
.catch(() => {})
3333
)

packages/datadog-instrumentations/test/multer.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ withVersions('multer', 'multer', version => {
8686
let payload
8787

8888
function handler (data) {
89-
store = storage.getStore()
89+
store = storage('legacy').getStore()
9090
payload = data
9191
}
9292
multerReadCh.subscribe(handler)

packages/datadog-instrumentations/test/passport-http.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ withVersions('passport-http', 'passport-http', version => {
174174

175175
it('should block when subscriber aborts', async () => {
176176
subscriberStub = sinon.spy(({ abortController }) => {
177-
storage.getStore().req.res.writeHead(403).end('Blocked')
177+
storage('legacy').getStore().req.res.writeHead(403).end('Blocked')
178178
abortController.abort()
179179
})
180180

packages/datadog-instrumentations/test/passport-local.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ withVersions('passport-local', 'passport-local', version => {
154154

155155
it('should block when subscriber aborts', async () => {
156156
subscriberStub = sinon.spy(({ abortController }) => {
157-
storage.getStore().req.res.writeHead(403).end('Blocked')
157+
storage('legacy').getStore().req.res.writeHead(403).end('Blocked')
158158
abortController.abort()
159159
})
160160

packages/datadog-instrumentations/test/passport.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ withVersions('passport', 'passport', version => {
145145
const cookie = login.headers['set-cookie'][0]
146146

147147
subscriberStub.callsFake(({ abortController }) => {
148-
const res = storage.getStore().req.res
148+
const res = storage('legacy').getStore().req.res
149149
res.writeHead(403)
150150
res.constructor.prototype.end.call(res, 'Blocked')
151151
abortController.abort()

0 commit comments

Comments
 (0)