-
Notifications
You must be signed in to change notification settings - Fork 98
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
Support redis tracing through the plugin API #377
Support redis tracing through the plugin API #377
Conversation
e013d67
to
8226276
Compare
src/plugins/plugin-redis.js
Outdated
labels.result = res; | ||
} | ||
} | ||
Object.keys(labels).forEach(function(key) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-redis.js
Outdated
|
||
function sendCommandWrap(api, send_command) { | ||
return function send_command_trace(cmd, args, cb) { | ||
var root = api.getTransaction(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-redis.js
Outdated
file: '', | ||
versions: '<2.3.x', | ||
patch: function(redis, api) { | ||
patchBelowV2_6(redis, api); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-redis.js
Outdated
file: '', | ||
versions: '2.6.x', | ||
patch: function(redis, api) { | ||
patchEqualV2_6(redis, api); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-redis.js
Outdated
return api.wrap(fn); | ||
} | ||
|
||
//////////////////////////////////////////////////////////////////////////////// |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-redis.js
Outdated
name: 'redis-' + cmd, | ||
skipFrames: skipped_frames + 1 | ||
}); | ||
Object.keys(labels).forEach(function(key) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
d94d518
to
9e9e5e6
Compare
9e9e5e6
to
d033ba7
Compare
src/plugins/plugin-redis.js
Outdated
|
||
function internalSendCommandWrap(api, internal_send_command) { | ||
return function internal_send_command_trace(cmd, args, cb) { | ||
var root = api.getRootSpan(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-redis.js
Outdated
|
||
function sendCommandWrap(api, send_command) { | ||
return function send_command_trace(cmd, args, cb) { | ||
var root = api.getRootSpan(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
return api.wrap(fn); | ||
} | ||
|
||
function wrapInternalSendCommand(redis, api) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
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.
Looks good from a correctness point of view -- but I think there are a few points that could use additional clarity.
src/plugins/plugin-redis.js
Outdated
|
||
function internalSendCommandWrap(api, internal_send_command) { | ||
return function internal_send_command_trace(cmd, args, cb) { | ||
if (arguments.length === 1 && typeof cmd === 'object') { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/plugins/plugin-redis.js
Outdated
} | ||
|
||
function startSpanFromArguments(api, cmd, args, cb, send_command) { | ||
if (!cmd || !args || typeof cmd !== 'string' || !Array.isArray(args) || |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
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.
LGTM as long as comments are made. No need to change the code structurally.
No description provided.