bump to gRPC v1.48.1 for bazel CIs #1786
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1786 +/- ##
==========================================
+ Coverage 85.73% 85.77% +0.04%
==========================================
Files 171 171
Lines 5240 5240
==========================================
+ Hits 4492 4494 +2
+ Misses 748 746 -2
|
bazel/repository.bzl
Outdated
| strip_prefix = "abseil-cpp-20211102.0", | ||
| urls = [ | ||
| "https://storage.googleapis.com/grpc-bazel-mirror/github.com/abseil/abseil-cpp/archive/20211102.0.tar.gz", | ||
| "https://github.com/abseil/abseil-cpp/archive/20211102.0.tar.gz", |
There was a problem hiding this comment.
repository.bzl is already using:
strip_prefix = "abseil-cpp-20220623.1",
Per the abseil release notes:
https://github.com/abseil/abseil-cpp/releases/tag/20220623.1
this is the latest abseil version using C++11.
Why do we need to use an older version ?
There was a problem hiding this comment.
The latest gRPC can not work with a old abseil-cpp , I think we can use a old gRPC and abseil-cpp with legacy toolchain and the latest version with modern toolchain.
There was a problem hiding this comment.
So, grpc-1.46.4 is the last gRPC release supporting C++11, which is dependent on abseil-cpp-20211102.0.
There was a problem hiding this comment.
gRPC 1.46.4 is the last with C++11 support which has dependency on abseil-cpp-20211102.0. But it seems it works with the latest abseil as well:
https://github.com/open-telemetry/opentelemetry-cpp/actions/runs/3516378955/jobs/5892867609
I'm going to remove it.
| @@ -59,7 +59,8 @@ mkdir -p "${BUILD_DIR}" | |||
| [ -z "${PLUGIN_DIR}" ] && export PLUGIN_DIR=$HOME/plugin | |||
| mkdir -p "${PLUGIN_DIR}" | |||
|
|
|||
| BAZEL_OPTIONS="--copt=-DENABLE_LOGS_PREVIEW --copt=-DENABLE_TEST --copt=-DENABLE_METRICS_EXEMPLAR_PREVIEW" | |||
| BAZEL_OPTIONS_DEFAULT="--copt=-DENABLE_LOGS_PREVIEW --copt=-DENABLE_TEST --copt=-DENABLE_METRICS_EXEMPLAR_PREVIEW" | |||
| BAZEL_OPTIONS="--cxxopt=-std=c++14 $BAZEL_OPTIONS_DEFAULT" | |||
There was a problem hiding this comment.
If I understand the PR description correctly, we will add the CI test for gRPC-1.46.4 ( i.e building with C++11) in separate PR?
There was a problem hiding this comment.
yes #1787, gRPC with C++11 needed to be defined in the main branch first.
| @@ -466,6 +467,7 @@ TEST_F(OtlpHttpExporterTestPeer, ExportJsonIntegrationTestSync) | |||
| TEST_F(OtlpHttpExporterTestPeer, ExportJsonIntegrationTestAsync) | |||
| { | |||
| ExportJsonIntegrationTestAsync(); | |||
| google::protobuf::ShutdownProtobufLibrary(); | |||
There was a problem hiding this comment.
Is protobuf library loaded explicitly?
There was a problem hiding this comment.
This call is needed to delete any global objects that were allocated by the Protocol Buffer: https://developers.google.com/protocol-buffers/docs/cpptutorial
There was a problem hiding this comment.
Thanks for sharing the doc. It looks optional in the doc. Also wondering whether live global objects should be considered as leak.
There was a problem hiding this comment.
yes it is optional, here we need it to prevent the leak report though.
Live global objects should not be considered as leak.
#1634 (issue) step 1/2
Changes
This PR bumps gRPC to v1.48.1 for bazel CIs. It also adds the latest gRPC with C++11 support, so that in a separate PR we can hava e2e C++11 test:
opentelemetry-cpp/examples/e2e/WORKSPACE
Line 15 in 21ced21
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes