[DI] Improve separation between RC and breakpoint logic (#4992)

This will make it easier to mock either one or the other in tests or to
add probes without relying on RC.

packages/dd-trace/src/debugger/devtools_client/breakpoints.js

@@ -0,0 +1,69 @@
+'use strict'
+
+const session = require('./session')
+const { findScriptFromPartialPath, probes, breakpoints } = require('./state')
+const log = require('../../log')
+
+let sessionStarted = false
+
+module.exports = {
+  addBreakpoint,
+  removeBreakpoint
+}
+
+async function addBreakpoint (probe) {
+  if (!sessionStarted) await start()
+
+  const file = probe.where.sourceFile
+  const line = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints
+
+  // Optimize for sending data to /debugger/v1/input endpoint
+  probe.location = { file, lines: [String(line)] }
+  delete probe.where
+
+  // TODO: Inbetween `await session.post('Debugger.enable')` and here, the scripts are parsed and cached.
+  // Maybe there's a race condition here or maybe we're guraenteed that `await session.post('Debugger.enable')` will
+  // not continue untill all scripts have been parsed?
+  const script = findScriptFromPartialPath(file)
+  if (!script) throw new Error(`No loaded script found for ${file} (probe: ${probe.id}, version: ${probe.version})`)
+  const [path, scriptId] = script
+
+  log.debug(`Adding breakpoint at ${path}:${line} (probe: ${probe.id}, version: ${probe.version})`)
+
+  const { breakpointId } = await session.post('Debugger.setBreakpoint', {
+    location: {
+      scriptId,
+      lineNumber: line - 1 // Beware! lineNumber is zero-indexed
+    }
+  })
+
+  probes.set(probe.id, breakpointId)
+  breakpoints.set(breakpointId, probe)
+}
+
+async function removeBreakpoint ({ id }) {
+  if (!sessionStarted) {
+    // We should not get in this state, but abort if we do, so the code doesn't fail unexpected
+    throw Error(`Cannot remove probe ${id}: Debugger not started`)
+  }
+  if (!probes.has(id)) {
+    throw Error(`Unknown probe id: ${id}`)
+  }
+
+  const breakpointId = probes.get(id)
+  await session.post('Debugger.removeBreakpoint', { breakpointId })
+  probes.delete(id)
+  breakpoints.delete(breakpointId)
+
+  if (breakpoints.size === 0) await stop()
+}
+
+async function start () {
+  sessionStarted = true
+  return session.post('Debugger.enable') // return instead of await to reduce number of promises created
+}
+
+async function stop () {
+  sessionStarted = false
+  return session.post('Debugger.disable') // return instead of await to reduce number of promises created
+}

packages/dd-trace/src/debugger/devtools_client/remote_config.js

@@ -1,13 +1,10 @@
 'use strict'
 
 const { workerData: { rcPort } } = require('node:worker_threads')
-const { findScriptFromPartialPath, probes, breakpoints } = require('./state')
-const session = require('./session')
+const { addBreakpoint, removeBreakpoint } = require('./breakpoints')
 const { ackReceived, ackInstalled, ackError } = require('./status')
 const log = require('../../log')
 
-let sessionStarted = false
-
 // Example log line probe (simplified):
 // {
 //   id: '100c9a5c-45ad-49dc-818b-c570d31e11d1',
@@ -46,16 +43,6 @@ rcPort.on('message', async ({ action, conf: probe, ackId }) => {
 })
 rcPort.on('messageerror', (err) => log.error(err))
 
-async function start () {
-  sessionStarted = true
-  return session.post('Debugger.enable') // return instead of await to reduce number of promises created
-}
-
-async function stop () {
-  sessionStarted = false
-  return session.post('Debugger.disable') // return instead of await to reduce number of promises created
-}
-
 async function processMsg (action, probe) {
   log.debug(`Received request to ${action} ${probe.type} probe (id: ${probe.id}, version: ${probe.version})`)
 
@@ -90,11 +77,13 @@ async function processMsg (action, probe) {
         break
       case 'apply':
         await addBreakpoint(probe)
+        ackInstalled(probe)
         break
       case 'modify':
         // TODO: Modify existing probe instead of removing it (DEBUG-2817)
         await removeBreakpoint(probe)
         await addBreakpoint(probe)
+        ackInstalled(probe) // TODO: Should we also send ackInstalled when modifying a probe?
         break
       default:
         throw new Error(
@@ -107,55 +96,6 @@ async function processMsg (action, probe) {
   }
 }
 
-async function addBreakpoint (probe) {
-  if (!sessionStarted) await start()
-
-  const file = probe.where.sourceFile
-  const line = Number(probe.where.lines[0]) // Tracer doesn't support multiple-line breakpoints
-
-  // Optimize for sending data to /debugger/v1/input endpoint
-  probe.location = { file, lines: [String(line)] }
-  delete probe.where
-
-  // TODO: Inbetween `await session.post('Debugger.enable')` and here, the scripts are parsed and cached.
-  // Maybe there's a race condition here or maybe we're guraenteed that `await session.post('Debugger.enable')` will
-  // not continue untill all scripts have been parsed?
-  const script = findScriptFromPartialPath(file)
-  if (!script) throw new Error(`No loaded script found for ${file} (probe: ${probe.id}, version: ${probe.version})`)
-  const [path, scriptId] = script
-
-  log.debug(`Adding breakpoint at ${path}:${line} (probe: ${probe.id}, version: ${probe.version})`)
-
-  const { breakpointId } = await session.post('Debugger.setBreakpoint', {
-    location: {
-      scriptId,
-      lineNumber: line - 1 // Beware! lineNumber is zero-indexed
-    }
-  })
-
-  probes.set(probe.id, breakpointId)
-  breakpoints.set(breakpointId, probe)
-
-  ackInstalled(probe)
-}
-
-async function removeBreakpoint ({ id }) {
-  if (!sessionStarted) {
-    // We should not get in this state, but abort if we do, so the code doesn't fail unexpected
-    throw Error(`Cannot remove probe ${id}: Debugger not started`)
-  }
-  if (!probes.has(id)) {
-    throw Error(`Unknown probe id: ${id}`)
-  }
-
-  const breakpointId = probes.get(id)
-  await session.post('Debugger.removeBreakpoint', { breakpointId })
-  probes.delete(id)
-  breakpoints.delete(breakpointId)
-
-  if (breakpoints.size === 0) await stop()
-}
-
 async function lock () {
   if (lock.p) await lock.p
   let resolve