diff --git a/.ci/tav.json b/.ci/tav.json index 146aabd26f..5a42657787 100644 --- a/.ci/tav.json +++ b/.ci/tav.json @@ -12,7 +12,6 @@ "cassandra-driver", "elasticsearch", "express", - "express-graphql", "express-queue", "fastify", "finalhandler", diff --git a/.npmrc b/.npmrc index 6553b68500..a9ac2ea3b1 100644 --- a/.npmrc +++ b/.npmrc @@ -1,4 +1,4 @@ -# Workaround unresolvable peerDependencies between express-graphql, graphql, +# Workaround unresolvable peerDependencies between graphql, @apollo-server, # and apollo-server-express. npm v7 (included with node v15) makes these # peerDependencies issues an install error. Until the community catches up # and resolves peerDependencies issues or apm-agent-nodejs.git's tests are diff --git a/.tav.yml b/.tav.yml index be49c4a31d..a8f9460284 100644 --- a/.tav.yml +++ b/.tav.yml @@ -173,12 +173,10 @@ ws-new: graphql-v0.7-v16: name: graphql - preinstall: npm uninstall express-graphql versions: '>=0.7.0 <0.11.0 || >=0.11.1 <16' commands: node test/instrumentation/modules/graphql.test.js graphql: name: graphql - preinstall: npm uninstall express-graphql node: '>=12' versions: '>=16.0.0 <17' commands: node test/instrumentation/modules/graphql.test.js @@ -191,43 +189,8 @@ express: - node test/instrumentation/modules/express/capture-exceptions-on.test.js - node test/instrumentation/modules/express/set-framework.test.js -# - Skip express-graphql@0.10.0 because it briefly, accidentally introduced -# syntax that required ES2020 (node v14). -# - Limit to node >=10.4 because of a known issue. -# https://github.com/elastic/apm-agent-nodejs/issues/2516 -express-graphql-0.10.1_graphql-14: - name: express-graphql - preinstall: npm uninstall apollo-server-express - peerDependencies: graphql@^14.6.0 - versions: '>=0.10.1 <0.11.0' - node: '>=10.4' - commands: node test/instrumentation/modules/express-graphql.test.js -express-graphql-0.10.1_graphql-15: - name: express-graphql - preinstall: npm uninstall apollo-server-express - peerDependencies: graphql@^15.0.0 - versions: '>=0.10.1 <0.11.0' - node: '>=10.4' - commands: node test/instrumentation/modules/express-graphql.test.js - -express-graphql-0.11.0_graphql-14: - name: express-graphql - preinstall: npm uninstall apollo-server-express - peerDependencies: graphql@^14.7.0 - versions: '>=0.11.0 <0.13.0' - node: '>=10.4' - commands: node test/instrumentation/modules/express-graphql.test.js -express-graphql-0.11.0_graphql-15: - name: express-graphql - preinstall: npm uninstall apollo-server-express - peerDependencies: graphql@^15.3.0 - versions: '>=0.11.0 <0.13.0' - node: '>=10.4' - commands: node test/instrumentation/modules/express-graphql.test.js - apollo-server-express-2_graphql-14: name: apollo-server-express - preinstall: npm uninstall express-graphql peerDependencies: graphql@^14.0.0 # We want this version range: # versions: '>=2.9.16 <2.2 || >= 2.3.2 <3' @@ -239,7 +202,6 @@ apollo-server-express-2_graphql-14: commands: node test/instrumentation/modules/apollo-server-express.test.js apollo-server-express-2_graphql-15: name: apollo-server-express - preinstall: npm uninstall express-graphql peerDependencies: graphql@^15.0.0 # We want this version range (2.12.0 was the first release of # apollo-server-express after graphql@15 was released): @@ -254,7 +216,6 @@ apollo-server-express-2_graphql-15: commands: node test/instrumentation/modules/apollo-server-express.test.js apollo-server-express-3_graphql-15: name: apollo-server-express - preinstall: npm uninstall express-graphql peerDependencies: graphql@^15.0.0 # We want this version range: # versions: '^3.0.0' diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 4a96e52016..5983b3b153 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -61,6 +61,9 @@ Notes: [float] ===== Chores +* Stop testing `express-graphql` instrumentation -- the module is deprecated. + ({pull}3304[#3304]) + [[release-notes-3.45.0]] ==== 3.45.0 2023/04/28 diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index 65554bd5e7..70919029e0 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -1008,7 +1008,7 @@ Example using options object: [source,js] ---- require('elastic-apm-node').start({ - disableInstrumentations: ['graphql', 'express-graphql'] + disableInstrumentations: ['graphql', 'redis'] }) ---- @@ -1016,7 +1016,7 @@ Example using environment variable: [source,bash] ---- -ELASTIC_APM_DISABLE_INSTRUMENTATIONS=graphql,express-graphql +ELASTIC_APM_DISABLE_INSTRUMENTATIONS=graphql,redis ---- For an always up-to-date list of modules for which instrumentation can be disabled, diff --git a/docs/supported-technologies.asciidoc b/docs/supported-technologies.asciidoc index 50696c8b37..185b89cadd 100644 --- a/docs/supported-technologies.asciidoc +++ b/docs/supported-technologies.asciidoc @@ -125,7 +125,7 @@ These modules override that behavior to give better insights into specialized HT [options="header"] |======================================================================= |Module |Version |Note -|https://www.npmjs.com/package/express-graphql[express-graphql] |>=0.6.1 <0.13.0 |Will name all transactions by the GraphQL query name. There is a https://github.com/elastic/apm-agent-nodejs/issues/2516[known issue with node <10.4]. Versions before 0.10.0 are no longer tested. +|https://www.npmjs.com/package/express-graphql[express-graphql] |>=0.6.1 <0.13.0 |Will name all transactions by the GraphQL query name. There is a https://github.com/elastic/apm-agent-nodejs/issues/2516[known issue with node <10.4]. This module is deprecated and is no longer tested. |https://www.npmjs.com/package/apollo-server-express[apollo-server-express] |>=2.0.4 <4|Will name all transactions by the GraphQL query name. Versions before 2.9.6 are no longer tested. |======================================================================= diff --git a/docs/upgrade-to-v3.asciidoc b/docs/upgrade-to-v3.asciidoc index 97fa9852aa..8b6c30382a 100644 --- a/docs/upgrade-to-v3.asciidoc +++ b/docs/upgrade-to-v3.asciidoc @@ -50,7 +50,7 @@ The following deprecated API's has been removed: [[v3-changes-in-collected-data]] ==== Changes in collected data -When instrumenting a GraphQL server that is run by either https://www.npmjs.com/package/apollo-server-express[`apollo-server-express`] or https://www.npmjs.com/package/express-graphql[`express-graphql`], +When instrumenting a GraphQL server that is run by https://www.npmjs.com/package/apollo-server-express[`apollo-server-express`] the Transaction type is now `graphql` instead of `request`. All Spans whose type was previously `ext` is now `external`. diff --git a/examples/trace-apollo-server-express.js b/examples/trace-apollo-server-express.js index c5ccda6b75..f62116fb5f 100644 --- a/examples/trace-apollo-server-express.js +++ b/examples/trace-apollo-server-express.js @@ -15,7 +15,7 @@ // export ELASTIC_APM_SERVER_URL=... // export ELASTIC_APM_SECRET_TOKEN=... // - Start the small GraphQL server: -// node examples/trace-express-graphql.js +// node examples/trace-apollo-server-express.js // - Make a GraphQL client request. E.g.: // curl -i localhost:3000/graphql -X POST -H content-type:application/json -d'{"query":"query HelloQuery { hello }"}' // diff --git a/examples/trace-express-graphql.js b/examples/trace-express-graphql.js deleted file mode 100644 index c926b3b573..0000000000 --- a/examples/trace-express-graphql.js +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and other contributors where applicable. - * Licensed under the BSD 2-Clause License; you may not use this file except in - * compliance with the BSD 2-Clause License. - */ - -// A small example showing Elastic APM tracing of 'express-graphql'. -// -// Usage: -// - `node examples/trace-express-graphql.js` to run a small GraphQL server. -// - Make a GraphQL client request. E.g.: -// curl -i localhost:3000/graphql -X POST -H content-type:application/json -d'{"query":"query HelloQuery { hello }"}' -// -// In the Elastic APM app you should expect to see a transaction and span -// something like: -// -// transaction "HelloQuery hello (/graphql)" -// `- span "GraphQL: HelloQuery hello" - -require('../').start({ // elastic-apm-node - serviceName: 'example-trace-express-graphql', - logUncaughtExceptions: true -}) - -const graphql = require('graphql') -const express = require('express') -const { graphqlHTTP } = require('express-graphql') - -var schema = graphql.buildSchema('type Query { hello: String }') -var root = { - hello () { - return 'Hello world!' - } -} - -var app = express() -app.use('/graphql', graphqlHTTP({ schema: schema, rootValue: root })) -app.listen(3000, function () { - console.warn('listening on http://localhost:3000') -}) diff --git a/lib/instrumentation/modules/graphql.js b/lib/instrumentation/modules/graphql.js index b9ef4e78a5..214cb80d11 100644 --- a/lib/instrumentation/modules/graphql.js +++ b/lib/instrumentation/modules/graphql.js @@ -140,8 +140,8 @@ module.exports = function (graphql, agent, { version, enabled }) { span.name = 'GraphQL: ' + (operationName ? operationName + ' ' : '') + queries.join(', ') } - // `_graphqlRoute` is a boolean set in instrumentations of other modules - // that specify 'graphql' in peerDependencies (e.g. 'express-graphql') to + // `_graphqlRoute` is a boolean, set in instrumentations of other modules + // that specify 'graphql' in peerDependencies (e.g. '@apollo/server') to // indicate that this transaction is for a GraphQL request. const trans = span.transaction if (trans._graphqlRoute) { diff --git a/package-lock.json b/package-lock.json index c05a7da5a2..fd1c486ddd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -80,7 +80,6 @@ "eslint-plugin-promise": "^4.3.1", "eslint-plugin-standard": "^4.1.0", "express": "^4.17.1", - "express-graphql": "^0.12.0", "express-queue": "^0.0.13", "fastify": "^4.16.3", "finalhandler": "^1.1.2", @@ -7153,49 +7152,6 @@ "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=", "dev": true }, - "node_modules/express-graphql": { - "version": "0.12.0", - "resolved": "https://registry.npmjs.org/express-graphql/-/express-graphql-0.12.0.tgz", - "integrity": "sha512-DwYaJQy0amdy3pgNtiTDuGGM2BLdj+YO2SgbKoLliCfuHv3VVTt7vNG/ZqK2hRYjtYHE2t2KB705EU94mE64zg==", - "dev": true, - "dependencies": { - "accepts": "^1.3.7", - "content-type": "^1.0.4", - "http-errors": "1.8.0", - "raw-body": "^2.4.1" - }, - "engines": { - "node": ">= 10.x" - }, - "peerDependencies": { - "graphql": "^14.7.0 || ^15.3.0" - } - }, - "node_modules/express-graphql/node_modules/http-errors": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.8.0.tgz", - "integrity": "sha512-4I8r0C5JDhT5VkvI47QktDW75rNlGVsUf/8hzjCC/wkWI/jdTRmBb9aI7erSG82r1bjKY3F6k28WnsVxB1C73A==", - "dev": true, - "dependencies": { - "depd": "~1.1.2", - "inherits": "2.0.4", - "setprototypeof": "1.2.0", - "statuses": ">= 1.5.0 < 2", - "toidentifier": "1.0.0" - }, - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/express-graphql/node_modules/toidentifier": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/toidentifier/-/toidentifier-1.0.0.tgz", - "integrity": "sha512-yaOH/Pk/VEhBWWTlhI+qXxDFXlejDGcQipMlyxda9nthulaxLZUNcUqFxokp0vcYnvteJln5FNQDRrxj3YcbVw==", - "dev": true, - "engines": { - "node": ">=0.6" - } - }, "node_modules/express-queue": { "version": "0.0.13", "resolved": "https://registry.npmjs.org/express-queue/-/express-queue-0.0.13.tgz", @@ -21123,39 +21079,6 @@ } } }, - "express-graphql": { - "version": "0.12.0", - "resolved": "https://registry.npmjs.org/express-graphql/-/express-graphql-0.12.0.tgz", - "integrity": "sha512-DwYaJQy0amdy3pgNtiTDuGGM2BLdj+YO2SgbKoLliCfuHv3VVTt7vNG/ZqK2hRYjtYHE2t2KB705EU94mE64zg==", - "dev": true, - "requires": { - "accepts": "^1.3.7", - "content-type": "^1.0.4", - "http-errors": "1.8.0", - "raw-body": "^2.4.1" - }, - "dependencies": { - "http-errors": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.8.0.tgz", - "integrity": "sha512-4I8r0C5JDhT5VkvI47QktDW75rNlGVsUf/8hzjCC/wkWI/jdTRmBb9aI7erSG82r1bjKY3F6k28WnsVxB1C73A==", - "dev": true, - "requires": { - "depd": "~1.1.2", - "inherits": "2.0.4", - "setprototypeof": "1.2.0", - "statuses": ">= 1.5.0 < 2", - "toidentifier": "1.0.0" - } - }, - "toidentifier": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/toidentifier/-/toidentifier-1.0.0.tgz", - "integrity": "sha512-yaOH/Pk/VEhBWWTlhI+qXxDFXlejDGcQipMlyxda9nthulaxLZUNcUqFxokp0vcYnvteJln5FNQDRrxj3YcbVw==", - "dev": true - } - } - }, "express-queue": { "version": "0.0.13", "resolved": "https://registry.npmjs.org/express-queue/-/express-queue-0.0.13.tgz", diff --git a/package.json b/package.json index 164c354a66..732f35292f 100644 --- a/package.json +++ b/package.json @@ -157,7 +157,6 @@ "eslint-plugin-promise": "^4.3.1", "eslint-plugin-standard": "^4.1.0", "express": "^4.17.1", - "express-graphql": "^0.12.0", "express-queue": "^0.0.13", "fastify": "^4.16.3", "finalhandler": "^1.1.2", diff --git a/test/config.test.js b/test/config.test.js index 525f2f747f..0c7bf12f5d 100644 --- a/test/config.test.js +++ b/test/config.test.js @@ -989,7 +989,6 @@ usePathAsTransactionNameTests.forEach(function (usePathAsTransactionNameTest) { }) test('disableInstrumentations', function (t) { - var expressGraphqlVersion = require('express-graphql/package.json').version var esVersion = safeGetPackageVersion('@elastic/elasticsearch') // require('apollo-server-core') is a hard crash on nodes < 12.0.0 @@ -1002,9 +1001,7 @@ test('disableInstrumentations', function (t) { if (isHapiIncompat('@hapi/hapi')) { modules.delete('@hapi/hapi') } - if (semver.lt(process.version, '7.6.0') && semver.gte(expressGraphqlVersion, '0.9.0')) { - modules.delete('express-graphql') - } + modules.delete('express-graphql') if (semver.lt(process.version, '10.0.0') && semver.gte(esVersion, '7.12.0')) { modules.delete('@elastic/elasticsearch') // - Version 7.12.0 dropped support for node v8. } diff --git a/test/instrumentation/modules/express-graphql.test.js b/test/instrumentation/modules/express-graphql.test.js deleted file mode 100644 index 53b01fe874..0000000000 --- a/test/instrumentation/modules/express-graphql.test.js +++ /dev/null @@ -1,236 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and other contributors where applicable. - * Licensed under the BSD 2-Clause License; you may not use this file except in - * compliance with the BSD 2-Clause License. - */ - -'use strict' - -if (process.env.GITHUB_ACTIONS === 'true' && process.platform === 'win32') { - console.log('# SKIP: GH Actions do not support docker services on Windows') - process.exit(0) -} - -var agent = require('../../..').start({ - serviceName: 'test', - secretToken: 'test', - captureExceptions: false, - metricsInterval: 0, - centralConfig: false, - spanStackTraceMinDuration: 0 // Always have span stacktraces. -}) - -// There is a known issue where express-graphql instrumentation does not work -// with express-graphql >=0.10.0 and node <10.4. -// https://github.com/elastic/apm-agent-nodejs/issues/2516 -const expressGraphqlVersion = require('../../../node_modules/express-graphql/package.json').version -const semver = require('semver') -if (semver.gte(expressGraphqlVersion, '0.10.0') && semver.lt(process.version, '10.4.0')) { - console.log(`# SKIP express-graphql@${expressGraphqlVersion} testing with node <10.4 (${process.version}) because of a known issue`) - process.exit() -} - -// graphql@16 crashes with node <12. -const graphqlVer = require('graphql/package.json').version -if (semver.satisfies(graphqlVer, '>=16') && !semver.satisfies(process.version, '>=12')) { - console.log(`# SKIP graphql@${graphqlVer} is incompatible with node ${process.version}`) - process.exit() -} - -var http = require('http') - -var buildSchema = require('graphql').buildSchema -var express = require('express') -var querystring = require('querystring') -var expressGraphql = require('express-graphql') -var test = require('tape') - -var mockClient = require('../../_mock_http_client') - -const paths = ['/graphql', '/'] - -// In express-graphql@0.10.0, `graphqlHTTP` changed from being the top export -// to an attribute of the export object. -const graphqlHTTP = expressGraphql.graphqlHTTP || expressGraphql - -paths.forEach(function (path) { - test(`POST ${path}`, function (t) { - resetAgent(done(t, 'hello', path)) - - var schema = buildSchema('type Query { hello: String }') - var root = { - hello () { - t.ok(agent._instrumentation.currTransaction(), 'have active transaction') - return 'Hello world!' - } - } - var query = '{"query":"{ hello }"}' - - var app = express() - app.use(path, graphqlHTTP({ schema: schema, rootValue: root })) - var server = app.listen(function () { - var port = server.address().port - var opts = { - method: 'POST', - port: port, - path, - headers: { 'Content-Type': 'application/json' } - } - var req = http.request(opts, function (res) { - var chunks = [] - res.on('data', chunks.push.bind(chunks)) - res.on('end', function () { - server.close() - var result = Buffer.concat(chunks).toString() - t.strictEqual(result, '{"data":{"hello":"Hello world!"}}') - agent.flush() - }) - }) - req.end(query) - }) - }) - - test(`GET ${path}`, function (t) { - resetAgent(done(t, 'hello', path)) - - var schema = buildSchema('type Query { hello: String }') - var root = { - hello () { - t.ok(agent._instrumentation.currTransaction(), 'have active transaction') - return 'Hello world!' - } - } - var query = querystring.stringify({ query: '{ hello }' }) - - var app = express() - app.use(path, graphqlHTTP({ schema: schema, rootValue: root })) - var server = app.listen(function () { - var port = server.address().port - var opts = { - method: 'GET', - port: port, - path: `${path}?${query}` - } - var req = http.request(opts, function (res) { - var chunks = [] - res.on('data', chunks.push.bind(chunks)) - res.on('end', function () { - server.close() - var result = Buffer.concat(chunks).toString() - t.strictEqual(result, '{"data":{"hello":"Hello world!"}}') - agent.flush() - }) - }) - req.end() - }) - }) - - test(`POST ${path} - named query`, function (t) { - resetAgent(done(t, 'HelloQuery hello', path)) - - var schema = buildSchema('type Query { hello: String }') - var root = { - hello () { - t.ok(agent._instrumentation.currTransaction(), 'have active transaction') - return 'Hello world!' - } - } - var query = '{"query":"query HelloQuery { hello }"}' - - var app = express() - app.use(path, graphqlHTTP({ schema: schema, rootValue: root })) - var server = app.listen(function () { - var port = server.address().port - var opts = { - method: 'POST', - port: port, - path, - headers: { 'Content-Type': 'application/json' } - } - var req = http.request(opts, function (res) { - var chunks = [] - res.on('data', chunks.push.bind(chunks)) - res.on('end', function () { - server.close() - var result = Buffer.concat(chunks).toString() - t.strictEqual(result, '{"data":{"hello":"Hello world!"}}') - agent.flush() - }) - }) - req.end(query) - }) - }) - - test(`POST ${path} - sort multiple queries`, function (t) { - resetAgent(done(t, 'hello, life', path)) - - var schema = buildSchema('type Query { hello: String, life: Int }') - var root = { - hello () { - t.ok(agent._instrumentation.currTransaction(), 'have active transaction') - return 'Hello world!' - }, - life () { - t.ok(agent._instrumentation.currTransaction(), 'have active transaction') - return 42 - } - } - var query = '{"query":"{ life, hello }"}' - - var app = express() - app.use(path, graphqlHTTP({ schema: schema, rootValue: root })) - var server = app.listen(function () { - var port = server.address().port - var opts = { - method: 'POST', - port: port, - path, - headers: { 'Content-Type': 'application/json' } - } - var req = http.request(opts, function (res) { - var chunks = [] - res.on('data', chunks.push.bind(chunks)) - res.on('end', function () { - server.close() - var result = Buffer.concat(chunks).toString() - t.strictEqual(result, '{"data":{"life":42,"hello":"Hello world!"}}') - agent.flush() - }) - }) - req.end(query) - }) - }) -}) - -function done (t, query, path) { - return function (data, cb) { - t.strictEqual(data.transactions.length, 1) - t.strictEqual(data.spans.length, 1) - - var trans = data.transactions[0] - var span = data.spans[0] - - t.strictEqual(trans.name, `${query} (${path})`) - t.strictEqual(trans.type, 'graphql') - t.strictEqual(span.name, 'GraphQL: ' + query) - t.strictEqual(span.type, 'db') - t.strictEqual(span.subtype, 'graphql') - t.strictEqual(span.action, 'execute') - - var offset = span.timestamp - trans.timestamp - t.ok(offset + span.duration * 1000 < trans.duration * 1000) - - t.end() - } -} - -function resetAgent (cb) { - agent._instrumentation.testReset() - // Cannot use the 'expected' argument to mockClient, because the way the - // tests above are structured, there is a race between the mockClient - // receiving events from the APM agent and the graphql request receiving a - // response. Using the 200ms delay in mockClient slows things down such that - // "done" should always come last. - agent._transport = mockClient(cb) - agent.captureError = function (err) { throw err } -}