openzipkin / zipkin-js

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

Feature request: GRPC client/server interceptors #73

Open devinsba opened 7 years ago

devinsba commented 7 years ago

It would be nice to have interceptors similar to the grpc ones from openzipkin/brave

devinsba commented 7 years ago

Turns out there is a missing feature in the nodejs grpc library: https://github.com/grpc/grpc/issues/8767

codefromthecrypt commented 7 years ago

since zipkin is very much like google cloud trace, we can probably borrow approach from them https://github.com/GoogleCloudPlatform/cloud-trace-nodejs

devinsba commented 7 years ago

Their approach overall is quite nice, if I end up with time I'll take a crack at implementing it.

If someone else gets to it first: https://github.com/GoogleCloudPlatform/cloud-trace-nodejs/blob/master/src/hooks/userspace/hook-grpc.js

ethanrubio commented 7 years ago

👍 Would love to see this as well!

lnshi commented 7 years ago

+1

codefromthecrypt commented 7 years ago

https://github.com/grpc/proposal/pull/14 indicates a future interceptor model (which doesn't necessarily help now)

wdittmer commented 6 years ago

I am using the code that @lnshi created already. I created a wrapper function that handles getting the metadata and finishing when the function is done (for server side). It would be nice if this could be included/done automatically when using this library. But it is a bit over my head at the moment.

function trace(func) {
    return async (call, callback) => {
        global.ZIPKIN_GRPC_INTCP.uponServerRecvGrpcCall({
            serviceName: ZIPKIN_SERVICE_NAME,
            grpcMetadataFromIncomingCtx: call.metadata
        });

        try {
            await func(call, callback);
        } finally {
            global.ZIPKIN_GRPC_INTCP.uponServerFinishRespond();
        }
    }
}

Then when assigning the call to the grpc server you just use grpcMethod: trace(your_original_grpc_method)

your_original_grpc_method must be async, e.g.

async function your_original_grpc_method(call, callback) {
    try {
         // do stuff
         return callback(null, {message: 'message'});
    } catch (err) {
        return callback(err);
    }
}
jcchavezs commented 5 years ago

I think this issue is partially fixes by https://github.com/openzipkin/zipkin-js/tree/master/packages/zipkin-instrumentation-grpc-client. Ping @ghermeto

ghermeto commented 5 years ago

it was... and the bad news is that gRPC in javascript doesn't have server interceptors (yet).