Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

Support nlohmann json 2.0.x? #47

Closed
ringerc opened this issue Jan 28, 2018 · 14 comments
Closed

Support nlohmann json 2.0.x? #47

ringerc opened this issue Jan 28, 2018 · 14 comments

Comments

@ringerc
Copy link
Contributor

ringerc commented Jan 28, 2018

I realise everyone loves adding support for old versions of libraries, so this isn't exactly a priority.

But nlohmann json 2.0.x is only currently found "in the wild" on Debian Testing and Unstable. It's not in Fedora 27, which carries 2.0.2, or in Debian stable or AFAICS in backports. Nor in Ubuntu Artful; it'll be in Bionic in April. Certainly not RHEL etc.

The build fails against 2.0.x even once I work around the issue with the header renaming from json.hpp to nlohmann/json.hpp.

I just don't have the c++-fu to understand what's going on here. Feel free to ignore if you don't care about compat with older widely deployed libs; that's fair enough.

Error spam:

In file included from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/baggage/RemoteRestrictionJSON.cpp:17:0:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/baggage/RemoteRestrictionJSON.h: In function ‘void jaegertracing::thrift::to_json(nlohmann::json&, const BaggageRestrictionList&)’:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/baggage/RemoteRestrictionJSON.h:56:17: error: no match for ‘operator=’ (operand types are ‘nlohmann::json {aka nlohmann::basic_json<>}’ and ‘const std::vector<jaegertracing::thrift::BaggageRestriction>’)
     json = list.success;
                 ^~~~~~~
In file included from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/baggage/RemoteRestrictionJSON.h:27:0,
                 from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/baggage/RemoteRestrictionJSON.cpp:17:
/usr/include/json.hpp:2125:16: note: candidate: nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::value_type& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::operator=(nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>) [with ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::value_type = nlohmann::basic_json<>]
     reference& operator=(basic_json other) noexcept (
                ^~~~~~~~
/usr/include/json.hpp:2125:16: note:   no known conversion for argument 1 from ‘const std::vector<jaegertracing::thrift::BaggageRestriction>’ to ‘nlohmann::basic_json<>’
/usr/include/json.hpp: In instantiation of ‘ValueType nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get() const [with ValueType = jaegertracing::thrift::BaggageRestriction; typename std::enable_if<(! std::is_pointer<_Ptr>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]’:
/usr/include/json.hpp:2690:13:   required from ‘nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(std::vector<T>*) const::<lambda(nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>)> [with T = jaegertracing::thrift::BaggageRestriction; typename std::enable_if<(std::is_convertible<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value && (! std::is_same<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value)), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]’
/usr/include/json.hpp:2687:72:   required from ‘struct nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(std::vector<T>*) const [with T = jaegertracing::thrift::BaggageRestriction; typename std::enable_if<(std::is_convertible<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value && (! std::is_same<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value)), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]::<lambda(class nlohmann::basic_json<>)>’
/usr/include/json.hpp:2686:27:   required from ‘std::vector<T> nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(std::vector<T>*) const [with T = jaegertracing::thrift::BaggageRestriction; typename std::enable_if<(std::is_convertible<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value && (! std::is_same<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value)), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]’
/usr/include/json.hpp:2946:24:   required from ‘ValueType nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get() const [with ValueType = std::vector<jaegertracing::thrift::BaggageRestriction>; typename std::enable_if<(! std::is_pointer<_Ptr>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]’
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/baggage/RemoteRestrictionJSON.h:61:62:   required from here
/usr/include/json.hpp:2946:24: error: no matching function for call to ‘nlohmann::basic_json<>::get_impl(jaegertracing::thrift::BaggageRestriction*) const’
         return get_impl(static_cast<ValueType*>(nullptr));
                ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/json.hpp:2622:7: note: candidate: template<class T, typename std::enable_if<(std::is_convertible<std::__cxx11::basic_string<char>, typename T::key_type>::value && std::is_convertible<nlohmann::basic_json<>, typename T::mapped_type>::value), int>::type <anonymous> > T nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(T*) const [with T = T; typename std::enable_if<(std::is_convertible<typename ObjectType<StringType, nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, std::less<StringType>, AllocatorType<std::pair<const StringType, nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType> > > >::key_type, typename T::key_type>::value && std::is_convertible<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, typename T::mapped_type>::value), int>::type <anonymous> = <enumerator>; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]
     T get_impl(T*) const
       ^~~~~~~~
/usr/include/json.hpp:2622:7: note:   template argument deduction/substitution failed:
/usr/include/json.hpp:2619:97: error: no type named ‘key_type’ in ‘class jaegertracing::thrift::BaggageRestriction’
                   std::is_convertible<typename object_t::key_type, typename T::key_type>::value and
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
                   std::is_convertible<basic_json_t, typename T::mapped_type>::value
                   ~~~                                                                            
/usr/include/json.hpp:2619:97: error: no type named ‘mapped_type’ in ‘class jaegertracing::thrift::BaggageRestriction’
/usr/include/json.hpp:2621:34: note: invalid template non-type parameter
                   , int>::type = 0>
                                  ^
/usr/include/json.hpp:2635:14: note: candidate: nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::object_t nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::object_t*) const [with ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::object_t = std::map<std::__cxx11::basic_string<char>, nlohmann::basic_json<>, std::less<std::__cxx11::basic_string<char> >, std::allocator<std::pair<const std::__cxx11::basic_string<char>, nlohmann::basic_json<> > > >]
     object_t get_impl(object_t*) const
              ^~~~~~~~
/usr/include/json.hpp:2635:14: note:   no known conversion for argument 1 from ‘jaegertracing::thrift::BaggageRestriction*’ to ‘nlohmann::basic_json<>::object_t* {aka std::map<std::__cxx11::basic_string<char>, nlohmann::basic_json<>, std::less<std::__cxx11::basic_string<char> >, std::allocator<std::pair<const std::__cxx11::basic_string<char>, nlohmann::basic_json<> > > >*}’
/usr/include/json.hpp:2656:7: note: candidate: template<class T, typename std::enable_if<((((std::is_convertible<nlohmann::basic_json<>, typename T::value_type>::value && (! std::is_same<nlohmann::basic_json<>, typename T::value_type>::value)) && (! std::is_arithmetic<_Tp>::value)) && (! std::is_convertible<std::__cxx11::basic_string<char>, T>::value)) && (! nlohmann::{anonymous}::has_mapped_type<T>::value)), int>::type <anonymous> > T nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(T*) const [with T = T; typename std::enable_if<((((std::is_convertible<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, typename T::value_type>::value && (! std::is_same<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, typename T::value_type>::value)) && (! std::is_arithmetic<T>::value)) && (! std::is_convertible<std::__cxx11::basic_string<char>, T>::value)) && (! nlohmann::{anonymous}::has_mapped_type<T>::value)), int>::type <anonymous> = <enumerator>; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]
     T get_impl(T*) const
       ^~~~~~~~
/usr/include/json.hpp:2656:7: note:   template argument deduction/substitution failed:
/usr/include/json.hpp:2650:84: error: no type named ‘value_type’ in ‘class jaegertracing::thrift::BaggageRestriction’
                   std::is_convertible<basic_json_t, typename T::value_type>::value and
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
                   not std::is_same<basic_json_t, typename T::value_type>::value and
                   ~~~~~~~                                                           
/usr/include/json.hpp:2651:19: error: no type named ‘value_type’ in ‘class jaegertracing::thrift::BaggageRestriction’
                   not std::is_same<basic_json_t, typename T::value_type>::value and
                   ^~~~~~~
/usr/include/json.hpp:2655:34: note: invalid template non-type parameter
                   , int>::type = 0>
                                  ^
/usr/include/json.hpp:2680:20: note: candidate: template<class T, typename std::enable_if<(std::is_convertible<nlohmann::basic_json<>, T>::value && (! std::is_same<nlohmann::basic_json<>, T>::value)), int>::type <anonymous> > std::vector<T> nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(std::vector<T>*) const [with T = T; typename std::enable_if<(std::is_convertible<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value && (! std::is_same<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, T>::value)), int>::type <anonymous> = <enumerator>; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]
     std::vector<T> get_impl(std::vector<T>*) const
                    ^~~~~~~~
/usr/include/json.hpp:2680:20: note:   template argument deduction/substitution failed:
/usr/include/json.hpp:2946:24: note:   ‘jaegertracing::thrift::BaggageRestriction’ is not derived from ‘std::vector<T, std::allocator<_CharT> >’
         return get_impl(static_cast<ValueType*>(nullptr));
                ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/json.hpp:2705:7: note: candidate: template<class T, typename std::enable_if<(std::is_same<nlohmann::basic_json<>, typename T::value_type>::value && (! nlohmann::{anonymous}::has_mapped_type<T>::value)), int>::type <anonymous> > T nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(T*) const [with T = T; typename std::enable_if<(std::is_same<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>, typename T::value_type>::value && (! nlohmann::{anonymous}::has_mapped_type<T>::value)), int>::type <anonymous> = <enumerator>; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]
     T get_impl(T*) const
       ^~~~~~~~
/usr/include/json.hpp:2705:7: note:   template argument deduction/substitution failed:
/usr/include/json.hpp:2702:75: error: no type named ‘value_type’ in ‘class jaegertracing::thrift::BaggageRestriction’
                   std::is_same<basic_json, typename T::value_type>::value and
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
                   not has_mapped_type<T>::value
                   ~~~~~~~~~~~~~~~~~~~~~~                                   
/usr/include/json.hpp:2704:34: note: invalid template non-type parameter
                   , int>::type = 0>
                                  ^
/usr/include/json.hpp:2718:13: note: candidate: nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::array_t nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::array_t*) const [with ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::array_t = std::vector<nlohmann::basic_json<>, std::allocator<nlohmann::basic_json<> > >]
     array_t get_impl(array_t*) const
             ^~~~~~~~
/usr/include/json.hpp:2718:13: note:   no known conversion for argument 1 from ‘jaegertracing::thrift::BaggageRestriction*’ to ‘nlohmann::basic_json<>::array_t* {aka std::vector<nlohmann::basic_json<>, std::allocator<nlohmann::basic_json<> > >*}’
/usr/include/json.hpp:2735:7: note: candidate: template<class T, typename std::enable_if<std::is_convertible<std::__cxx11::basic_string<char>, T>::value, int>::type <anonymous> > T nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(T*) const [with T = T; typename std::enable_if<std::is_convertible<StringType, T>::value, int>::type <anonymous> = <enumerator>; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]
     T get_impl(T*) const
       ^~~~~~~~
/usr/include/json.hpp:2735:7: note:   template argument deduction/substitution failed:
/usr/include/json.hpp:2734:34: error: no type named ‘type’ in ‘struct std::enable_if<false, int>’
                   , int>::type = 0>
                                  ^
/usr/include/json.hpp:2734:34: note: invalid template non-type parameter
/usr/include/json.hpp:2752:7: note: candidate: template<class T, typename std::enable_if<std::is_arithmetic<_Tp>::value, int>::type <anonymous> > T nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(T*) const [with T = T; typename std::enable_if<std::is_arithmetic<T>::value, int>::type <anonymous> = <enumerator>; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator]
     T get_impl(T*) const
       ^~~~~~~~
/usr/include/json.hpp:2752:7: note:   template argument deduction/substitution failed:
/usr/include/json.hpp:2751:33: error: no type named ‘type’ in ‘struct std::enable_if<false, int>’
                  , int>::type = 0>
                                 ^
/usr/include/json.hpp:2751:33: note: invalid template non-type parameter
/usr/include/json.hpp:2779:25: note: candidate: constexpr nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::boolean_t nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::get_impl(nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::boolean_t*) const [with ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType>::boolean_t = bool]
     constexpr boolean_t get_impl(boolean_t*) const
                         ^~~~~~~~
/usr/include/json.hpp:2779:25: note:   no known conversion for argument 1 from ‘jaegertracing::thrift::BaggageRestriction*’ to ‘nlohmann::basic_json<>::boolean_t* {aka bool*}’
CMakeFiles/jaegertracing.dir/build.make:350: recipe for target 'CMakeFiles/jaegertracing.dir/src/jaegertracing/baggage/RemoteRestrictionJSON.cpp.o' failed
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Jan 28, 2018
jaeger-cpp defaults to using Hunter to download its dependencies off the 'net,
even if they exist locally. This is convenient for development but not
practical for some production environments. It also makes life hard for clients
that want to link to jaeger-cpp since Hunter doesn't install those
dependencies. It's necessary to also use Hunter in apps that use a jaeger-cpp
built this way... and that's not always practical.

Accordingly, add support for finding jaeger-cpp's dependencies via the normal
CMake package discovery mechanism.

A sligtly unsightly hack is required for nlohmann json, because its header
moved from json.hpp to nlohmann/json.hpp in 2.1.0.

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

WIP:

  - nothing packages nlohmann json 2.1.0 yet, 2.0.2 is widespread.
    but jaegertracing's code doesn't appear to support 2.0.2. For now you
    should work around it by installing 2.1.x locally from source.
    (jaegertracing#47)

  - will fail to compile tests if the local thrift is 0.9.1 since there are
    committed generated files from 0.9.2 (jaegertracing#45) and there's no mechanism
    to regenerate them.

This seeks to address the beginnings of jaegertracing#38.

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
@isaachier
Copy link
Contributor

Interesting. I'm strongly considering another approach for you instead of using system-wide packages. The few I can think of are: wget/curl from archives and submodules. Any reason not to do that instead?

@ringerc
Copy link
Contributor Author

ringerc commented Jan 29, 2018 via email

@isaachier
Copy link
Contributor

To be honest, the only reason I developed this library the current way is to allow for rapid development. Using apt-get makes it very difficult to port to Mac, etc. Hunter produces the same build on all supported platforms. So I'm definitely interested in enlarging the scope of Jaeger C++ to allow for deb/rpm. However, I would like to keep Hunter as default because it makes building so much easier for the casual developer.

Meanwhile, the idea of Jaeger C++ being a library/platform level library does bother me. Especially, considering the use of C++11 in C executables, which no one loves. If we really want a lowest common denominator utility, we should push forward the idea of opentracing-c and follow up with jaeger-c. That library would be painful to develop, but probably much easier to link in with the classic C/C++ development environment, and have much less bloat.

My final point, think of Jaeger C++ as a project with a lot of room for improvement. The basic proof of concept is here and it works in production, but compatibility could be extended to other platforms and build systems. I am totally open to hearing about features you'd like, and better yet, seeing pull requests with initial implementations.

@ringerc
Copy link
Contributor Author

ringerc commented Jan 29, 2018

Certainly no complaint about Hunter as default. I can see how it's handy for development, for container based work, etc. Though it'd be much, much more useful if you could do something like

 cmake -DCMAKE_INSTALL_PREFIX=/opt/jaeger-cpp -DHUNTER_ENABLED=1 -DHUNTER_INSTALL_DEPENDENCIES=1
 make
 make install

and get everything Hunter built as well as jaeger-cpp its self. So you can use it from other apps that may in turn not be able to use Hunter. Or where the user doesn't have the time/knowledge to convert the app's build system to use Hunter and just wants to use jaeger-cpp. Big barrier to entry there. (That's what I was getting at with #44)

I don't have a huge problem with thunking C++11 into C. It's awkward and annoying, but less so than writing a whole new library and API. Sure, if it massively takes off, that might make sense, but for now just making the C++ lib play a bit better with local installs (and documenting it) would be enough IMO. That's what I've been trying to do here, but am running out of time. I can keep working on it if you're OK with committing to maintaining a degree of lib version compatibility going forward, but otherwise need to drop it here.

Definitely no time to write a pure C client.

Working on it re doing the work and actually sending pulls. I'm trying to help not just complain. Lots to learn and figure out as I go given I've never met this Hunter tool before - a sort of Maven for C. And that jaeger-cpp hasn't had to care about dependency versions or compatibility so there are lots of unknowns there.

@isaachier
Copy link
Contributor

I am willing to agree to that. Good work so far!

@isaachier
Copy link
Contributor

@ringerc you've inspired me to begin a minimal client in C. This is a side project so can't guarantee anything about if/when it will be completed. Nonetheless, this might be more along the lines of what you wanted: jaeger-c-client.

@ringerc
Copy link
Contributor Author

ringerc commented Jan 31, 2018

@isaachier Cool. I wasn't asking you to, and I've been trying to do what I can to help make the C++ one usable from C. IMO it's OK enough, the problems are related to builds and library management more than language. Thunking from C++ to C is ugly but manageable.

@ringerc
Copy link
Contributor Author

ringerc commented Jan 31, 2018

The bigger problem has been Hunter, specific Thrift and json library dependencies not widely packaged, etc.

Sure, some folks will whine about C++, and I won't pretend it's a joy to work with in a C project, but that's not where the main challenges have been.

@isaachier
Copy link
Contributor

I find C makes tends to be better as a base layer especially when considering language bindings. C99 compiles pretty much anywhere unlike C++11. The C library is more of a challenge for me to implement a bare bones client with as few dependencies as possible.

@ringerc
Copy link
Contributor Author

ringerc commented Jan 31, 2018

@isaachier So long as you're happy :)

I imagine it'd do good things for adoption if it's easy to plug in, mind.

Here are some things I'd be considering, if I were the one doing the work. But since I'm not putting my hand up for that part, feel free to ignore everything I say! I have my hands full trying to get the C++ lib usable for my needs and getting some mergable pulls together for you.

Anyway, considerations in a C infrastructure lib:

  • exposing jaeger_malloc() and jaeger_free(...) wrappers for malloc() and free(). Memory allocated in the client to be freed by jaeger should be allocated with jaeger_malloc(); memory allocated by jaeger to be freed by the client should be freed with jaeger_free(). This ensures you can ensure you use the same C library to free memory returned by jaeger c-client as was used to allocate it. Important for Windows. Standard pattern for C libraries.

  • Allocator wrappers should be function pointers so applications can replace the allocator used by jaeger-cpp.

  • Minimising allocations by letting the client pass pointers to pre-allocated, possibly on-stack, memory where possible. Lets client apps manage memory lifetimes if they have their own allocators (e.g. PostgreSQL palloc/MemoryContextAlloc, Samba's talloc, etc. And lets client apps integrate their own memory statistics.

  • Low cost of trace before the "should I send this" decision is made so hot paths can be traced. Preferably avoiding heap allocations before sampler/filter decisions are made.

  • Shameless use of static inline functions and macros to minimise overheads. (This is one area C++ rocks, I miss inline template functions so much when I write C).

  • Doesn't call abort(). Ever.

@isaachier
Copy link
Contributor

If you check out the repo, the alloc.h header allows you to replace the Jaeger allocator. Before joining Uber, used to work at Bloomberg where they rewrote the STL with custom allocators, so I understand the importance of that. Otherwise, all sound like good suggestions I will use going forward.

@ringerc
Copy link
Contributor Author

ringerc commented Jan 31, 2018

Thanks.

Closing this issue. I've had a look at the changelog between 2.0.2 and 2.1.0 in nlohmann json and it's huge. I have no chance of figuring out what's needed to make jaeger cpp-client work with 2.0.2. There are changes to assignment operators and conversion support, tons of things.

I suggest documenting a current required version and sticking to it for a while; the world will catch up. And it's a header-only library so the usual issues with packaging aren't such a concern.

@ringerc ringerc closed this as completed Jan 31, 2018
@ringerc
Copy link
Contributor Author

ringerc commented Jan 31, 2018

BTW, rewrote the STL o_O ? Seems an odd choice given std::allocator and the way the templates all take allocator arguments. Presumably issues with the pre-c++11 requirement that custom allocators must be stateless.

But gee. I thought PostgreSQL liked wheel reinvention. That's a new level.

@isaachier
Copy link
Contributor

Agreed about the plan for libs and will document eventually :/.

BSL is a big debate there. Now that you mention it, who do you think was behind that C++11 change to the standard? Bloomberg engineers wrote the proposal: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2008/n2554.pdf. Anyway, I didn't use any scoped allocators in the Jaeger code so no issue here.

ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Jan 31, 2018
Add support for finding jaeger-cpp's dependencies via the normal local
CMake package discovery mechanism of Find modules.

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Jan 31, 2018
Add support for finding jaeger-cpp's dependencies via the normal local
CMake package discovery mechanism of Find modules.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Jan 31, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Jan 31, 2018
Add support for finding jaeger-cpp's dependencies via the normal local
CMake package discovery mechanism of Find modules.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Jan 31, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 1, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 2, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 2, 2018
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 13, 2018
Restructure the build system to follow CMake conventions: use import
libraries consistently, separate CMakeLists.txt for targets, etc.

Add Find packages to discover local dependencies not already supported
by CMake natively. These are used when Hunter is disabled to allow
jaeger cpp-client to use locally installed libraries.

This is a preliminary step in splitting the test sources out of the
library.

Improves jaegertracing#38 Offline builds

This introduces support for building with

   cmake -DHUNTER_ENABLED=0

Limitations for local dependency builds:

  - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY,
    not newer or older. These versions are not widely packaged
    so a local install is necessary.  (jaegertracing#45)

  - Requires nlohmann json 2.1.0 or newer, which is not widely
    packaged in Linux distros. Install a local copy. (jaegertracing#47)

Signed-off-by: Craig Ringer <craig@2ndquadrant.com>
ringerc added a commit to ringerc/jaeger-cpp-client that referenced this issue Feb 13, 2018
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

2 participants