Skip to content
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.

Sign up for GitHub

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

fix: rewrite plugin loader #686

Merged
merged 6 commits into from
Mar 23, 2018
Merged

fix: rewrite plugin loader #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d7f1c12
refactor: remove trace-plugin-loader.ts
kjin Mar 20, 2018
9ba58b5
refactor: re-implement plugin loader
kjin Mar 20, 2018
e0e6599
test: rewrite plugin loader tests
kjin Feb 20, 2018
c40a9fd
test: update other tests
kjin Feb 20, 2018
bd9160e
fix: drop warning message for non-patched modules
kjin Mar 19, 2018
eb986a3
refactor: address comments
kjin Mar 21, 2018
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: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
node_modules/
!test/fixtures/loader/node_modules/

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

npm-debug.log
.DS_Store
.nyc_output/
Expand Down
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"node": ">=0.12"
},
"devDependencies": {
"@types/builtin-modules": "^2.0.0",
"@types/connect": "^3.4.31",
"@types/continuation-local-storage": "^3.2.1",
"@types/express": "^4.11.0",
Expand Down Expand Up @@ -97,13 +98,15 @@
},
"dependencies": {
"@google-cloud/common": "^0.16.2",
"builtin-modules": "^2.0.0",
"continuation-local-storage": "^3.2.1",
"extend": "^3.0.0",
"gcp-metadata": "^0.4.1",
"is": "^3.2.0",
"lodash.findindex": "^4.4.0",
"lodash.isequal": "^4.0.0",
"methods": "^1.1.1",
"require-in-the-middle": "^2.2.1",
"semver": "^5.4.1",
"shimmer": "^1.2.0",
"uuid": "^3.0.1"
Expand Down
10 changes: 7 additions & 3 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import * as extend from 'extend';
import * as path from 'path';
import * as PluginTypes from './plugin-types';
import {PluginLoaderConfig} from './trace-plugin-loader';
import * as pluginLoader from './trace-plugin-loader';
import {pluginLoader} from './trace-plugin-loader';
import {TraceAgent} from './trace-api';
import {traceWriter, TraceWriterConfig} from './trace-writer';
import * as traceUtil from './util';
Expand Down Expand Up @@ -116,7 +116,11 @@ function stop() {
if (traceAgent && traceAgent.isActive()) {
traceWriter.get().stop();
traceAgent.disable();
pluginLoader.deactivate();
try {
const loader = pluginLoader.get().deactivate();
} catch {
// Plugin loader wasn't even created. No need to de-activate
}
cls.destroyNamespace();
}
}
Expand Down Expand Up @@ -168,7 +172,7 @@ export function start(projectConfig?: Config): PluginTypes.TraceAgent {
});

traceAgent.enable(logger, config);
pluginLoader.activate(logger, config);
pluginLoader.create(logger, config).activate();

if (typeof config.projectId !== 'string' &&
typeof config.projectId !== 'undefined') {
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/plugin-knex.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
var shimmer = require('shimmer');
var util = require('util');

// knex 0.10.x and 0.11.x do not need patching

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

var VERSIONS = '>=0.12 <=0.13';

function patchClient(Client, api) {
Expand Down Expand Up @@ -54,6 +53,10 @@ module.exports = [
versions: VERSIONS,
patch: patchClient,
unpatch: unpatchClient
},
{
// knex 0.10.x and 0.11.x do not need patching
versions: '>=0.10 <=0.11'
}
];

Expand Down
Loading