tests: stop testing 'express-graphql' instrumentation (#3304)

trentm · web-flow · commit c90d10d413aa · 2023-05-03T13:55:41.000-07:00
https://www.npmjs.com/package/express-graphql is deprecated. https://github.com/graphql/express-graphql is archived. The latest release, express-graphql@0.12.0 was 2022-11-19. Support for instrumenting this will remain, but we will stop testing it. A motivation for dropping this is that the `preinstall: npm uninstall express-graphql` parts of the `graphql` TAV testing could be significantly contributing to the graphql TAV tests being among the slowest; similar for `apollo-server-express` TAV tests.
diff --git a/.ci/tav.json b/.ci/tav.json
@@ -13,7 +13,6 @@
     "cassandra-driver",
     "elasticsearch",
     "express",
-    "express-graphql",
     "express-queue",
     "fastify",
     "finalhandler",
diff --git 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
@@ -184,12 +184,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
@@ -202,43 +200,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'
@@ -250,7 +213,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):
@@ -265,7 +227,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
@@ -68,6 +68,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
@@ -1008,15 +1008,15 @@ Example using options object:
 [source,js]
 ----
 require('elastic-apm-node').start({
-  disableInstrumentations: ['graphql', 'express-graphql']
+  disableInstrumentations: ['graphql', 'redis']
 })
 ----
 
 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
@@ -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
@@ -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
@@ -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
diff --git 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
diff --git a/package.json b/package.json
@@ -158,7 +158,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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-# Workaround unresolvable peerDependencies between express-graphql, graphql,`
	`1`	`+# Workaround unresolvable peerDependencies between graphql, @apollo-server,`
`2`	`2`	`# and apollo-server-express. npm v7 (included with node v15) makes these`
`3`	`3`	`# peerDependencies issues an install error. Until the community catches up`
`4`	`4`	`# and resolves peerDependencies issues or apm-agent-nodejs.git's tests are`