Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .ci/tav.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"cassandra-driver",
"elasticsearch",
"express",
"express-graphql",
"express-queue",
"fastify",
"finalhandler",
Expand Down
2 changes: 1 addition & 1 deletion .npmrc
Original file line number Diff line number Diff line change
@@ -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
Expand Down
39 changes: 0 additions & 39 deletions .tav.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 [email protected] 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'
Expand All @@ -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):
Expand All @@ -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'
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs/configuration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion docs/supported-technologies.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
|=======================================================================

Expand Down
2 changes: 1 addition & 1 deletion docs/upgrade-to-v3.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
2 changes: 1 addition & 1 deletion examples/trace-apollo-server-express.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }"}'
//
Expand Down
40 changes: 0 additions & 40 deletions examples/trace-express-graphql.js

This file was deleted.

4 changes: 2 additions & 2 deletions lib/instrumentation/modules/graphql.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
77 changes: 0 additions & 77 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 1 addition & 4 deletions test/config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
}
Expand Down
Loading