local tracing about zipkin-js

[eirslett]

We can do something like

const result = tracer.local('myoperation', () => {
  return someComputation();
});

then you're guaranteed that start() and finish() will be called correctly.

[codefromthecrypt]

FWIW, here's a proposal I made in finagle a while back. I didn't implement it because there weren't enough people engaged on the topic. You know me.. rule-of-three

tracer.recordAnnotation(new Annotation.Start())
tracer.recordAnnotation(new Annotation.Operation(spanName))
tracer.recordAnnotation(new Annotation.Finish())
// or on abend
tracer.recordAnnotation(new Annotation.Error(message))

[codefromthecrypt]

works for me. @wirehead @fedj others.. any opinions?

[DanielMSchmidt]

@adriancole Thanks for linking this issue, but I am unable to guess the usage from this conversation as I don't know what was implemented and what was just a suggestion. Is there a piece of documentation on how to use this and when? Is it for every action that is not causing an HTTP request?

[eirslett]

It was never implemented, just a sketch. You could use it for actions that cause HTTP requests as well, it could work with promises.

[DanielMSchmidt]

So only the workaround is currently possible?

[codefromthecrypt]

I will spike this today if folks can help review it.. deal?

…

On Fri, Nov 10, 2017 at 5:32 AM, Daniel Schmidt ***@***.***> wrote: So only the workaround is currently possible? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#104 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD61-xQT4KsUqVvQv8HMCN-9C_Ru_ruks5s029XgaJpZM4NNtKd> .

[codefromthecrypt]

as promised #156

[codefromthecrypt]

couple things to consider before merge. Please comment within the next couple days if you can, as we should do a release soon.

• Maybe we should add a constructor parameter in Tracer for the local endpoint.

This can reduce duplication in all the instrumentation where we keep passing serviceName (and sometimes get it wrong). Most importantly here, we can attach a local endpoint to the span so that you can look it up (most importantly you can attach it without the user having to pass it around).

I would offer to add a constructor and backport all the instrumentation libraries to default to the tracer's localEndpoint. This would essentially deprecate having to pass serviceName at each instrumentation point. I'd merge that prior to the local tracing one, so that I can reuse it there.

• Maybe we should rename Tracer.local to Tracer.wrap(callable) or Tracer.recordingCallable(callable) Tracer.recordDuration(callable) or something..

Reason is all the operations in tracer are prefixed record. local as a keyword still makes sense to me, just some final notes prior to merge

[codefromthecrypt]

Added #159 which is the minimum change besides the main PR. This centralizes assignment of the local service name (similar to how is done in brave). feedback welcome

[codefromthecrypt]

ok ready to merge #156.. last warning :)

this keeps Tracer.local as a keyword that means "trace this callable" as no-one seems bothered about it.