Skip to content
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

[suggestion] Better scope definition in the Readme #14

Open
yurishkuro opened this issue May 9, 2022 · 12 comments
Open

[suggestion] Better scope definition in the Readme #14

yurishkuro opened this issue May 9, 2022 · 12 comments

Comments

@yurishkuro
Copy link

It's not very clear what kind of OTEL integration this approach provide. Does it produce metrics? Distributed traces? Logs?

@edeNFed
Copy link
Contributor

edeNFed commented May 10, 2022

Hi @yurishkuro,
This project currently supports distributed traces. I guess in the future we will also support metrics.
I will update the README to reflect that. Thanks for the feedback.

@yurishkuro
Copy link
Author

When you say it supports distributed traces, does it works on a completely uninstrumented code base, or are there certain prerequisites? E.g., do you use eBPF to inject trace context into the requests going on the wire?

@edeNFed
Copy link
Contributor

edeNFed commented May 13, 2022

It does work on uninstrumented code bases (that's one of the main goals of this project).

As we are still in the early days of this project, headers are not currently automatically added. This is something that we are currently working on and should be ready very soon.

@yurishkuro
Copy link
Author

It would also be very useful to have some kind of architecture document that discusses how exactly you're planning to implement context prop, as that is by far the most complex part of distributed tracing instrumentation.

@edeNFed
Copy link
Contributor

edeNFed commented May 19, 2022

Agree. I will let you know once the document is finished.

@edeNFed
Copy link
Contributor

edeNFed commented Jun 30, 2022

Hi @yurishkuro, I just merged a design doc describing how we are going to implement context propagation.
I'll be happy to hear your thoughts.

@yurishkuro
Copy link
Author

Thanks for sharing the doc. From my understanding, the proposal works as long as the execution of a request remains on the same goroutine. This is not always the case and I often encountered services that use other goroutines during request execution (or after). In most cases the Context object for those is passed manually. I did not see anything about that in the doc, so I assume this is currently out of scope - a reasonable position, but worth calling out explicitly, to manage expectations.

Another question, just out of curiosity: a few years back there were musings about obtaining goroutine IDs and how it was a very bad idea. Your proposal depends on goroutine IDs. I assume those are more easily accessible if you look under the hood of Go runtime, but is that a reliable way?

@edeNFed
Copy link
Contributor

edeNFed commented Jul 1, 2022

You are correct. Currently, spans will belong to the same trace if they are executed on the same goroutine. By using the same uprobes we are already using, we will be able to track the goroutine creation tree (which goroutine created which) and come up with a better logic that could handle cases like what you described. I edited the design doc to reflect that decision. I think this is something we definitely want to implement, but it will probably happen at a later stage of this project. I wonder how other auto instrumentations for multithreaded languages handle this (java for example), do you have any idea?

Regarding depending on goroutine ids: The main objection that I saw is that Go's internal structures like runtime.G can greatly change between Go versions and therefore it is hard to assume which offset the goid field will have in the G struct. This is solved by automatically tracking the goroutine id field in every version of Go. In the very unlikely case that the goid field will be removed (Go's scheduler also need some way to track goroutines) the instrumentation will fail at compile time. We then will probably write a different logic based on some other unique identifier for goroutines.

@yurishkuro
Copy link
Author

I wonder how other auto instrumentations for multithreaded languages handle this (java for example), do you have any idea?

In Java it's very common these days for request to jump across many threads because of futures-based programming models. So if auto-instrumentation doesn't handle that it's pretty useless. Fortunately, in Java there are well-defined entry points for those transitions (e.g. Executors), so it's easier to track.

@edeNFed
Copy link
Contributor

edeNFed commented Sep 20, 2022

@yurishkuro I am going to demo a new version of the project that includes automatic context propagation via eBPF and without dependency on goroutine id in today's Go auto instrumentation SIG meeting at 9:30AM PT. You are welcome to join if you are interested.

@edeNFed
Copy link
Contributor

edeNFed commented Sep 29, 2022

Version v0.6.0 was released with context propagation. You are welcome to try the updated getting started guide

@vasiliy-grinko
Copy link

Hi @yurishkuro, This project currently supports distributed traces. I guess in the future we will also support metrics. I will update the README to reflect that. Thanks for the feedback.

do you have any ideas how to implement metrics? I wanna add some metrics for an application and don't know yet what would be a better solution, I saw that net/http instrumentation supports metrics and traces, but I see only traces there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants