census-instrumentation / opencensus-cpp

A stats collection and distributed tracing framework
https://opencensus.io/
Apache License 2.0
145 stars 76 forks source link

CMake stackdriver enhancements #447

Open alichnewsky opened 4 years ago

alichnewsky commented 4 years ago

do not merge yet, it is still work in progress. I probably also left quite a few comments that need removing.

many enhancements to CMake support ( see #244 )

Ongoing issues / unresolved:

Unfortunately I did not read many of the outstanding PRs when I started this work, so it may be very redundant with ongoing efforts.

alichnewsky commented 4 years ago

tools/docker-format/Dockerfile is obsolete (#448).
hence, no pre-commit formatting, and significant effort to get the linux/ubuntu ci builds green.

related PR here #449.

alichnewsky commented 4 years ago

circular depdendency between opencensus_trace.so and opencensus_context.so building shared libraries with code as is gives a linking error:

[ 70%] Linking CXX shared library libopencensus_trace.so
CMakeFiles/opencensus_trace.dir/internal/context_util.cc.o: In function `opencensus::trace::GetCurrentSpan()':
context_util.cc:(.text+0x5): undefined reference to `opencensus::context::Context::Current()'
CMakeFiles/opencensus_trace.dir/internal/with_span.cc.o: In function `opencensus::trace::WithSpan::WithSpan(opencensus::trace::Span const&, bool, bool)':
with_span.cc:(.text+0x4d): undefined reference to `opencensus::context::Context::InternalMutableCurrent()'
CMakeFiles/opencensus_trace.dir/internal/with_span.cc.o: In function `opencensus::trace::WithSpan::ConditionalSwap()':
with_span.cc:(.text+0xcf): undefined reference to `opencensus::context::Context::InternalMutableCurrent()'
CMakeFiles/opencensus_trace.dir/internal/with_span.cc.o: In function `opencensus::trace::WithSpan::~WithSpan()':
with_span.cc:(.text+0x10d): undefined reference to `opencensus::context::Context::InternalMutableCurrent()'
with_span.cc:(.text+0x1a4): undefined reference to `opencensus::context::Context::InternalMutableCurrent()'
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
opencensus/trace/CMakeFiles/opencensus_trace.dir/build.make:406: recipe for target 'opencensus/trace/libopencensus_trace.so' failed
make[2]: *** [opencensus/trace/libopencensus_trace.so] Error 1
make[2]: Leaving directory '/opencensus-cpp/cmake-out'
CMakeFiles/Makefile2:8650: recipe for target 'opencensus/trace/CMakeFiles/opencensus_trace.dir/all' failed
make[1]: *** [opencensus/trace/CMakeFiles/opencensus_trace.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

adding context as a direct depdendency to opencensus_trace.so would give a configuration error. Cyclic depdendencies.

...
-- Configuring done
CMake Error: The inter-target dependency graph contains the following strongly connected component (cycle):
  "opencensus_context" of type SHARED_LIBRARY
    depends on "opencensus_trace" (weak)
  "opencensus_trace" of type SHARED_LIBRARY
    depends on "opencensus_context" (weak)
At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies are allowed only among static libraries.
-- Build files have been written to: /opencensus-cpp/cmake-out
Makefile:2658: recipe for target 'cmake_check_build_system' failed
make: *** [cmake_check_build_system] Error 1
make: Leaving directory '/opencensus-cpp/cmake-out'

As a result, as of today, the only way to assemble this code is with a static library. Worthy of a separate issue ?