-
Notifications
You must be signed in to change notification settings - Fork 39
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
improv: opencensus mysql #227
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to patch the lib/Pool
too so we can support ppl using the connection pool
|
||
if (this.internalFilesExports.Connection) { | ||
this.logger.debug('patching mysql.Connection.createQuery') | ||
shimmer.wrap(this.internalFilesExports.Connection, 'createQuery', this.getPatchCreateQuery()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to patch the prototype here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
/** Unpatches all Mysql patched functions. */ | ||
applyUnpatch (): void { | ||
shimmer.unwrap(this.internalFilesExports.Connection, 'createQuery') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and unwrap the prototype
src/census/plugins/mysql.ts
Outdated
return function (...args: any[]) { | ||
var span = plugin.tracer.startChildSpan('mysql-query', plugin.SPAN_MYSQL_QUERY_TYPE) | ||
if (span === null) return original.apply(this, arguments) | ||
var query = original.apply(this, arguments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace with const
} | ||
} | ||
|
||
private getPatchGetConnection() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should more something like this :
private getPatchGetConnection() {
const plugin = this
return (original: Function) => {
return function (cb: Function) {
return original.call(this, plugin.tracer.wrap(cb))
}
}
}
src/census/plugins/mysql.ts
Outdated
private getPatchGetConnection() { | ||
return (original: Function) => { | ||
return function (...args: any[]) { | ||
return original.call(this, this.tracer.wrap(arguments)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the call to this.tracer
will be undefined cause this
refer to the function scope.
arguments will not work too cause .wrap
accept function and not array
@@ -0,0 +1,145 @@ | |||
import { CoreTracer, RootSpan, SpanEventListener, logger } from '@pm2/opencensus-core' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need test for the connection pool patchs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding mysql for oc tracing