fix: propagate context in koa tracing #594

kjin · 2017-11-13T03:56:43Z

Fixes #593

The Koa tracing plugin ignored the return value of api.wrap, resulting in trace context not being propagated correctly. This affects both Koa 1 and 2, but Koa 1 needs different patching logic altogether (as it relies on generator functions).

src/plugins/plugin-koa.ts

@@ -16,6 +16,7 @@
 'use strict';

 const shimmer = require('shimmer');
+const co = require('co');
 var urlParse = require('url').parse;

 function startSpanForRequest(api, req, res, next) {


src/plugins/plugin-koa.ts

@@ -16,6 +16,7 @@
 'use strict';

 const shimmer = require('shimmer');
+const co = require('co');


test/plugins/test-trace-koa.ts

@@ -42,7 +42,7 @@ describe('koa', function() {
  });

  Object.keys(appBuilders).forEach(function(version) {
-    describe(version, function() {
+    describe.skip(version, function() {


src/plugins/plugin-koa.ts

-
-    startSpanForRequest(api, req, res, next);
-
+    console.log(next);


src/plugins/plugin-koa.ts

-
+    console.log(next);
+    next = startSpanForRequest(api, req, res, next);
+    console.log(next);


src/plugins/plugin-koa.ts

@@ -16,6 +16,7 @@
 'use strict';

 const shimmer = require('shimmer');
+const co = require('co');
 var urlParse = require('url').parse;

 function startSpanForRequest(api, req, res, next) {


jinwoo

I hope the functions are type-declared (at least for exported functions). But I guess that can be a separate PR.

kjin · 2017-11-13T23:43:58Z

@jinwoo Yep, adding types for plugins is in the pipeline. I'm trying to figure out how to import types for multiple versions of the same module, which is the major blocker.

fix: propagate context in koa tracing

9c3487a

kjin requested review from ofrobots, jinwoo, gaofanmichael and DominicKramer November 13, 2017 03:56

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

jmdobry added the in progress label Nov 13, 2017

jinwoo reviewed Nov 13, 2017

View reviewed changes

kjin force-pushed the koa-prop branch 2 times, most recently from 870d481 to 21c4c2c Compare November 13, 2017 19:43

jinwoo approved these changes Nov 13, 2017

View reviewed changes

kjin force-pushed the koa-prop branch from 21c4c2c to f7956cf Compare November 13, 2017 20:59

fix: re-enable skipped koa tests

08dd9bd

kjin force-pushed the koa-prop branch from f7956cf to 08dd9bd Compare November 13, 2017 23:41

kjin merged commit b9e6a3b into googleapis:master Nov 14, 2017

jmdobry removed the in progress label Nov 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: propagate context in koa tracing #594

fix: propagate context in koa tracing #594

kjin commented Nov 13, 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.

jinwoo left a comment

kjin commented Nov 13, 2017

fix: propagate context in koa tracing #594

fix: propagate context in koa tracing #594

Conversation

kjin commented Nov 13, 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.

jinwoo left a comment

Choose a reason for hiding this comment

kjin commented Nov 13, 2017