-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
local tracing #104
Comments
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)) |
We can do something like const result = tracer.local('myoperation', () => {
return someComputation();
}); then you're guaranteed that start() and finish() will be called correctly. |
@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? |
It was never implemented, just a sketch. You could use it for actions that cause HTTP requests as well, it could work with promises. |
So only the workaround is currently possible? |
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>
.
|
as promised #156 |
couple things to consider before merge. Please comment within the next couple days if you can, as we should do a release soon.
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
Reason is all the operations in tracer are prefixed |
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 |
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. |
There are a number of zipkin libraries supporting local tracing (operations that spawn in-process and are not RPCs).
We've recently had a request by @wirehead for how to do this.
workaround
I suggested a workaround, which is to use Client annotations while we figure out how we want to address this here.
There's a background issue in finagle which might steer implementation. twitter/finagle#541
cc @fedj
The text was updated successfully, but these errors were encountered: