search

openzipkin / zipkin-js

Zipkin instrumentation for Node.js and browsers
Apache License 2.0
564 stars 171 forks source link

zipkin-instrumentation-redis will throw error if not set callback #194

Open sssession opened 6 years ago

sssession commented 6 years ago

https://github.com/openzipkin/zipkin-js/blob/master/packages/zipkin-instrumentation-redis/src/zipkinClient.js#L23

If I didn't set callback when execute redis command like del, will throw a error as below.

Record at (spanId=be186f731a57d3d5, parentId=be186f731a57d3d5, traceId=be186f731a57d3d5): Rpc("del")
Record at (spanId=be186f731a57d3d5, parentId=be186f731a57d3d5, traceId=be186f731a57d3d5): ServiceName("test")
Record at (spanId=be186f731a57d3d5, parentId=be186f731a57d3d5, traceId=be186f731a57d3d5): ServerAddr(serviceName="redis", host="InetAddress(undefined)", port=0)
Record at (spanId=be186f731a57d3d5, parentId=be186f731a57d3d5, traceId=be186f731a57d3d5): ClientSend()
Record at (spanId=be186f731a57d3d5, parentId=be186f731a57d3d5, traceId=be186f731a57d3d5): ClientRecv()
/Library/WebServer/projects/qr-wap-pay/node_modules/continuation-local-storage/context.js:55
    throw exception;
    ^

TypeError: callback.apply is not a function
    at /Library/WebServer/projects/qr-wap-pay/node_modules/zipkin-instrumentation-redis/lib/zipkinClient.js:31:18
    at /Library/WebServer/projects/qr-wap-pay/node_modules/zipkin-context-cls/lib/CLSContext.js:53:16
    at /Library/WebServer/projects/qr-wap-pay/node_modules/zipkin-context-cls/lib/CLSContext.js:42:18
    at Namespace.run (/Library/WebServer/projects/qr-wap-pay/node_modules/continuation-local-storage/context.js:48:5)
    at CLSContext.scoped (/Library/WebServer/projects/qr-wap-pay/node_modules/zipkin-context-cls/lib/CLSContext.js:41:21)
    at CLSContext.letContext (/Library/WebServer/projects/qr-wap-pay/node_modules/zipkin-context-cls/lib/CLSContext.js:51:19)
    at Tracer.letId (/Library/WebServer/projects/qr-wap-pay/node_modules/zipkin/lib/tracer/index.js:73:28)
    at zipkinCallback (/Library/WebServer/projects/qr-wap-pay/node_modules/zipkin-instrumentation-redis/lib/zipkinClient.js:30:14)
    at Object.callbackOrEmit [as callback_or_emit] (/Library/WebServer/projects/qr-wap-pay/node_modules/redis/lib/utils.js:89:9)
    at RedisClient.return_error (/Library/WebServer/projects/qr-wap-pay/node_modules/redis/index.js:701:11)

test code:

const zipkin = require('zipkin');
const CLSContext = require('zipkin-context-cls');
const Redis = require('redis');
const zipkinClient = require('zipkin-instrumentation-redis');

const ctxImpl = new CLSContext('zipkin');
const tracer = new zipkin.Tracer({
    ctxImpl,
    recorder: new zipkin.ConsoleRecorder(),
    localServiceName: 'test',
});
const redis = zipkinClient(tracer, Redis, {});

redis.del('test');

If should judgment the callback is set or not before apply callback ?

jcchavezs commented 6 years ago

@sssession can you please open a PR with the test case (and the potential fix)?

ShrawanLakhe commented 5 years ago

@sssession did you find any fixes?

jcchavezs commented 5 years ago

@ShrawanLakhe would you like to take this issue over?