Support redis tracing through the plugin API #377

DominicKramer · 2017-02-08T16:25:45Z

No description provided.

src/plugins/plugin-redis.js

+        labels.result = res;
+      }
+    }
+    Object.keys(labels).forEach(function(key) {


src/plugins/plugin-redis.js

+
+function sendCommandWrap(api, send_command) {
+  return function send_command_trace(cmd, args, cb) {
+    var root = api.getTransaction();


src/plugins/plugin-redis.js

+    file: '',
+    versions: '<2.3.x',
+    patch: function(redis, api) {
+      patchBelowV2_6(redis, api);


src/plugins/plugin-redis.js

+    file: '',
+    versions: '2.6.x',
+    patch: function(redis, api) {
+      patchEqualV2_6(redis, api);


src/plugins/plugin-redis.js

+  return api.wrap(fn);
+}
+
+////////////////////////////////////////////////////////////////////////////////


src/plugins/plugin-redis.js

+    name: 'redis-' + cmd,
+    skipFrames: skipped_frames + 1
+  });
+  Object.keys(labels).forEach(function(key) {


src/plugins/plugin-redis.js

+
+function internalSendCommandWrap(api, internal_send_command) {
+  return function internal_send_command_trace(cmd, args, cb) {
+    var root = api.getRootSpan();


src/plugins/plugin-redis.js

+
+function sendCommandWrap(api, send_command) {
+  return function send_command_trace(cmd, args, cb) {
+    var root = api.getRootSpan();


src/plugins/plugin-redis.js

+  return api.wrap(fn);
+}
+
+function wrapInternalSendCommand(redis, api) {


kjin

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

+
+function internalSendCommandWrap(api, internal_send_command) {
+  return function internal_send_command_trace(cmd, args, cb) {
+    if (arguments.length === 1 && typeof cmd === 'object') {


src/plugins/plugin-redis.js

+}
+
+function startSpanFromArguments(api, cmd, args, cb, send_command) {
+  if (!cmd || !args || typeof cmd !== 'string' || !Array.isArray(args) ||


kjin

LGTM as long as comments are made. No need to change the code structurally.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 8, 2017

DominicKramer force-pushed the plugin/implement-redis branch from e013d67 to 8226276 Compare February 8, 2017 16:26

kjin suggested changes Feb 10, 2017

View reviewed changes

DominicKramer force-pushed the plugin/implement-redis branch 3 times, most recently from d94d518 to 9e9e5e6 Compare February 15, 2017 22:43

DominicKramer added 10 commits February 16, 2017 13:27

Initial commit

45b3ade

Now unit tests pass

7249f0f

Remove dependence on span-data.js

1330a79

Delete hook-redis.js since it is no longer needed

13555a1

Clean up the code and implement unpatch

e3069b0

Remove redundant code

3a711e6

Remove an unnecessary unwrap call

8945340

Add a missing wrap/unwrap pair for redis 2.6

7a24465

Resolved some GitHub suggestions

260c429

Update to reflect changes to the plugin API

d033ba7

DominicKramer force-pushed the plugin/implement-redis branch from 9e9e5e6 to d033ba7 Compare February 16, 2017 21:27

matthewloring suggested changes Feb 16, 2017

View reviewed changes

Resolve some GitHub comments

5ac713b

matthewloring approved these changes Feb 16, 2017

View reviewed changes

kjin suggested changes Feb 16, 2017

View reviewed changes

kjin approved these changes Feb 16, 2017

View reviewed changes

Add a comment and a TODO

937672e

DominicKramer merged commit bb926ac into googleapis:master Feb 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support redis tracing through the plugin API #377

Support redis tracing through the plugin API #377

DominicKramer commented Feb 8, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

kjin left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

kjin left a comment •

edited

Loading

Support redis tracing through the plugin API #377

Support redis tracing through the plugin API #377

Conversation

DominicKramer commented Feb 8, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

kjin left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

kjin left a comment • edited Loading

Choose a reason for hiding this comment

kjin left a comment •

edited

Loading