Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

OpenTracing support for dynamic loading. #28

Closed
rnburn opened this issue Oct 24, 2017 · 17 comments
Closed

OpenTracing support for dynamic loading. #28

rnburn opened this issue Oct 24, 2017 · 17 comments

Comments

@rnburn
Copy link
Contributor

rnburn commented Oct 24, 2017

Many C/C++ projects (e.g. nginx, haproxy, varnish, envoy) that would benefit from OpenTracing instrumentation are distributed as binary executables instead of libraries.

When instrumenting such projects, the practice so far has been to write code for each tracer that exposes its configuration options as part of the applications configuration. (See for example nginx-opentracing/lightstep and nginx-opentracing/zipkin).

This increases the cost of maintaining these projects and goes against OpenTracing's goal of making it easy to switch tracing implementations. Furthermore, since many of the projects are used as building blocks in larger systems that interact with the application's configuration, the cost compounds since those systems also need to be aware a tracer's configuration. (See for example how OpenTracing was incorporated into Kubernetes' ingress-nginx controller).

I propose we adopt some conventions for C++ tracers that would allow applications to load tracers in dynamically using dlopen and decouple applications from specific tracer configuration options.

This could be done by standardizing on a function that could be looked up from a tracer's shared library using dlsym. Consider for example a C function (to avoid mangling) like

extern "C" int opentracing_make_vendor_tracer(void** tracer, const char* json_configuration);

that would take in a configuration string for the tracer in JSON, set tracer, and return 0 on success or some non-zero error code on failure.

A user of an application like nginx, then, might configure a tracer by adding something like

opentracing_tracer_option access_token abc123;
opentracing_tracer_option collector_host localhost;
opentracing_load_tracer liblightstep_tracer.so;

into nginx.conf which would cause nginx to lookup opentracing_make_vendor_tracerin liblightstep_tracer.so and call it with

{
  "access_token": "abc123",
  "collector_host": "localhost"
}

avoiding the need to have specific configuration directives for each LightStep option.

Projects that want to avoid the pains associated with dynamic library management could still link in some tracers statically, but might also add dynamic loading functionality like this as an optional way to plug in additional tracers.

Standards around both trace encoding and transport (such as what's outlined here) might make it possible some day to have a vendor neutral tracer that could report to any tracing system and avoid the need to dynamically load anything in, but this would address a more immediate need to make OpenTracing scalable and maintainable for these projects.

@yurishkuro
Copy link
Member

Another alternative for configuration is environment variables (e.g. in Jaeger Java)

@isaachier
Copy link
Contributor

I agree with this sentiment. I think it boils down to who invokes the constructor. As of now, OpenTracing assumes the user installs the appropriate tracer. But this makes it impossible for users to change the tracer on the fly without changing the source code, etc. The solution involving dynamic loading definitely makes sense to me. @yurishkuro I think the focus is less about config and more about inversion of control. We need some generic opentracing factory for all potential vendor tracers.

@isaachier
Copy link
Contributor

I've been thinking about this and wondering how it is C/C++ specific. If I wanted to instrument a popular Java or Go application, I would face the same challenge. How is C or C++ unique here?

@rnburn
Copy link
Contributor Author

rnburn commented Oct 25, 2017

I don't think they are unique, other than I would guess a greater portion of the C/C++ project relevant for tracing are distributed as binary applications.

@yurishkuro
Copy link
Member

Go has the same problem, but in Java you can easily swap the jars, and there's a "tracer resolver" contrib project that dynamically loads the tracer from the classpath.

@isaachier
Copy link
Contributor

Any progress here? Just need a consistent name and a doc for it. There isn't really a good way I know of to guarantee any tracer must implement this method.

@rnburn
Copy link
Contributor Author

rnburn commented Nov 5, 2017

I did start working on it. I'm hoping to have a prototype of it working with nginx-opentracing, lightstep, and zipkin before proposing a PR for it.

This is what I have for the signature so far. Some of the goals are

  • Have the prototype weak so that you can link in multiple tracing libraries without getting a multiple definition error.
  • Pass in version information so you can detect when there's a mismatch.
  • Allow a mechanism to get back meaningful error messages from the tracer library.

It still needs some work. The method to declare weak attributes and dynamically load libraries differs on windows so I'm planning on adding some preprocessor code to handle that.

@isaachier
Copy link
Contributor

@rnburn isn't it easier to use the dlopen/dlsym interface to enforce these rules? That is how nginx itself loads the modules (https://github.com/nginx/nginx/blob/master/src/os/unix/ngx_dlopen.h).

@rnburn
Copy link
Contributor Author

rnburn commented Nov 6, 2017

@isaachier Not sure what you mean. I am using those functions to write the code that loads the libraries:
https://github.com/rnburn/opentracing-cpp/blob/dlopen/src/dynamic_load.cpp#L55

@isaachier
Copy link
Contributor

OK my confusion is why not enforce a pattern. That way the names are guaranteed to be unique. For example, the Jaeger C++ client might define opentracing_make_jaeger_tracer and that would solve the issues surrounding duplicate symbols.

@rnburn
Copy link
Contributor Author

rnburn commented Nov 6, 2017

I'm ok with that approach... would the function name be derived from the library name?

@isaachier
Copy link
Contributor

Yes. I think Lua uses this approach to "guess" the function name for shared objects imported via the require function call. It tries looking for a function called luaopen_<import_name>. You could do essentially the same thing with opentracing_make_<tracer_lib_name>_tracer.

@isaachier
Copy link
Contributor

This seems to be somewhat supported in Go using the somewhat new Plugin package. Wonder if it is worth standardizing in opentracing-go as well.

@rnburn
Copy link
Contributor Author

rnburn commented Nov 17, 2017

Makes sense to me.

But I'm having seconds thoughts on using the library name to determine the function to call. If a users were to rename their shared library (which sometimes happens), then it could break dynamic loading. Also, sometimes projects use an alternative name for their debug build (e.g. jaeger calls it libjaegertracingd.so) which could break dynamic loading as well.

@isaachier
Copy link
Contributor

True. I still think it might be better than using the same name with weak linkage. In practice, weak linkage does not seem to be very portable seeing as there is no standard way to do it.

@isaachier
Copy link
Contributor

Any progress here?

@rnburn
Copy link
Contributor Author

rnburn commented Dec 17, 2017

Yes, I was working on this last week. I've been developing the API alongside adding dynamic loading support to envoy. I'll likely be putting in a PR soon.

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

No branches or pull requests

3 participants