-
Notifications
You must be signed in to change notification settings - Fork 7
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
Basic API Discussion #2
Comments
Sorry, tested the links this time :-) |
Agreed, I think you are running into the classic rust issue "different trait impls in the same match". I've been bitten by this myself in the past on other projects - I asked in the rust channel for some advice and they came up with a couple of approaches:
I'm still not sure which would be the best approach, but I hope we can come up with an approach where we don't have to box it all the way down in the API itself, since then we are requiring all downstream implementations to perform heap allocations (especially since most of the time you only have one impl that you are going to use I think). |
I am still getting used to rust so I'm likely to be thinking of it in incorrect terms.
I see your point about avoiding boxes to avoid heap allocations. I wonder if we can come up with an API based on associated types for applications that want to avid heap allocation and a boxing wrapper on it for applications that rather support runtime tracer selection. Would something like this help us? |
@stefano-pogliani yeah, one thing to keep in mind is: most of the time, I think people only want one tracer impl which is the one they use throughout their infrastructure. So I think we should optimize for the common case and then think about, maybe in the util package ? provide a boxed implementation of the tracer (or a similar solution) which helps with this case. |
I'm probably thinking of it from a too "opsy" point of view so I have a few questions to understand the goal better:
And my answers (which hopefully will help understand my point) are:
I know I am being painfully fixated on this point (and I'm sorry for that) but the complexity of supporting runtime selection of the tracer in rustracing is the reason I have started my own project (which I would have avoided otherwise). |
I guess my point is: regardless of how we provide a runtime tracer selection, can this be a first-class citizen alongside the API itself (even in a I'm happy to be responsible for keeping this wrapper layer (or whatever it is) up to date as the API evolves. |
@stefano-pogliani totally agreed that we need to keep this as a first-class feature. |
Well with that said 😄 shall we look at the API? I followed a bottom up approach following the opentracing specification:
@daschl do you think this approach would work? |
yes let's start with the I created #6 to track the |
One thing that doesn't make sense to me - the drafts you have seem to have everything on heap and consumed via trait objects, which is undesirable for high performance stacks. Rust's zero-cost idioms should be flexible enough to not require this. I'm going to guess that the underlying desire is to have purely runtime hooking-in of new drivers. Currently most things in the rust ecosystem are designed to be built locally by the apps using them: the pattern with cargo is to download source and build an optimised-for-your-compiler-and-settings build, rather than 'here have a binary blob that has the right entry points'. So I'd suggest we should be saying that that would be the use here (which is how e.g. rustracing_jaeger works). And - if we want to allow a binary distributed application to have binary-injected tracer implementations, we write a specific 'binary driver' backend that would sit alongside native 'jaeger', 'zipkin', 'lightbox' etc tracers, and take care of the indirection itself. A similar thunk could wrap any native tracer up for use via the binary driver. That would then give the following scenarios:
|
@stefano-pogliani regarding the interop issue: for an entire ecosystem to switch over to a new implementation will require changes end to end - expecting otherwise is to ignore the realities. Consider, for instance, that even if everything is dlopened and in just one language - containers won't have the new implementation available to dlopen until that container is rebuilt. And in a large enterprise noone is going to ship a new thing willy-nilly. In fact, wearing a project mgmt hat for a second I'd expect migration from tracer A to tracer B to look like this:
Doing a coordinated atomic pivot to a new tracer would be hard with just one service that runs processes. Doing it for hundreds or thousands of microservices is going to be a nightmare. So for the question of 'but everyone has to do something to use a new tracer' - my expectation is yes, yes they will. Should we make that as easy as possible? Sure. But lets not compromise on the common case - which is steady state, one tracer in use system wide, and that should be reliable, debuggable and fast. |
Hi @rbtcollins, I believe we agreed with @daschl that the API will be based on generic traits and not trait objects. For in-house applications this would add no value but for "out of the box" applications (e.g. database software) forcing every user to re-build the entire system with their tracer of choice seems ... harsh. My original use case (which rustracing could not easily accomodate IMHO) is an infrastructure tool. As for the interop issue: changing the tracer was not the use case I had in mind. I'm working on a PR for the |
Hey, ok cool thats good, sounds like my concerns are addressed. With respect to the infrastructure tool - you mean you want a precompiled binary that supports a fixed set of tracers at runtime? that sounds like either internally-boxed or an internally routing implementation of the traits would do : both of which should fit easily as layers on the API rustracing has. I'm likely missing some nuance though! I didn't mean to suggest that it was easy for anyone to rebuild everything - far from it. What I meant to suggest was that whatever the usual integration method for a particular component was, that that would be sufficient because there are always additional layers and vetting and coordination that take place in production environments. |
Is this project still alive? |
Hi @ddcprg. I believe https://opentelemetry.io/ is a new iteration on OpenTracing and OpenMetrics as an all in one. |
Indeed, OpenTelemetry is the way to go these days and the rust implementation is a good working progress over there. |
Since #1 went a bit off the rails towards API, let's continue here.
The text was updated successfully, but these errors were encountered: