Skip to content

Commit cf28ba2

Browse files
rochdevrochdev
authored
refactor rewriter with internal api matching real orchestrion (#7677)
1 parent 24556a1 commit cf28ba2

File tree

10 files changed

+353
-144
lines changed

10 files changed

+353
-144
lines changed

‎packages/datadog-instrumentations/src/helpers/rewriter/index.js‎

Lines changed: 26 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -1,95 +1,45 @@
11
'use strict'
22

3-
/*
4-
This rewriter is basically a JavaScript version of Orchestrion-JS. The goal is
5-
not to replace Orchestrion-JS, but rather to make it easier and faster to write
6-
new integrations in the short-term, especially as many changes to the rewriter
7-
will be needed as all the patterns we need have not been identified yet. This
8-
will avoid the back and forth of having to make Rust changes to an external
9-
library for every integration change or addition that requires something new.
10-
11-
In the meantime, we'll work concurrently on a change to Orchestrion-JS that
12-
adds an "arbitrary transform" or "plugin" system that can be used from
13-
JavaScript, in order to enable quick iteration while still using Orchestrion-JS.
14-
Once that's done we'll use that, so that we can remove this JS approach and
15-
return to using Orchestrion-JS.
16-
17-
The long term goal is to backport any additional features we add to the JS
18-
rewriter (or using the plugin system in Orchestrion-JS once we're using that)
19-
to Orchestrion-JS once we're confident that the implementation is fairly
20-
complete and has all features we need.
21-
22-
Here is a list of the additions and changes in this rewriter compared to
23-
Orchestrion-JS that will need to be backported:
24-
25-
(NOTE: Please keep this list up-to-date whenever new features are added)
26-
27-
- Supports an `astQuery` field to filter AST nodes with an esquery query. This
28-
is mostly meant to be used when experimenting or if what needs to be queried
29-
is not a function. We'll see over time if something like this is needed to be
30-
backported or if it can be replaced by simpler queries.
31-
- Supports replacing methods of child class instances in the base constructor.
32-
- Supports tracing iterator (sync/async) returning functions (sync/async).
33-
*/
34-
353
const { readFileSync } = require('fs')
364
const { join } = require('path')
37-
const semifies = require('../../../../../vendor/dist/semifies')
385
const log = require('../../../../dd-trace/src/log')
39-
const { getEnvironmentVariable } = require('../../../../dd-trace/src/config/helper')
40-
const { transform } = require('./transformer')
41-
const { generate, parse, traverse } = require('./compiler')
426
const instrumentations = require('./instrumentations')
7+
const { create } = require('./orchestrion')
438

44-
const NODE_OPTIONS = getEnvironmentVariable('NODE_OPTIONS')
45-
46-
/** @type {Record<string, Set<string>>} map of module base name to supported function query versions */
47-
const supported = {}
9+
/** @type {Record<string, string>} map of module base name to version */
10+
const moduleVersions = {}
4811
const disabled = new Set()
49-
50-
// TODO: Source maps without `--enable-source-maps`.
51-
const enableSourceMaps = NODE_OPTIONS?.includes('--enable-source-maps') ||
52-
process.execArgv?.some(arg => arg.includes('--enable-source-maps'))
53-
54-
let SourceMapGenerator
12+
const matcher = create(instrumentations, 'dc-polyfill')
5513

5614
function rewrite (content, filename, format) {
5715
if (!content) return content
16+
if (!filename.includes('node_modules')) return content
5817

59-
const sourceType = format === 'module' ? 'module' : 'script'
60-
61-
try {
62-
let ast
63-
64-
filename = filename.replace('file://', '')
65-
66-
for (const inst of instrumentations) {
67-
const { astQuery, functionQuery = {}, module: { name, versionRange, filePath } } = inst
18+
filename = filename.replace('file://', '')
6819

69-
if (disabled.has(name)) continue
70-
if (!filename.endsWith(`${name}/${filePath}`)) continue
71-
if (!satisfies(filename, filePath, versionRange)) continue
20+
const moduleType = format === 'module' ? 'esm' : 'cjs'
21+
const [modulePath] = filename.split('/node_modules/').reverse()
22+
const moduleParts = modulePath.split('/')
23+
const splitIndex = moduleParts[0].startsWith('@') ? 2 : 1
24+
const moduleName = moduleParts.slice(0, splitIndex).join('/')
25+
const filePath = moduleParts.slice(splitIndex).join('/')
26+
const version = getVersion(filename, filePath)
7227

73-
ast ??= parse(content.toString(), { range: true, sourceType })
28+
if (disabled.has(moduleName)) return content
7429

75-
const query = astQuery || fromFunctionQuery(functionQuery)
76-
const state = { ...inst, sourceType, functionQuery }
30+
const transformer = matcher.getTransformer(moduleName, version, filePath)
7731

78-
traverse(ast, query, (...args) => transform(state, ...args))
79-
}
32+
if (!transformer) return content
8033

81-
if (ast) {
82-
if (!enableSourceMaps) return generate(ast)
34+
try {
35+
// TODO: pass existing sourcemap as input for remapping
36+
const { code, map } = transformer.transform(content, moduleType)
8337

84-
// TODO: Can we use the same version of `source-map` that DI uses?
85-
SourceMapGenerator ??= require('../../../../../vendor/dist/@datadog/source-map').SourceMapGenerator
38+
if (!map) return code
8639

87-
const sourceMap = new SourceMapGenerator({ file: filename })
88-
const code = generate(ast, { sourceMap })
89-
const map = Buffer.from(sourceMap.toString()).toString('base64')
40+
const inlineMap = Buffer.from(map).toString('base64')
9041

91-
return code + '\n' + `//# sourceMappingURL=data:application/json;base64,${map}`
92-
}
42+
return code + '\n' + `//# sourceMappingURL=data:application/json;base64,${inlineMap}`
9343
} catch (e) {
9444
log.error(e)
9545
}
@@ -101,55 +51,20 @@ function disable (instrumentation) {
10151
disabled.add(instrumentation)
10252
}
10353

104-
function satisfies (filename, filePath, versions) {
54+
function getVersion (filename, filePath) {
10555
const [basename] = filename.split(filePath)
10656

107-
supported[basename] ??= new Set()
108-
109-
if (!supported[basename].has(versions)) {
57+
if (!moduleVersions[basename]) {
11058
try {
11159
const pkg = JSON.parse(readFileSync(
11260
join(basename, 'package.json'), 'utf8'
11361
))
11462

115-
if (semifies(pkg.version, versions)) {
116-
supported[basename].add(versions)
117-
}
63+
moduleVersions[basename] = pkg.version
11864
} catch {}
11965
}
12066

121-
return supported[basename].has(versions)
122-
}
123-
124-
// TODO: Support index
125-
function fromFunctionQuery (functionQuery) {
126-
const { methodName, functionName, expressionName, className } = functionQuery
127-
const queries = []
128-
129-
if (className) {
130-
queries.push(
131-
`[id.name="${className}"]`,
132-
`[id.name="${className}"] > ClassExpression`,
133-
`[id.name="${className}"] > ClassBody > [key.name="${methodName}"] > [async]`,
134-
`[id.name="${className}"] > ClassExpression > ClassBody > [key.name="${methodName}"] > [async]`
135-
)
136-
} else if (methodName) {
137-
queries.push(
138-
`ClassBody > [key.name="${methodName}"] > [async]`,
139-
`Property[key.name="${methodName}"] > [async]`
140-
)
141-
}
142-
143-
if (functionName) {
144-
queries.push(`FunctionDeclaration[id.name="${functionName}"][async]`)
145-
} else if (expressionName) {
146-
queries.push(
147-
`FunctionExpression[id.name="${expressionName}"][async]`,
148-
`ArrowFunctionExpression[id.name="${expressionName}"][async]`
149-
)
150-
}
151-
152-
return queries.join(', ')
67+
return moduleVersions[basename]
15368
}
15469

15570
module.exports = { rewrite, disable }

packages/datadog-instrumentations/src/helpers/rewriter/compiler.js renamed to packages/datadog-instrumentations/src/helpers/rewriter/orchestrion/compiler.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict'
22

3-
const log = require('../../../../dd-trace/src/log')
3+
const log = require('../../../../../dd-trace/src/log')
44

55
// eslint-disable-next-line camelcase, no-undef
66
const runtimeRequire = typeof __webpack_require__ === 'function' ? __non_webpack_require__ : require
@@ -25,7 +25,7 @@ const compiler = {
2525
log.error(e)
2626

2727
// Fallback for when OXC is not available.
28-
const meriyah = require('../../../../../vendor/dist/meriyah')
28+
const meriyah = require('../../../../../../vendor/dist/meriyah')
2929

3030
compiler.parse = (sourceText, { range, sourceType } = {}) => {
3131
return meriyah.parse(sourceText.toString(), {
@@ -40,15 +40,15 @@ const compiler = {
4040
},
4141

4242
generate: (...args) => {
43-
const astring = require('../../../../../vendor/dist/astring')
43+
const astring = require('../../../../../../vendor/dist/astring')
4444

4545
compiler.generate = astring.generate
4646

4747
return compiler.generate(...args)
4848
},
4949

5050
traverse: (ast, query, visitor) => {
51-
const esquery = require('../../../../../vendor/dist/esquery').default
51+
const esquery = require('../../../../../../vendor/dist/esquery').default
5252

5353
compiler.traverse = (ast, query, visitor) => {
5454
return esquery.traverse(ast, esquery.parse(query), visitor)
@@ -58,7 +58,7 @@ const compiler = {
5858
},
5959

6060
query: (ast, query) => {
61-
const esquery = require('../../../../../vendor/dist/esquery').default
61+
const esquery = require('../../../../../../vendor/dist/esquery').default
6262

6363
compiler.query = esquery.query
6464

‎packages/datadog-instrumentations/src/helpers/rewriter/orchestrion/index.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
'use strict'
2+
3+
/*
4+
This folder is basically a JavaScript version of Orchestrion-JS. The goal is
5+
not to replace Orchestrion-JS, but rather to make it easier and faster to write
6+
new integrations in the short-term, especially as many changes to the rewriter
7+
will be needed as all the patterns we need have not been identified yet. This
8+
will avoid the back and forth of having to make Rust changes to an external
9+
library for every integration change or addition that requires something new.
10+
11+
In the meantime, we'll work concurrently on a change to Orchestrion-JS that
12+
adds an "arbitrary transform" or "plugin" system that can be used from
13+
JavaScript, in order to enable quick iteration while still using Orchestrion-JS.
14+
Once that's done we'll use that, so that we can remove this JS approach and
15+
return to using Orchestrion-JS.
16+
17+
The long term goal is to backport any additional features we add to the JS
18+
rewriter (or using the plugin system in Orchestrion-JS once we're using that)
19+
to Orchestrion-JS once we're confident that the implementation is fairly
20+
complete and has all features we need.
21+
22+
Here is a list of the additions and changes in this rewriter compared to
23+
Orchestrion-JS that will need to be backported:
24+
25+
(NOTE: Please keep this list up-to-date whenever new features are added)
26+
27+
- Supports an `astQuery` field to filter AST nodes with an esquery query. This
28+
is mostly meant to be used when experimenting or if what needs to be queried
29+
is not a function. We'll see over time if something like this is needed to be
30+
backported or if it can be replaced by simpler queries.
31+
- Supports replacing methods of child class instances in the base constructor.
32+
- Supports tracing iterator (sync/async) returning functions (sync/async).
33+
*/
34+
35+
/* eslint-disable camelcase */
36+
37+
const { InstrumentationMatcher } = require('./matcher')
38+
39+
function create (configs, dc_module) {
40+
return new InstrumentationMatcher(configs, dc_module)
41+
}
42+
43+
module.exports = { create }

‎packages/datadog-instrumentations/src/helpers/rewriter/orchestrion/matcher.js‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
'use strict'
2+
3+
/* eslint-disable camelcase */
4+
5+
const semifies = require('../../../../../../vendor/dist/semifies')
6+
const { Transformer } = require('./transformer')
7+
8+
// TODO: addTransform
9+
10+
class InstrumentationMatcher {
11+
#configs = []
12+
#dc_module = null
13+
#transformers = {}
14+
15+
constructor (configs, dc_module) {
16+
this.#configs = configs
17+
this.#dc_module = dc_module || 'diagnostics_channel'
18+
}
19+
20+
free () {
21+
this.#transformers = {}
22+
}
23+
24+
getTransformer (module_name, version, file_path) {
25+
const id = `${module_name}/${file_path}@${version}`
26+
27+
if (this.#transformers[id]) return this.#transformers[id]
28+
29+
const configs = this.#configs.filter(({ module: { name, filePath, versionRange } }) =>
30+
name === module_name &&
31+
filePath === file_path &&
32+
semifies(version, versionRange)
33+
)
34+
35+
if (configs.length === 0) return
36+
37+
this.#transformers[id] = new Transformer(
38+
module_name,
39+
version,
40+
file_path,
41+
configs,
42+
this.#dc_module
43+
)
44+
45+
return this.#transformers[id]
46+
}
47+
}
48+
49+
module.exports = { InstrumentationMatcher }

0 commit comments

Comments
 (0)