From b934c61731145d88f261895e933a5c4b0a506b23 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 10 Aug 2026 23:12:21 +0200 Subject: [PATCH] chore(skills): derive integration inspector evidence The inspector treated evolving workflows, tests, instrumentation source graphs, plugin traits, and serverless references as closed inventories. That could report valid repository evidence as missing and send an agent to the wrong implementation or test path. --- scripts/verify-integration-skills.js | 249 ++++++++++++++------- scripts/verify-integration-skills.spec.mjs | 108 ++++++++- 2 files changed, 281 insertions(+), 76 deletions(-) diff --git a/scripts/verify-integration-skills.js b/scripts/verify-integration-skills.js index 67672395365..a3354d18895 100644 --- a/scripts/verify-integration-skills.js +++ b/scripts/verify-integration-skills.js @@ -17,17 +17,22 @@ const { parse: parseYaml } = require('yaml') const INTEGRATION_PATTERN = /^[a-z0-9]+(?:[-_][a-z0-9]+)*$/ const MODES = new Set(['add', 'review', 'debug', 'serverless']) -const ORCHESTRION_TRAITS = new Set(['async', 'auto', 'callback', 'cjs-esm', 'orchestrion']) const PLUGIN_BASE_DIRECTORY = 'packages/dd-trace/src/plugins/' -const PLUGIN_BASE_TRAITS = new Set([ - 'cache', - 'client', - 'consumer', - 'database', - 'producer', - 'router', - 'server', - 'tracing', +const PLUGIN_BASE_TRAITS = new Map([ + ['cache', `${PLUGIN_BASE_DIRECTORY}cache.js`], + ['ci', `${PLUGIN_BASE_DIRECTORY}ci_plugin.js`], + ['client', `${PLUGIN_BASE_DIRECTORY}client.js`], + ['composite', `${PLUGIN_BASE_DIRECTORY}composite.js`], + ['consumer', `${PLUGIN_BASE_DIRECTORY}consumer.js`], + ['database', `${PLUGIN_BASE_DIRECTORY}database.js`], + ['log', `${PLUGIN_BASE_DIRECTORY}log_plugin.js`], + ['plugin', `${PLUGIN_BASE_DIRECTORY}plugin.js`], + ['producer', `${PLUGIN_BASE_DIRECTORY}producer.js`], + ['router', 'packages/datadog-plugin-router/src/index.js'], + ['schema', `${PLUGIN_BASE_DIRECTORY}schema.js`], + ['server', `${PLUGIN_BASE_DIRECTORY}server.js`], + ['storage', `${PLUGIN_BASE_DIRECTORY}storage.js`], + ['tracing', `${PLUGIN_BASE_DIRECTORY}tracing.js`], ]) const SOURCE_SUFFIXES = ['.js', '.cjs', '.mjs'] const TEST_SUFFIXES = ['.spec.js', '.spec.cjs', '.spec.mjs'] @@ -43,22 +48,13 @@ const TRAIT_KINDS = new Map([ ['async', 'Async'], ['auto', 'Auto'], ['callback', 'Callback'], + ['sync', 'Sync'], ]) +const ORCHESTRION_TRAITS = new Set([...TRAIT_KINDS.keys(), 'cjs-esm', 'orchestrion']) const TRAITS = new Set([ - 'async', - 'auto', - 'cache', - 'callback', - 'cjs-esm', - 'client', - 'consumer', - 'database', - 'orchestrion', - 'producer', - 'router', - 'server', + ...ORCHESTRION_TRAITS, + ...PLUGIN_BASE_TRAITS.keys(), 'shimmer', - 'tracing', ]) const DISCOVERY_METADATA = new Map([ ['.agents/skills/apm-integrations/agents/openai.yaml', 'apm-integrations'], @@ -108,6 +104,7 @@ const failures = [] /** * @typedef {object} InspectionTargets * @property {string} [instrumentation] + * @property {string[]} instrumentationDependencies * @property {string} [rewriter] * @property {string[]} plugins * @property {string[]} dependents @@ -468,6 +465,18 @@ function findWorkflowLines (filename, integrations) { return locations } +/** + * @param {string[]} integrations + * @returns {string[]} + */ +function findWorkflowRegistrations (integrations) { + const locations = [] + for (const filename of listRelativeFiles('.github/workflows', ['.yml', '.yaml'])) { + locations.push(...findWorkflowLines(filename, integrations)) + } + return locations +} + /** * @param {string} filename * @returns {boolean} @@ -797,35 +806,56 @@ function findPackages (integration, registrations, packageName) { /** * @param {string[]} sources - * @param {string[]} pluginDirectories + * @param {(filename: string) => boolean} includes * @returns {string[]} */ -function findContractSources (sources, pluginDirectories) { +function findSourceDependencies (sources, includes) { const queue = [...sources] const visited = new Set(sources) - const contracts = new Set() + const dependencies = new Set() for (let i = 0; i < queue.length; i++) { - const source = queue[i] - const sourceCode = parseJavaScript(source) + const sourceCode = parseJavaScript(queue[i]) if (!sourceCode) continue for (const { request } of findStaticRequires(sourceCode)) { - const dependency = resolveLocalSource(source, request) - if (!dependency || visited.has(dependency)) continue - - const isBase = dependency.startsWith(PLUGIN_BASE_DIRECTORY) && - !dependency.slice(PLUGIN_BASE_DIRECTORY.length).includes('/') - const isCrossPlugin = dependency.startsWith('packages/datadog-plugin-') && - !pluginDirectories.some(directory => dependency.startsWith(`${directory}/`)) - if (!isBase && !isCrossPlugin) continue + const dependency = resolveLocalSource(queue[i], request) + if (!dependency || visited.has(dependency) || !includes(dependency)) continue visited.add(dependency) - contracts.add(dependency) + dependencies.add(dependency) queue.push(dependency) } } - return [...contracts].sort() + return [...dependencies].sort() +} + +/** + * @param {string[]} sources + * @param {string[]} pluginDirectories + * @returns {string[]} + */ +function findContractSources (sources, pluginDirectories) { + return findSourceDependencies(sources, (dependency) => { + const isBase = dependency.startsWith(PLUGIN_BASE_DIRECTORY) && + !dependency.slice(PLUGIN_BASE_DIRECTORY.length).includes('/') + const isCrossPlugin = dependency.startsWith('packages/datadog-plugin-') && + !pluginDirectories.some(directory => dependency.startsWith(`${directory}/`)) + return isBase || isCrossPlugin + }) +} + +/** + * @param {string | undefined} instrumentation + * @returns {string[]} + */ +function findInstrumentationDependencies (instrumentation) { + if (!instrumentation) return [] + + const directory = 'packages/datadog-instrumentations/src/' + return findSourceDependencies([instrumentation], (dependency) => { + return dependency.startsWith(directory) && !dependency.startsWith(`${directory}helpers/`) + }) } /** @@ -833,10 +863,8 @@ function findContractSources (sources, pluginDirectories) { * @returns {string | undefined} */ function findTraitSource (trait) { - const filename = trait === 'router' - ? 'packages/datadog-plugin-router/src/index.js' - : `packages/dd-trace/src/plugins/${trait}.js` - return PLUGIN_BASE_TRAITS.has(trait) ? existingPath(filename) : undefined + const filename = PLUGIN_BASE_TRAITS.get(trait) + return filename ? existingPath(filename) : undefined } /** @@ -885,6 +913,29 @@ function listPluginFiles (pluginDirectories, subdirectory, suffixes) { return files.sort() } +/** + * @param {string} integration + * @param {string[]} pluginDirectories + * @returns {string[]} + */ +function findTests (integration, pluginDirectories) { + const tests = new Set(listPluginFiles(pluginDirectories, 'test', TEST_SUFFIXES)) + + for (const filename of listRelativeFiles('packages/datadog-instrumentations/test', TEST_SUFFIXES)) { + if (filename.startsWith(`packages/datadog-instrumentations/test/${integration}/`)) { + tests.add(filename) + continue + } + const basename = path.basename(filename) + const name = basename.slice(0, basename.lastIndexOf('.spec.')) + if (name === integration || name.startsWith(`${integration}-`)) tests.add(filename) + } + for (const filename of listRelativeFiles(`integration-tests/${integration}`, TEST_SUFFIXES)) { + tests.add(filename) + } + return [...tests].sort() +} + /** * @param {string[]} filenames * @returns {string[]} @@ -935,13 +986,48 @@ function findRegistrationLedger (ids) { docs: findLines('docs/API.md', ids.flatMap(id => [`id="${id}"`, `[${id}]`])), docsTest: findLines('docs/test.ts', ids.flatMap(id => [`use('${id}'`, `use("${id}"`])), codeowners: findCodeownersCoverage(ids), - workflows: [ - ...findWorkflowLines('.github/workflows/apm-integrations.yml', ids), - ...findWorkflowLines('.github/workflows/serverless.yml', ids), - ], + workflows: findWorkflowRegistrations(ids), } } +/** + * @param {string} filename + * @returns {Map>} + */ +function findStaticClassStrings (filename) { + const properties = new Map() + const sourceCode = parseJavaScript(filename) + if (!sourceCode) return properties + + for (const step of sourceCode.traverse()) { + if (step.type !== 'visit' || step.phase !== 1) continue + + const node = step.target + let name + let value + if (node.type === 'PropertyDefinition' && node.static) { + name = findPropertyName(node) + value = node.value ? findStaticString(node.value) : undefined + } else if (node.type === 'MethodDefinition' && node.static && node.kind === 'get' && + node.value.body.body.length === 1) { + const statement = node.value.body.body[0] + name = findPropertyName(node) + value = statement.type === 'ReturnStatement' && statement.argument + ? findStaticString(statement.argument) + : undefined + } + if (name === undefined || value === undefined) continue + + let values = properties.get(name) + if (!values) { + values = new Set() + properties.set(name, values) + } + values.add(value) + } + return properties +} + /** * @param {string[]} filenames * @param {string[]} publicIds @@ -955,6 +1041,12 @@ function findSchemaRegistrations (filenames, publicIds) { const coordinates = new Map([['type', types], ['kind', kinds], ['id', ids], ['operation', operations]]) for (const filename of filenames) { + for (const [name, values] of findStaticClassStrings(filename)) { + const coordinate = coordinates.get(name) + if (coordinate) { + for (const value of values) coordinate.add(value) + } + } const sourceCode = parseJavaScript(filename) if (!sourceCode) continue @@ -962,32 +1054,17 @@ function findSchemaRegistrations (filenames, publicIds) { if (step.type !== 'visit' || step.phase !== 1) continue const node = step.target - let name - let value - if (node.type === 'PropertyDefinition' && node.static) { - name = findPropertyName(node) - value = node.value ? findStaticString(node.value) : undefined - } else if (node.type === 'MethodDefinition' && node.static && node.kind === 'get' && - node.value.body.body.length === 1) { - const statement = node.value.body.body[0] - name = findPropertyName(node) - value = statement.type === 'ReturnStatement' && statement.argument - ? findStaticString(statement.argument) - : undefined - } else if (node.type === 'CallExpression' && node.callee.type === 'MemberExpression' && - (findPropertyName(node.callee) === 'operationName' || findPropertyName(node.callee) === 'serviceName') && - node.arguments[0]?.type === 'ObjectExpression') { + if (node.type === 'CallExpression' && node.callee.type === 'MemberExpression' && + (findPropertyName(node.callee) === 'operationName' || findPropertyName(node.callee) === 'serviceName') && + node.arguments[0]?.type === 'ObjectExpression') { for (const property of node.arguments[0].properties) { if (property.type !== 'Property') continue - name = findPropertyName(property) - value = findStaticString(property.value) + const name = findPropertyName(property) + const value = findStaticString(property.value) if (name !== 'operation' && value !== undefined) coordinates.get(name)?.add(value) } - continue } - - if (value !== undefined) coordinates.get(name)?.add(value) } } @@ -1048,13 +1125,15 @@ function findReferences (mode, traits, hasRewriter) { /** * @param {string} integration * @param {string[]} traits + * @param {string} mode * @returns {{ integration: string, files: string[], registrations: string[] } | undefined} */ -function findClosestReference (integration, traits) { +function findClosestReference (integration, traits, mode) { if (traits.length === 0) return + const isServerless = mode === 'serverless' const isShimmer = traits.includes('shimmer') - const directory = isShimmer + const directory = isShimmer || isServerless ? 'packages/datadog-instrumentations/src' : 'packages/datadog-instrumentations/src/helpers/rewriter/instrumentations' const requestedBase = traits.find(trait => PLUGIN_BASE_TRAITS.has(trait)) @@ -1063,22 +1142,29 @@ function findClosestReference (integration, traits) { let closestScore = 0 for (const filename of listRelativeFiles(directory, SOURCE_SUFFIXES)) { - if (isShimmer && path.posix.dirname(filename) !== directory) continue + if ((isShimmer || isServerless) && path.posix.dirname(filename) !== directory) continue const candidate = path.basename(filename, path.extname(filename)) if (candidate === 'index' || candidate === integration) continue const source = read(filename) - if (isShimmer + if (!isServerless && (isShimmer ? !source.includes('datadog-shimmer') || findHookPackages(candidate).size === 0 - : findRewriterRegistrations(candidate).length === 0) continue + : findRewriterRegistrations(candidate).length === 0)) continue const moduleName = source.match(/name:\s*['"]([^'"]+)['"]/)?.[1] ?? candidate const registrations = findIntegrationRegistrations(candidate) const pluginSources = listPluginFiles(registrations.pluginDirectories, 'src', SOURCE_SUFFIXES) + if (isServerless && (findHookPackages(candidate).size === 0 || !pluginSources.some(filename => { + return findStaticClassStrings(filename).get('type')?.has('serverless') + }))) continue const contractSources = findContractSources(pluginSources, registrations.pluginDirectories) - let score = isShimmer || traits.includes('orchestrion') ? 1 : 0 + + let score = isServerless || isShimmer || traits.includes('orchestrion') ? 1 : 0 if (requestedSource && contractSources.includes(requestedSource)) score += 8 + if (isServerless && requestedBase && pluginSources.some(filename => { + return findStaticClassStrings(filename).get('kind')?.has(requestedBase) + })) score += 8 if (traits.includes('cjs-esm') && /(?:cjs|commonjs)/i.test(source) && /esm/i.test(source)) score += 4 for (const [trait, kind] of TRAIT_KINDS) { if (traits.includes(trait) && ( @@ -1088,11 +1174,16 @@ function findClosestReference (integration, traits) { if (score <= closestScore) continue closestScore = score - const tests = listPluginFiles(registrations.pluginDirectories, 'test', TEST_SUFFIXES) + const tests = findTests(candidate, registrations.pluginDirectories) const pluginIndex = existingPath(`packages/datadog-plugin-${candidate}/src/index.js`) ?? pluginSources.find(filename => path.basename(filename) === 'index.js') const integrationTest = existingPath(`packages/datadog-plugin-${candidate}/test/index.spec.js`) ?? tests.find(filename => path.basename(filename) === 'index.spec.js') + let fixturePackage + for (const directory of registrations.pluginDirectories) { + fixturePackage = existingPath(`${directory}/test/fixtures/package.json`) + if (fixturePackage) break + } const ledger = findRegistrationLedger(registrations.publicIds) closest = { integration: candidate, @@ -1101,6 +1192,7 @@ function findClosestReference (integration, traits) { resolveLocalSource('packages/datadog-instrumentations/src/helpers/hooks.js', `../${candidate}`), pluginIndex ?? pluginSources[0], integrationTest ?? tests[0], + fixturePackage, ]), registrations: compactPaths([ registrations.hooks.get(moduleName), @@ -1133,12 +1225,14 @@ function inspectIntegration (integration, packageName, mode, traits) { const packageRegistrations = findIntegrationRegistrations(integration) const plugins = listPluginFiles(packageRegistrations.pluginDirectories, 'src', SOURCE_SUFFIXES) const dependents = findPluginDependents(packageRegistrations.pluginDirectories) - const tests = listPluginFiles(packageRegistrations.pluginDirectories, 'test', TEST_SUFFIXES) + const instrumentationDependencies = findInstrumentationDependencies(instrumentation) + const tests = findTests(integration, packageRegistrations.pluginDirectories) const contractSources = findContractSources(plugins, packageRegistrations.pluginDirectories) const { publicIds } = packageRegistrations const ledger = findRegistrationLedger(publicIds) const channelSources = compactPaths([ instrumentation, + ...instrumentationDependencies, rewriter, ...plugins, ...dependents, @@ -1150,6 +1244,7 @@ function inspectIntegration (integration, packageName, mode, traits) { traits, targets: { instrumentation, + instrumentationDependencies, rewriter, plugins, dependents, @@ -1165,7 +1260,7 @@ function inspectIntegration (integration, packageName, mode, traits) { ...ledger, schemas: findSchemaRegistrations([...plugins, ...contractSources], publicIds), }, - reference: findClosestReference(integration, traits), + reference: findClosestReference(integration, traits, mode), references: findReferences(mode, traits, rewriter !== undefined), } } @@ -1212,10 +1307,14 @@ function renderInspection (packet) { ` instrumentation: ${packet.targets.instrumentation ? escapeControlCharacters(packet.targets.instrumentation) : 'missing'}`, + ` instrumentation dependencies: ${packet.targets.instrumentationDependencies.length}`, ` rewriter: ${packet.targets.rewriter ? escapeControlCharacters(packet.targets.rewriter) : 'missing'}`, ` plugin sources: ${packet.targets.plugins.length}`, ` dependent sources: ${packet.targets.dependents.length}`, ` tests: ${packet.targets.tests.length}`, + 'Instrumentation dependencies:', + ...(packet.targets.instrumentationDependencies.length ? packet.targets.instrumentationDependencies : ['none']) + .map(filename => ` ${escapeControlCharacters(filename)}`), 'Plugin sources:', ...(packet.targets.plugins.length ? packet.targets.plugins : ['none']) .map(filename => ` ${escapeControlCharacters(filename)}`), diff --git a/scripts/verify-integration-skills.spec.mjs b/scripts/verify-integration-skills.spec.mjs index 76407e453be..bbe87b703e9 100644 --- a/scripts/verify-integration-skills.spec.mjs +++ b/scripts/verify-integration-skills.spec.mjs @@ -689,7 +689,10 @@ module.exports = require('../../dd-trace/src/plugins/pro' + 'ducer') [['--inspect', '../fixture'], /integration id must contain only lowercase letters/], ] - assert.deepStrictEqual(underscore.registrations.workflows, ['.github/workflows/apm-integrations.yml:221']) + assert.deepStrictEqual(underscore.registrations.workflows, [ + '.github/workflows/apm-integrations.yml:221', + '.github/workflows/instrumentation.yml:99', + ]) for (const [args, expected] of cases) { const { status, stderr } = spawnSync(process.execPath, [verifierPath, ...args], { encoding: 'utf8' }) assert.strictEqual(status, 1) @@ -729,6 +732,7 @@ module.exports = require('../../dd-trace/src/plugins/pro' + 'ducer') assert.match(missing.stdout, /^ {2}rewriter: missing$/m) assert.match(missing.stdout, /^ {2}plugin sources: 0$/m) assert.match(missing.stdout, /^ {2}new-plugin \(candidate\)$/m) + assert.match(missing.stdout, /^Instrumentation dependencies:\n {2}none$/m) assert.match(missing.stdout, /^Contract sources:\n {2}none$/m) assert.match(missing.stdout, /^ {2}packages\/dd-trace\/src\/plugins\/cache\.js$/m) assert.match(missing.stdout, /^Closest current reference: fixture$/m) @@ -901,6 +905,47 @@ module.exports = require('../../dd-trace/src/plugins/pro' + 'ducer') assert.strictEqual(reference.files.includes('packages/datadog-instrumentations/src/ioredis.js'), true) }) + it('routes current sync and composite references', () => { + const sync = inspect(runRepositoryTool, 'new-plugin', ['--traits', 'sync']) + const composite = inspect(runRepositoryTool, 'new-plugin', ['--traits', 'composite']) + + assert.strictEqual(sync.reference.files.some((filename) => { + return readFileSync(join(repositoryDirectory, filename), 'utf8').includes("kind: 'Sync'") + }), true) + assert.strictEqual( + composite.references.includes('packages/dd-trace/src/plugins/composite.js'), + true + ) + }) + + it('follows split instrumentation sources', () => { + const packet = inspect(runRepositoryTool, 'http') + const rendered = runRepositoryTool(['--inspect', 'http']) + + assert.deepStrictEqual(packet.targets.instrumentationDependencies, [ + 'packages/datadog-instrumentations/src/http/client.js', + 'packages/datadog-instrumentations/src/http/server.js', + ]) + assert.strictEqual(rendered.status, 0, rendered.stderr) + assert.match( + rendered.stdout, + /^Instrumentation dependencies:\n {2}packages\/datadog-instrumentations\/src\/http\/client\.js$/m + ) + assert.match(rendered.stdout, /^ {2}packages\/datadog-instrumentations\/src\/http\/server\.js$/m) + assert.strictEqual( + packet.evidence.channelAnchors.some(anchor => anchor.startsWith( + 'packages/datadog-instrumentations/src/http/client.js:' + )), + true + ) + assert.strictEqual( + packet.evidence.channelAnchors.some(anchor => anchor.startsWith( + 'packages/datadog-instrumentations/src/http/server.js:' + )), + true + ) + }) + it('keeps package linkage narrow when an instrumentation uses a shared plugin', () => { const packet = inspect(runRepositoryTool, 'webdriverio') @@ -913,6 +958,20 @@ module.exports = require('../../dd-trace/src/plugins/pro' + 'ducer') assert.match(packet.packages[2].plugin, /packages\/dd-trace\/src\/plugins\/index\.js:/) assert.strictEqual(packet.packages[3].plugin, undefined) assert.deepStrictEqual(packet.targets.plugins, ['packages/datadog-plugin-mocha/src/index.js']) + assert.strictEqual( + packet.targets.tests.includes('packages/datadog-instrumentations/test/webdriverio.spec.js'), + true + ) + assert.strictEqual( + packet.targets.tests.includes('integration-tests/webdriverio/webdriverio.spec.js'), + true + ) + assert.strictEqual( + packet.registrations.workflows.some(filename => filename.startsWith( + '.github/workflows/test-optimization.yml:' + )), + true + ) assert.strictEqual( packet.evidence.contractSources.includes('packages/dd-trace/src/plugins/ci_plugin.js'), true @@ -975,6 +1034,53 @@ module.exports = require('../../dd-trace/src/plugins/pro' + 'ducer') assert.match(rendered.stdout, /^ {2}packages\/datadog-plugin-http\/src\/index\.js$/m) }) + it('finds instrumentation tests and every owning workflow', () => { + const packet = inspect(runRepositoryTool, 'crypto') + const nested = inspect(runRepositoryTool, 'path-to-regexp') + + assert.strictEqual( + packet.targets.tests.includes('packages/datadog-instrumentations/test/crypto.spec.js'), + true + ) + assert.strictEqual( + nested.targets.tests.includes('packages/datadog-instrumentations/test/path-to-regexp/index.spec.js'), + true + ) + assert.strictEqual( + packet.targets.tests.includes('packages/datadog-plugin-crypto/test/integration-test/client.spec.js'), + true + ) + assert.deepStrictEqual( + packet.registrations.workflows.map(filename => filename.replace(/:\d+$/, '')), + [ + '.github/workflows/apm-integrations.yml', + '.github/workflows/instrumentation.yml', + ] + ) + }) + + it('uses a serverless integration as the closest serverless reference', () => { + const reference = inspect(runRepositoryTool, 'new-cloud', [ + '--mode', + 'serverless', + '--traits', + 'server', + ]).reference + + assert.strictEqual( + ['azure-durable-functions', 'azure-functions'].includes(reference.integration), + true + ) + assert.strictEqual( + reference.files.some(filename => filename.endsWith('/test/fixtures/package.json')), + true + ) + assert.strictEqual( + reference.registrations.some(filename => filename.startsWith('.github/workflows/serverless.yml:')), + true + ) + }) + it('routes router integrations through their actual contract sources', () => { const packet = inspect(runRepositoryTool, 'express', ['--traits', 'router'])