opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

[BUILD] CURL versions discrepancies with bazel

Open ecourreges-orange opened this issue 1 year ago • 5 comments

Describe your environment Ubuntu 20.04 with its curl/libcurl v 7.68.0-1ubuntu2.21 bazel 5.4.1

Steps to reproduce Created an OtlpHttpExporter

What is the expected behavior? No problem connecting, unsupported options not being used

What is the actual behavior?

[Error] File: external/io_opentelemetry_cpp/ext/src/http/client/curl/http_operation_curl.cc:526 CURL, set option <20312> failed: <An unknown option was passed in to libcurl>
[Error] File: external/io_opentelemetry_cpp/exporters/otlp/src/otlp_http_client.cc:200 [OTLP HTTP Client] Session state: connection failed.An unknown option was passed in to libcurl
[Error] File: external/io_opentelemetry_cpp/exporters/otlp/src/otlp_http_client.cc:168 [OTLP HTTP Client] Session state: session create failed.
[Error] File: external/io_opentelemetry_cpp/exporters/otlp/src/otlp_http_exporter.cc:118 [OTLP HTTP Client] ERROR: Export 4 trace span(s) error: 1 

20312 is CURLOPT_PREREQFUNCTION you can see here that it is supported in curl 7.80.0 and above, although otlp exporter tries to use it for 7.5 and above which is exactly my case for producing the bug 7.5<7.68<7.80

So I think simply changing the test for the curl version will fix this bug, and I will post a PR for that in a couple of minutes.

https://github.com/open-telemetry/opentelemetry-cpp/blob/d32f9609b965dbb34030c1740ddb464d93cd3065/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h#L105

https://github.com/open-telemetry/opentelemetry-cpp/blob/d32f9609b965dbb34030c1740ddb464d93cd3065/ext/src/http/client/curl/http_operation_curl.cc#L155

https://github.com/open-telemetry/opentelemetry-cpp/blob/d32f9609b965dbb34030c1740ddb464d93cd3065/ext/src/http/client/curl/http_operation_curl.cc#L1047

The other option CURLOPT_PREREQDATA which is also concerned appeared in the same curl version 7.80.0, so all should be fine in one fix.

Not sure I can unit test that though...

ecourreges-orange avatar Feb 01 '24 15:02 ecourreges-orange

Bonjour @ecourreges-orange

Please double check which version of curl headers are used, and which version of the curl library is used to link, in your build: I suspect a discrepancy there.

See comments in the PR.

marcalff avatar Feb 01 '24 16:02 marcalff

Bonjour @marcalff
Thanks for your expertise, the problem is I guess only on the bazel build, because the curl version downloaded is fixed and is 8.4.0, and my system (Ubuntu 20.04 focal) has 7.68.0
https://github.com/open-telemetry/opentelemetry-cpp/blob/d32f9609b965dbb34030c1740ddb464d93cd3065/bazel/repository.bzl#L152
In the cmake build, the user brings his own curl version.
I am running an apache module which is dynamically loaded, I can't afford to have a different curl than the system, because other apache modules probably use libcurl of the system. An ldd of my app confirms that curl is loaded dynamically from the system, which causes the version discrepancy.

As far as the solutions, I suppose the bazel build could use the system curl if it founds it, instead of downloading it?

ecourreges-orange avatar Feb 05 '24 09:02 ecourreges-orange

Thanks for the details.

This issue needs to stay opened, to see how to address bazel.

marcalff avatar Feb 05 '24 10:02 marcalff

For now I have a working version in my "nobazelcurl" fork, replacing libcurl deps like this to use the system version of curl. Maybe just having a switch between this and the existing code would make it a decent enough solution for a PR.

https://github.com/ecourreges-orange/opentelemetry-cpp/blob/88a5f141628c5de7c52cd9eb31d8dfee66918c3a/bazel/repository.bzl#L155

    native.new_local_repository(
        name = "curl",
        path = "/usr",
        build_file = "@io_opentelemetry_cpp//bazel:systemcurl.BUILD"
    )

https://github.com/ecourreges-orange/opentelemetry-cpp/blob/nobazelcurl/bazel/systemcurl.BUILD

cc_import(
    name = "curl",
    shared_library = "lib/x86_64-linux-gnu/libcurl.so",
    static_library = "lib/x86_64-linux-gnu/libcurl.a",
    visibility = ["//visibility:public"],
)

ecourreges-orange avatar Feb 05 '24 15:02 ecourreges-orange

This issue was marked as stale due to lack of activity.

github-actions[bot] avatar Apr 08 '24 01:04 github-actions[bot]