From 50f4a5a9d3b6925ee826a3b803cb5fe0609ad96b Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Wed, 24 Jul 2024 10:37:26 -0400 Subject: [PATCH] feat!: Removed instrumentation for `director` --- lib/instrumentation/director.js | 64 ---- lib/instrumentations.js | 1 - test/versioned/director/director.tap.js | 471 ------------------------ test/versioned/director/newrelic.js | 25 -- test/versioned/director/package.json | 21 -- 5 files changed, 582 deletions(-) delete mode 100644 lib/instrumentation/director.js delete mode 100644 test/versioned/director/director.tap.js delete mode 100644 test/versioned/director/newrelic.js delete mode 100644 test/versioned/director/package.json diff --git a/lib/instrumentation/director.js b/lib/instrumentation/director.js deleted file mode 100644 index 2320238e20..0000000000 --- a/lib/instrumentation/director.js +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const { MiddlewareSpec, MiddlewareMounterSpec } = require('../shim/specs') - -module.exports = function initialize(agent, director, moduleName, shim) { - shim.setFramework(shim.DIRECTOR) - - shim.setRouteParser(function routeParser(shim, fn, fnName, route) { - return route instanceof Array ? route.join('/') : route - }) - - const methods = ['on', 'route'] - const proto = director.Router.prototype - shim.wrapMiddlewareMounter( - proto, - methods, - new MiddlewareMounterSpec({ - route: shim.SECOND, - wrapper: function wrapMiddleware(shim, middleware, name, path) { - return shim.recordMiddleware( - middleware, - new MiddlewareSpec({ - route: path, - req: function getReq() { - return this.req - }, - params: function getParams() { - return this.params - }, - next: shim.LAST - }) - ) - } - }) - ) - - shim.wrap(proto, 'mount', function wrapMount(shim, mount) { - return function wrappedMount(routes, path) { - const isAsync = this.async - shim.wrap(routes, director.http.methods, function wrapRoute(shim, route) { - return shim.recordMiddleware( - route, - new MiddlewareSpec({ - route: path.join('/'), - req: function getReq() { - return this.req - }, - params: function getParams() { - return this.params - }, - next: isAsync ? shim.LAST : null - }) - ) - }) - const args = [routes, path] - return mount.apply(this, args) - } - }) -} diff --git a/lib/instrumentations.js b/lib/instrumentations.js index 4b0f7f8d07..9088c83756 100644 --- a/lib/instrumentations.js +++ b/lib/instrumentations.js @@ -24,7 +24,6 @@ module.exports = function instrumentations() { 'bunyan': { type: InstrumentationDescriptor.TYPE_GENERIC }, 'cassandra-driver': { type: InstrumentationDescriptor.TYPE_DATASTORE }, 'connect': { type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK }, - 'director': { type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK }, 'express': { type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK }, 'fastify': { type: InstrumentationDescriptor.TYPE_WEB_FRAMEWORK }, 'generic-pool': { type: InstrumentationDescriptor.TYPE_GENERIC }, diff --git a/test/versioned/director/director.tap.js b/test/versioned/director/director.tap.js deleted file mode 100644 index eb643a6c89..0000000000 --- a/test/versioned/director/director.tap.js +++ /dev/null @@ -1,471 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -const tap = require('tap') -const http = require('http') -const helper = require('../../lib/agent_helper') - -tap.test('basic director test', function (t) { - let server = null - const agent = helper.instrumentMockedAgent() - - const director = require('director') - - function fn0() { - t.ok(agent.getTransaction(), 'transaction is available') - this.res.writeHead(200, { 'Content-Type': 'application/json' }) - this.res.end('{"status":"ok"}') - } - - // this will still get hit even though the fn0 is ending response - function fn1() { - return true - } - - const routes = { - '/hello': { - 'get': fn0, - '/(\\w+)/': { - get: fn1 - } - } - } - - const router = new director.http.Router(routes).configure({ recurse: 'forward' }) - - t.teardown(function () { - helper.unloadAgent(agent) - server.close(function () {}) - }) - - // need to capture parameters - agent.config.attributes.enabled = true - - agent.on('transactionFinished', function (transaction) { - t.equal(transaction.name, 'WebTransaction/Director/GET//hello', 'transaction has expected name') - - t.equal(transaction.url, '/hello/eric', 'URL is left alone') - t.equal(transaction.statusCode, 200, 'status code is OK') - t.equal(transaction.verb, 'GET', 'HTTP method is GET') - t.ok(transaction.trace, 'transaction has trace') - - const web = transaction.trace.root.children[0] - t.ok(web, 'trace has web segment') - t.equal(web.name, transaction.name, 'segment name and transaction name match') - - t.equal(web.partialName, 'Director/GET//hello', 'should have partial name for apdex') - - const handler0 = web.children[0] - t.equal( - handler0.name, - 'Nodejs/Middleware/Director/fn0//hello', - 'route 0 segment has correct name' - ) - - const handler1 = web.children[1] - t.equal( - handler1.name, - 'Nodejs/Middleware/Director/fn1//hello/(\\w+)/', - 'route 1 segment has correct name' - ) - }) - - server = http.createServer(function (req, res) { - router.dispatch(req, res, function (err) { - if (err) { - res.writeHead(404) - res.end() - } - }) - }) - - helper.randomPort(function (port) { - server.listen(port, function () { - const url = 'http://localhost:' + port + '/hello/eric' - helper.makeGetRequest(url, function (error, res, body) { - t.equal(res.statusCode, 200, 'nothing exploded') - t.same(body, { status: 'ok' }, 'got expected response') - t.end() - }) - }) - }) -}) - -tap.test('backward recurse director test', function (t) { - let server = null - const agent = helper.instrumentMockedAgent() - - const director = require('director') - - function fn0() { - this.res.writeHead(200, { 'Content-Type': 'application/json' }) - this.res.end('{"status":"ok"}') - } - function fn1() { - null - } - - const routes = { - '/hello': { - 'get': fn0, - '/(\\w+)/': { - get: fn1 - } - } - } - - const router = new director.http.Router(routes).configure({ recurse: 'backward' }) - - t.teardown(function () { - helper.unloadAgent(agent) - server.close(function () {}) - }) - // need to capture parameters - agent.config.attributes.enabled = true - - agent.on('transactionFinished', function (transaction) { - t.equal(transaction.name, 'WebTransaction/Director/GET//hello', 'transaction has expected name') - - const web = transaction.trace.root.children[0] - t.equal(web.partialName, 'Director/GET//hello', 'should have partial name for apdex') - }) - - server = http.createServer(function (req, res) { - router.dispatch(req, res, function (err) { - if (err) { - res.writeHead(404) - res.end() - } - }) - }) - - helper.randomPort(function (port) { - server.listen(port, function () { - const url = 'http://localhost:' + port + '/hello/eric' - helper.makeGetRequest(url, { json: true }, function (error, res, body) { - t.equal(res.statusCode, 200, 'nothing exploded') - t.same(body, { status: 'ok' }, 'got expected response') - t.end() - }) - }) - }) -}) - -tap.test('two routers with same URI director test', function (t) { - let server = null - const agent = helper.instrumentMockedAgent() - - const director = require('director') - - const router = new director.http.Router() - - t.teardown(function () { - helper.unloadAgent(agent) - server.close(function () {}) - }) - - // need to capture parameters - agent.config.attributes.enabled = true - - agent.on('transactionFinished', function (transaction) { - t.equal( - transaction.name, - 'WebTransaction/Director/GET//helloWorld', - 'transaction has expected name' - ) - - const web = transaction.trace.root.children[0] - t.equal(web.partialName, 'Director/GET//helloWorld', 'should have partial name for apdex') - }) - - router.get('/helloWorld', function () {}) - router.get('/helloWorld', function () { - this.res.writeHead(200, { 'Content-Type': 'application/json' }) - this.res.end('{"status":"ok"}') - }) - - server = http.createServer(function (req, res) { - router.dispatch(req, res, function (err) { - if (err) { - res.writeHead(404) - res.end() - } - }) - }) - - helper.randomPort(function (port) { - server.listen(port, function () { - const url = 'http://localhost:' + port + '/helloWorld' - helper.makeGetRequest(url, { json: true }, function (error, res, body) { - t.equal(res.statusCode, 200, 'nothing exploded') - t.same(body, { status: 'ok' }, 'got expected response') - t.end() - }) - }) - }) -}) - -tap.test('director async routes test', function (t) { - let server = null - const agent = helper.instrumentMockedAgent() - - const director = require('director') - - const router = new director.http.Router().configure({ async: true }) - - t.teardown(function () { - helper.unloadAgent(agent) - server.close(function () {}) - }) - - // need to capture parameters - agent.config.attributes.enabled = true - - agent.on('transactionFinished', function (transaction) { - t.equal( - transaction.name, - 'WebTransaction/Director/GET//:foo/:bar/:bazz', - 'transaction has expected name' - ) - - const web = transaction.trace.root.children[0] - t.equal(web.partialName, 'Director/GET//:foo/:bar/:bazz', 'should have partial name for apdex') - - const handler0 = web.children[0] - - t.equal( - handler0.name, - 'Nodejs/Middleware/Director/fn0//:foo/:bar/:bazz', - 'route 0 segment has correct name' - ) - - const handler1 = web.children[1] - - t.equal( - handler1.name, - 'Nodejs/Middleware/Director/fn1//:foo/:bar/:bazz', - 'route 1 segment has correct name' - ) - }) - - router.get('/:foo/:bar/:bazz', function fn0(foo, bar, bazz, next) { - setTimeout( - function () { - next() - }, - 100, - this - ) - }) - router.get('/:foo/:bar/:bazz', function fn1() { - setTimeout( - function (self) { - self.res.end('dog') - }, - 100, - this - ) - }) - - server = http.createServer(function (req, res) { - router.dispatch(req, res, function (err) { - if (err) { - res.writeHead(404) - res.end() - } - }) - }) - - helper.randomPort(function (port) { - server.listen(port, function () { - const url = 'http://localhost:' + port + '/three/random/things' - helper.makeGetRequest(url, { json: true }, function (error, res, body) { - t.equal(res.statusCode, 200, 'nothing exploded') - t.same(body, 'dog', 'got expected response') - t.end() - }) - }) - }) -}) - -tap.test('express w/ director subrouter test', function (t) { - t.plan(4) - const agent = helper.instrumentMockedAgent() - - const director = require('director') - - const express = require('express') - const expressRouter = express.Router() // eslint-disable-line new-cap - const app = express() - let server - - function helloWorld() { - this.res.writeHead(200, { 'Content-Type': 'text/plain' }) - this.res.end('eric says hello') - } - - const routes = { - '/hello': { get: helloWorld } - } - const router = new director.http.Router(routes) - - t.teardown(function () { - helper.unloadAgent(agent) - server.close(function () {}) - }) - - // need to capture parameters - agent.config.attributes.enabled = true - - agent.on('transactionFinished', function (transaction) { - t.equal( - transaction.name, - 'WebTransaction/Director/GET//express/hello', - 'transaction has expected name' - ) - - const web = transaction.trace.root.children[0] - t.equal(web.partialName, 'Director/GET//express/hello', 'should have partial name for apdex') - }) - - expressRouter.use(function myMiddleware(req, res, next) { - router.dispatch(req, res, function (err) { - if (err) { - next(err) - } - }) - }) - - app.use('/express/', expressRouter) - - helper.randomPort(function (port) { - server = app.listen(port, 'localhost', function () { - const url = 'http://localhost:' + port + '/express/hello' - helper.makeGetRequest(url, { json: true }, function (error, res, body) { - t.equal(res.statusCode, 200, 'nothing exploded') - t.same(body, 'eric says hello', 'got expected response') - }) - }) - }) -}) - -tap.test('director instrumentation', function (t) { - t.plan(10) - - t.test('should allow null routers through constructor on http router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const routes = { - '/hello': null - } - - new director.http.Router(routes) // eslint-disable-line no-new - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow null routers through constructor on base router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const routes = { - '/hello': null - } - - new director.Router(routes) // eslint-disable-line no-new - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow null routers through constructor on cli router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const routes = { - '/hello': null - } - - new director.cli.Router(routes) // eslint-disable-line no-new - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow routers through .on on cli router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const router = new director.cli.Router() - router.on(/^$/, function () {}) - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow routers through .on on http router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const router = new director.http.Router() - router.on('get', /^$/, function () {}) - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow routers through .on on base router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const router = new director.Router() - router.on(/^$/, function () {}) - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow null routers through method mounters', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const router = new director.http.Router() - - router.get('/tes/', null) - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow null routers through .on on http router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const router = new director.http.Router() - - router.on('get', '/test/') - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow null routers through .on on cli router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const router = new director.cli.Router() - - router.on('get', 'test') - - helper.unloadAgent(agent) - t.end() - }) - - t.test('should allow null routers through .on on base router', function (t) { - const agent = helper.instrumentMockedAgent() - const director = require('director') - const router = new director.Router() - - router.on('get', 'test') - - helper.unloadAgent(agent) - t.end() - }) -}) diff --git a/test/versioned/director/newrelic.js b/test/versioned/director/newrelic.js deleted file mode 100644 index 5bfe53711f..0000000000 --- a/test/versioned/director/newrelic.js +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -'use strict' - -exports.config = { - app_name: ['My Application'], - license_key: 'license key here', - logging: { - level: 'trace', - filepath: '../../../newrelic_agent.log' - }, - utilization: { - detect_aws: false, - detect_pcf: false, - detect_azure: false, - detect_gcp: false, - detect_docker: false - }, - transaction_tracer: { - enabled: true - } -} diff --git a/test/versioned/director/package.json b/test/versioned/director/package.json deleted file mode 100644 index 3cfeb51f9f..0000000000 --- a/test/versioned/director/package.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "name": "director-tests", - "targets": [{"name":"director","minAgentVersion":"2.0.0"}], - "version": "0.0.0", - "private": true, - "tests": [ - { - "engines": { - "node": ">=16" - }, - "dependencies": { - "director": ">=1.2.0", - "express": "4.16" - }, - "files": [ - "director.tap.js" - ] - } - ], - "dependencies": {} -}