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.

Sign up for GitHub

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

[BUILD] Add missing target dependencies #2128

Merged
merged 3 commits into from
May 3, 2023
Merged

[BUILD] Add missing target dependencies #2128

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
009406d
fix target link libraries
lalitb May 3, 2023
0002eb2
markdown lint failure
lalitb May 3, 2023
2ab56e7
Merge branch 'main' into add-target-libs
ThomsonTan May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ update the semantic convention in instrumentation library is needed.
* [BUILD] Don't require applications using jaeger exporter to know about libcurl
[#1518](https://github.com/open-telemetry/opentelemetry-cpp/pull/1518)
* [EXPORTER] Inline print_value() in ostream exporter [#1512](https://github.com/open-telemetry/opentelemetry-cpp/pull/1512)
* [SDK] fix: urlPaser will incorrect parsing url like "http://abc.com/xxx@xxx/a/b"
* [SDK] fix: urlPaser will incorrect parsing url like `http://abc.com/xxx@xxx/a/b`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI is failing for bare URL here, so fixing this.

[#1511](https://github.com/open-telemetry/opentelemetry-cpp/pull/1511)
* [SDK] Rename `InstrumentationLibrary` to `InstrumentationScope` [#1507](https://github.com/open-telemetry/opentelemetry-cpp/pull/1507)
* [BUILD] Try to build nlohmann-json only it's depended. [#1505](https://github.com/open-telemetry/opentelemetry-cpp/pull/1505)
Expand Down
3 changes: 2 additions & 1 deletion exporters/otlp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ if(WITH_OTLP_GRPC)

target_link_libraries(
opentelemetry_exporter_otlp_grpc_client
PUBLIC opentelemetry_sdk opentelemetry_ext opentelemetry_proto)
PUBLIC opentelemetry_sdk opentelemetry_common opentelemetry_ext
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This problem is already fixed in #2097 .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct me otherwise, but I don't see this is fixed with #2097. Also validated by building shared libs from that PR, and below is the result:

$ ldd -r -d libopentelemetry_*so  2>&1 | grep undefin
undefined symbol: _ZNK13opentelemetry2v13sdk8resource8Resource13GetAttributesEv (./libopentelemetry_exporter_ostream_metrics.so)
undefined symbol: _ZN13opentelemetry2v13sdk6common12internal_log16GlobalLogHandler18GetHandlerAndLevelEv        (./libopentelemetry_exporter_otlp_grpc_client.so)
undefined symbol: _ZN13opentelemetry2v13sdk6common12internal_log16GlobalLogHandler18GetHandlerAndLevelEv        (./libopentelemetry_http_client_curl.so)
undefined symbol: _ZN13opentelemetry2v13sdk6common12internal_log16GlobalLogHandler18GetHandlerAndLevelEv        (./libopentelemetry_exporter_otlp_http_client.so)
undefined symbol: _ZN13opentelemetry2v13sdk6common12internal_log16GlobalLogHandler18GetHandlerAndLevelEv        (./libopentelemetry_http_client_curl.so)
undefined symbol: _ZN4grpc24g_core_codegen_interfaceE   (./libopentelemetry_proto_grpc.so)
undefined symbol: _ZN4grpc6g_glipE      (./libopentelemetry_proto_grpc.so)
undefined symbol: _ZN4absl12lts_202103245MutexD1Ev      (./libopentelemetry_proto_grpc.so)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I mean the proto dependency of opentelemetry_exporter_otlp_grpc_client is added in #2097 . I didn't see opentelemetry_common before and it's still useful.
I can solve the conflict later.

opentelemetry_proto)

target_link_libraries(opentelemetry_exporter_otlp_grpc_client
PRIVATE gRPC::grpc++)
Expand Down
3 changes: 2 additions & 1 deletion ext/src/http/client/curl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ add_library(
set_target_properties(opentelemetry_http_client_curl
PROPERTIES EXPORT_NAME http_client_curl)
set_target_version(opentelemetry_http_client_curl)

target_link_libraries(opentelemetry_http_client_curl
PUBLIC opentelemetry_common)
if(TARGET CURL::libcurl)
target_link_libraries(
opentelemetry_http_client_curl
Expand Down
3 changes: 2 additions & 1 deletion sdk/src/metrics/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ add_library(
set_target_properties(opentelemetry_metrics PROPERTIES EXPORT_NAME metrics)
set_target_version(opentelemetry_metrics)

target_link_libraries(opentelemetry_metrics PUBLIC opentelemetry_common)
target_link_libraries(opentelemetry_metrics PUBLIC opentelemetry_common
opentelemetry_resources)

target_include_directories(
opentelemetry_metrics
Expand Down