Skip to content
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

Jaeger Exporter Compilation error #1680

Closed
mcarajatchawla opened this issue Oct 13, 2022 · 7 comments · Fixed by #1714
Closed

Jaeger Exporter Compilation error #1680

mcarajatchawla opened this issue Oct 13, 2022 · 7 comments · Fixed by #1714
Assignees
Labels
bug Something isn't working

Comments

@mcarajatchawla
Copy link

mcarajatchawla commented Oct 13, 2022

Description

getting below issue with version 1.6.1 compilation with flag -DWITH_JAEGER=ON

69.18 -- The C compiler identification is GNU 9.4.0
69.24 -- The CXX compiler identification is GNU 9.4.0

123.6 Scanning dependencies of target opentelemetry_exporter_jaeger_trace
123.6 [ 60%] Building CXX object exporters/jaeger/CMakeFiles/opentelemetry_exporter_jaeger_trace.dir/src/jaeger_exporter.cc.o
124.6 In file included from /usr/src/opentelemetry-cpp/exporters/jaeger/src/http_transport.h:3,
124.6 from /usr/src/opentelemetry-cpp/exporters/jaeger/src/jaeger_exporter.cc:9:
124.6 /usr/src/opentelemetry-cpp/exporters/jaeger/src/THttpTransport.h:23:8: error: 'bool opentelemetry::v1::exporter::jaeger::THttpTransport::isOpen() const' marked 'override', but does not override
124.6 23 | bool isOpen() const override;
124.6 | ^~~~~~
124.7 In file included from /usr/src/opentelemetry-cpp/exporters/jaeger/src/udp_transport.h:6,
124.7 from /usr/src/opentelemetry-cpp/exporters/jaeger/src/jaeger_exporter.cc:11:
124.7 /usr/src/opentelemetry-cpp/exporters/jaeger/src/TUDPTransport.h:31:8: error: 'bool opentelemetry::v1::exporter::jaeger::TUDPTransport::isOpen() const' marked 'override', but does not override
124.7 31 | bool isOpen() const override;
124.7 | ^~~~~~
125.7 make[2]: *** [exporters/jaeger/CMakeFiles/opentelemetry_exporter_jaeger_trace.dir/build.make:63: exporters/jaeger/CMakeFiles/opentelemetry_exporter_jaeger_trace.dir/src/jaeger_exporter.cc.o] Error 1
125.7 make[1]: *** [CMakeFiles/Makefile2:913: exporters/jaeger/CMakeFiles/opentelemetry_exporter_jaeger_trace.dir/all] Error 2
125.7 make: *** [Makefile:130: all] Error 2

Steps to reproduce

FROM ubuntu:20.04
"install dependencies"
RUN git clone --recursive https://github.com/open-telemetry/opentelemetry-cpp /usr/src/opentelemetry-cpp
&& mkdir -p /usr/src/opentelemetry-cpp/build && cd /usr/src/opentelemetry-cpp
&& git checkout v1.6.1
&& cd build
&& cmake .. -DBUILD_TESTING=OFF -DWITH_JAEGER=ON
&& cmake --build . --target all
&& cmake --install .
&& rm -rf /usr/src/opentelemetry-cpp

What is the expected behavior?
compilation should be success

What is the actual behavior?
compilation error

Additional context
Add any other context about the problem here.

@mcarajatchawla mcarajatchawla added the bug Something isn't working label Oct 13, 2022
@marcalff
Copy link
Member

Thanks for the report.

Please indicate which THRIFT_VERSION is used during the build.

See file
/usr/include/thrift/version.h
or similar, and look for:

#define THRIFT_VERSION "0.10.0"

@mcarajatchawla
Copy link
Author

THRIFT_VERSION 0.12.0

@ThomsonTan
Copy link
Contributor

I just created run the above command in a fresh ubuntu:20.04 container, with thrift 0.13.0 installed, and done the build successfully.

@mcarajatchawla probably try upgrade thrift to 0.13.0?

@marcalff
Copy link
Member

Note that a change request is currently in discussion in the opentelemetry specification to deprecate the jeager exporter.

open-telemetry/opentelemetry-specification#2858

Instead, the recommended path is to use the OTLP exporter to talk to jaeger, which now understand OTLP natively.

@marcalff
Copy link
Member

If using the jeager exporter, the thrift version used to test opentelemetry-cpp internally is 0.14.1

@lalitb
Copy link
Member

lalitb commented Oct 25, 2022

We can document the version specific information for thrift in dependencies.md.

@ThomsonTan
Copy link
Contributor

Here is the commit in thrift which fixed the issue, and it was available since 0.13.0.

apache/thrift@f83d3f9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants