Comments (19)
@isaachier Aah, that's git
's lovely default of silently ignoring submodules biting me. Thanks for the pointer. I'll look. Guess I'll need to glue that into the CMake build.
from jaeger-client-cpp.
OK so the Thrift schema files are shared among many language bindings. Hence, they are actually in a submodule idl
. I'm assuming git grep
is skipping submodules. You can regenerate using the Makefile in that directory. I agree I am not a fan of the use of version 0.9.2, but this has more to do with Uber's internal use than anything else.
from jaeger-client-cpp.
For other readers that means
git submodule init
git submodule update
then you'll find that idl/
is populated with origin https://github.com/uber/jaeger-idl.git
.
Surprising it's in /uber/
, but whatever.
Will look at plugging it into the cmake build later, out of time now.
from jaeger-client-cpp.
I don't think you need to plug in to build at all (I didn't for a reason). Just regenerate a move to your thrift-gen directory.
from jaeger-client-cpp.
from jaeger-client-cpp.
Regarding the Thrift sources, the idl directory should never have to be used. The fact that we use Thrift 0.9.2 does force users of more recent versions to manually generate the idl files. If they had the "right" version, they wouldn't need to know about it.
The target audience is a good question and followed up there
from jaeger-client-cpp.
I just tried to build with the latest Thrift in Rawhide
$ thrift --version
Thrift version 0.10.0
and it fails too:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/Agent.cpp: In member function ‘uint32_t jaegertracing::agent::thrift::Agent_emitZipkinBatch_args::write(apache::thrift::protocol::TProtocol*) const’:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/Agent.cpp:70:10: error: ‘class apache::thrift::protocol::TProtocol’ has no member named ‘incrementRecursionDepth’; did you mean ‘incrementInputRecursionDepth’?
oprot->incrementRecursionDepth();
^~~~~~~~~~~~~~~~~~~~~~~
since it looks like the Thrift folks renamed the member.
So it appears that jaeger cpp-client probably currently works with thrift [0.9.2,0.9.3]
only.
I strongly argue that committing generated files from IDL is bad practice, especially where the result of code-generation from the IDL doesn't follow a strong specification and isn't guaranteed to be compatible across versions. It doesn't look to me like Thrift generated code is meant to be used with a different library version.
It'd be better to generate the code from the IDL, per something like https://github.com/snikulov/cmake-modules/blob/master/FindThrift.cmake / https://github.com/apache/parquet-cpp/blob/master/cmake_modules/FindThrift.cmake where a THRIFT_COMPILER
is exposed.
from jaeger-client-cpp.
Ok, Thrift installed with
git clone //git-wip-us.apache.org/repos/asf/thrift.git
cd thrift
git checkout 0.9.2
./bootstrap.sh
./configure --with-cpp --with-java=no --with-python=no --with-lua=no --with-perl=no --enable-shared=yes --enable-static=no --enable-tutorial=no --with-qt4=no
make -s
from jaeger-client-cpp.
@isaachier I did a manual rebuild of the IDL with the existing Thrift 0.9.2, to see if I could reproduce the current sources. I'm then going to go on to make sure it works with any suitable Thrift.
But the generated sources do not match what's in the Jaeger cpp-client
tree. It looks like the tracetest_types
and zipkincore_constants
sources have produced different sources. I have no way to tell if these are local changes made manually to the generated code or if they're a generation issue since they're in the first commit.
The diff can be seen in ringerc@819eff4 ; I pushed a branch with it for convenient viewing.
Were these changes made locally by hand?
I'm using the same Thrift version as the IDL scripts use, and the Makefile in idl/ doesn't pass extra arguments. Looking at the diff I'm inclined to think "local changes" ?
from jaeger-client-cpp.
Causes build failures:
$ make
[ 2%] Built target testutils
[ 2%] Building CXX object CMakeFiles/jaegertracing.dir/src/jaegertracing/thrift-gen/TracedService.cpp.o
In file included from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.h:11:0,
from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.cpp:7:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:64:14: error: field ‘downstream’ has incomplete type ‘jaegertracing::crossdock::thrift::Downstream’
Downstream downstream;
^~~~~~~~~~
In file included from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.h:11:0,
from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.cpp:7:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:47:7: note: definition of ‘class jaegertracing::crossdock::thrift::Downstream’ is not complete until the closing brace
class Downstream {
^~~~~~~~~~
In file included from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.h:11:0,
from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.cpp:7:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:280:17: error: field ‘downstream’ has incomplete type ‘jaegertracing::crossdock::thrift::TraceResponse’
TraceResponse downstream;
^~~~~~~~~~
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:267:7: note: definition of ‘class jaegertracing::crossdock::thrift::TraceResponse’ is not complete until the closing brace
class TraceResponse {
^~~~~~~~~~~~~
CMakeFiles/jaegertracing.dir/build.make:1694: recipe for target 'CMakeFiles/jaegertracing.dir/src/jaegertracing/thrift-gen/TracedService.cpp.o' failed
which looks to be down to
@@ -56,7 +61,9 @@ class Downstream {
std::string host;
std::string port;
Transport::type transport;
- boost::shared_ptr<Downstream> downstream;
+ Downstream downstream;
If I try building with Thrift 0.9.1, thrift generation fails with
[ERROR:/home/craig/projects/2Q/opentracing-jaeger-cpp-client/idl/thrift/crossdock/tracetest.thrift:28] (last token was 'Downstream')
Type "Downstream" has not been defined.
so it looks like Thrift is having issues with a self-reference of some sort.
If the IDL cannot be regenerated it'll be hard to keep up with changes to the protocol.
BTW, shouldn't the IDL be part of the opentracing project? Or am I missing how all this works?
from jaeger-client-cpp.
Same issue if I use Thrift 0.10.0
In file included from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.h:12:0,
from /home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/TracedService.cpp:7:
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:62:14: error: field ‘downstream’ has incomplete type ‘jaegertracing::crossdock::thrift::Downstream’
Downstream downstream;
^~~~~~~~~~
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:48:7: note: definition of ‘class jaegertracing::crossdock::thrift::Downstream’ is not complete until the closing brace
class Downstream : public virtual ::apache::thrift::TBase {
^~~~~~~~~~
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:290:17: error: field ‘downstream’ has incomplete type ‘jaegertracing::crossdock::thrift::TraceResponse’
TraceResponse downstream;
^~~~~~~~~~
/home/craig/projects/2Q/opentracing-jaeger-cpp-client/src/jaegertracing/thrift-gen/tracetest_types.h:280:7: note: definition of ‘class jaegertracing::crossdock::thrift::TraceResponse’ is not complete until the closing brace
class TraceResponse : public virtual ::apache::thrift::TBase {
^~~~~~~~~~~~~
CMakeFiles/jaegertracing.dir/build.make:1694: recipe for target 'CMakeFiles/jaegertracing.dir/src/jaegertracing/thrift-gen/TracedService.cpp.o' failed
from jaeger-client-cpp.
Zipkin isn't used in C++ client. The tracetest changes are indeed a real issue and one reason I cannot foresee making the Thrift generation part of the standard build. See this issue and ensuing discussion: jaegertracing/jaeger-idl#35. I had to painstakingly fix the generated code by hand to avoid a serious issue in the schema definition.
from jaeger-client-cpp.
@isaachier Thanks for the reference, I never would've found that.
Seems like a horrible decision in Thrift about how to translate the IDL to C++. __isset
? Ew.
std::optional
in C++17 would help but .. well, c++17. Pity they didn't just import one of the older implementations of that concept as a thrift::optional
instead of their separate member __isset
hack.
The tests they added in apache/thrift#84 cover various self-referential forms other than optional self-inclusion.
Looking for a Thrift bug.
Seems like it'd be hard to fix this without changing the opentracing IDL. But maybe that's what should happen.
from jaeger-client-cpp.
Didn't find a Thrift bug so created https://issues.apache.org/jira/browse/THRIFT-4484
from jaeger-client-cpp.
"one of" the reasons? Other known issues?
from jaeger-client-cpp.
@isaachier I think I found a fix for the IDL issue; ringerc/jaeger-idl#1 .
from jaeger-client-cpp.
I'm abandoning making it work on different Thrift versions for now. Thrift 0.11.0 and newwer build with std::shared_ptr
instead of boost::shared_ptr
unless you define -DFORCE_BOOST_SMART_PTR
. So we'd have to detect if it's a newer Thrift and (if even possible) how it was built, and switch to std::shared_ptr
using a namespace hack like Thrift does in its lib/cpp/src/thrift/stdcxx.h
.
This goes in the too-hard basket at this point, given that the IDL is also busted per jaegertracing/jaeger-idl#35 and even Thrift 0.11.0 generates bad code for the fix using by-reference inclusion. I'll push a branch with my WIP, but unless someone has more time/care than me, it looks like the dependency on 0.9.2-0.9.3 stays.
from jaeger-client-cpp.
Resolved in latest release with #94.
from jaeger-client-cpp.
This still requires a patch file to be applied by the build on top of the generated Thrift IDL right? A patch that's probably specific to the Thrift version used?
Not a complaint in any way, just for clarity in terms of how it's resolved.
from jaeger-client-cpp.
Related Issues (20)
- Provide access to context HTTP Header name
- Jaeger 32 bit C/C++ client required
- QUESTION: Can not use with hunter manager
- how to release version to real scene
- Unit test: Socket.testFailBind fails when run as "root" HOT 6
- Can we please ditch hunter HOT 1
- Enable ppc64le arch for jaeger-client-cpp
- Unable to build windows in a pipeline.
- Add support for adding tracer tags via config yaml HOT 1
- My jaeger version is 1.23, and the jaeger-collector component has enabled --sampling.strategies-reload-interval, but the collected logs show that the sampling strategy is still old HOT 4
- Envoy - failed to load dynamic library opentracing versions are incomptable
- `RemoteReporter::report()` is blocked
- Default queue size & buffer flush interval are not used when reading configuration from YAML
- Issue facing while trying to build jaeger-client-cpp
- Compilation issue on Ubuntu 20.04 HOT 2
- When the sampling type is rate, the number of actual results is equal to the number of threads multiplied by the configured rate parameter
- warning const' was hidden [-Woverloaded-virtual] when using jaeger client with opentracing libraries
- Enablement TravisCI integration for IBM ppc64le architecture HOT 1
- Deprecate Jaeger C++ SDK HOT 3
- [Bug]: W3C Propagation is wrongfully encoding debug span flag into traceparent HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from jaeger-client-cpp.