Skip to content

Commit

Permalink
chore: address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
vmarchaud committed Dec 27, 2020
1 parent 685fece commit d249f3d
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 27 deletions.
30 changes: 12 additions & 18 deletions packages/opentelemetry-instrumentation-grpc/src/grpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
SpanKind,
SpanOptions,
Status,
setSpan,
} from '@opentelemetry/api';
import { RpcAttribute } from '@opentelemetry/semantic-conventions';
import {
Expand Down Expand Up @@ -227,7 +228,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
[RpcAttribute.GRPC_KIND]: spanOptions.kind,
});

plugin.tracer.withSpan(span, () => {
context.with(setSpan(context.active(), span), () => {
switch (type) {
case 'unary':
case 'client_stream':
Expand Down Expand Up @@ -306,7 +307,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
[RpcAttribute.GRPC_ERROR_MESSAGE]: err.message,
});
} else {
span.setStatus({ code: StatusCode.OK });
span.setStatus({ code: StatusCode.UNSET });
span.setAttribute(
RpcAttribute.GRPC_STATUS_CODE,
grpcClient.status.OK.toString()
Expand All @@ -319,7 +320,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
return callback(err, value, trailer, flags);
}

plugin.tracer.bind(call);
context.bind(call);
return (original as Function).call(self, call, patchedCallback);
}

Expand All @@ -338,7 +339,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
}
};

plugin.tracer.bind(call);
context.bind(call);
call.on('finish', () => {
span.setStatus(_grpcStatusCodeToSpanStatus(call.status.code));
span.setAttribute(
Expand Down Expand Up @@ -425,14 +426,8 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
const span = plugin.tracer.startSpan(name, {
kind: SpanKind.CLIENT,
});
return plugin.tracer.withSpan(span, () =>
plugin._makeGrpcClientRemoteCall(
original,
args,
metadata,
this,
plugin
)(span)
return context.with(setSpan(context.active(), span), () =>
plugin._makeGrpcClientRemoteCall(original, args, metadata, this)(span)
);
};
};
Expand All @@ -445,8 +440,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
original: GrpcClientFunc,
args: any[],
metadata: grpcTypes.Metadata,
self: grpcTypes.Client,
plugin: GrpcInstrumentation
self: grpcTypes.Client
) {
/**
* Patches a callback so that the current span for this trace is also ended
Expand All @@ -471,7 +465,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
[RpcAttribute.GRPC_ERROR_MESSAGE]: err.message,
});
} else {
span.setStatus({ code: StatusCode.OK });
span.setStatus({ code: StatusCode.UNSET });
span.setAttribute(
RpcAttribute.GRPC_STATUS_CODE,
grpcClient.status.OK.toString()
Expand All @@ -481,7 +475,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
span.end();
callback(err, res);
};
return plugin.tracer.bind(wrappedFn);
return context.bind(wrappedFn);
}

return (span: Span) => {
Expand Down Expand Up @@ -523,7 +517,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
spanEnded = true;
}
};
plugin.tracer.bind(call);
context.bind(call);
((call as unknown) as events.EventEmitter).on(
'error',
(err: grpcTypes.ServiceError) => {
Expand All @@ -542,7 +536,7 @@ export class GrpcInstrumentation extends InstrumentationBase<typeof grpcTypes> {
((call as unknown) as events.EventEmitter).on(
'status',
(status: Status) => {
span.setStatus({ code: StatusCode.OK });
span.setStatus({ code: StatusCode.UNSET });
span.setAttribute(
RpcAttribute.GRPC_STATUS_CODE,
status.code.toString()
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-instrumentation-grpc/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const _grpcStatusCodeToOpenTelemetryStatusCode = (
status?: grpcTypes.status
): StatusCode => {
if (status !== undefined && status === 0) {
return StatusCode.OK;
return StatusCode.UNSET;
}
return StatusCode.ERROR;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-instrumentation-grpc/src/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@
*/

// this is autogenerated file, see scripts/version-update.js
export const VERSION = '0.13.0';
export const VERSION = '0.14.0';
19 changes: 13 additions & 6 deletions packages/opentelemetry-instrumentation-grpc/test/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,15 @@
* limitations under the License.
*/

import { context, SpanKind, propagation } from '@opentelemetry/api';
import { NoopLogger, HttpTraceContext } from '@opentelemetry/core';
import {
context,
SpanKind,
propagation,
NoopLogger,
setSpan,
getSpan,
} from '@opentelemetry/api';
import { HttpTraceContext } from '@opentelemetry/core';
import { NodeTracerProvider } from '@opentelemetry/node';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import { ContextManager } from '@opentelemetry/context-base';
Expand Down Expand Up @@ -506,8 +513,8 @@ export const runTests = (
const span = provider
.getTracer('default')
.startSpan('TestSpan', { kind: SpanKind.PRODUCER });
return provider.getTracer('default').withSpan(span, async () => {
const rootSpan = provider.getTracer('default').getCurrentSpan();
return context.with(setSpan(context.active(), span), async () => {
const rootSpan = getSpan(context.active());
if (!rootSpan) {
return assert.ok(false);
}
Expand Down Expand Up @@ -602,8 +609,8 @@ export const runTests = (
const span = provider
.getTracer('default')
.startSpan('TestSpan', { kind: SpanKind.PRODUCER });
return provider.getTracer('default').withSpan(span, async () => {
const rootSpan = provider.getTracer('default').getCurrentSpan();
return context.with(setSpan(context.active(), span), async () => {
const rootSpan = getSpan(context.active());
if (!rootSpan) {
return assert.ok(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const grpcStatusCodeToOpenTelemetryStatusCode = (
status: grpc.status | grpcJs.status
): StatusCode => {
if (status !== undefined && status === 0) {
return StatusCode.OK;
return StatusCode.UNSET;
}
return StatusCode.ERROR;
};
Expand Down

0 comments on commit d249f3d

Please sign in to comment.