Update gRPC

[owent]

Fixes #665

Changes

Update gRPC to 1.38.0 .

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed

[codecov]

Codecov Report

Merging #666 (7097e1a) into main (f793e6e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #666   +/-   ##
=======================================
  Coverage   96.19%   96.19%           
=======================================
  Files         153      153           
  Lines        6444     6444           
=======================================
  Hits         6198     6198           
  Misses        246      246           

[owent]

The problem in Bazel tsan config seems to be the problem of abseil implement of gpr_once_init, which is called by internal API grpc::internal::GrpcLibrary::init of gRPC.

Don't know why compiling with gcc 4.8 is still failed after I have added --cxxopt="-std=gnu++11" --conlyopt="-std=gnu99" into build command.Could anyone help?

[lalitb]

Don't know why compiling with gcc 4.8 is still failed after I have added --cxxopt="-std=gnu++11" --conlyopt="-std=gnu99" into build command.Could anyone help?

I will give it a try with my limited bazel understanding :)

[lalitb]

Don't know why compiling with gcc 4.8 is still failed after I have added --cxxopt="-std=gnu++11" --conlyopt="-std=gnu99" into build command.Could anyone help?

It looks like GCC 4.8 is not supported by the latest versions of gRPC :

https://grpc.io/docs/

Language | OS | Compilers / SDK
--       | -- | --
C/C++ | Linux, Mac | GCC 4.9+, Clang 3.4+

[ThomsonTan]

Don't know why compiling with gcc 4.8 is still failed after I have added --cxxopt="-std=gnu++11" --conlyopt="-std=gnu99" into build command.Could anyone help?

It looks like GCC 4.8 is not supported by the latest versions of gRPC :

grpc.io/docs

Language | OS | Compilers / SDK
--       | -- | --
C/C++ | Linux, Mac | GCC 4.9+, Clang 3.4+

Could we reference a different (supported) gRPC for GCC 4.8, instead of just not supporting GCC 4.8?

[owent]

I found https://abseil.io/docs/cpp/platforms/platforms shows abseil only support gcc 5.1+. Which is also different from gRPC.

[lalitb]

I found https://abseil.io/docs/cpp/platforms/platforms shows abseil only supports GCC 5.1+. Which is also different from gRPC.

Yes, I too noticed that. Not sure how gRPC uses abseil and whether it's only the part of abseil which compiles with GCC 4.9. We can discuss this in the community meeting (today) about how to proceed with the gRPC upgrade ( as this is needed for the build using Bazel 4.0.0 ).

[lalitb]

My proposal would be

• Have a dependency matrix ( in terms of compiler versions, platform, libraries versions ) for api/sdk and exporters. Say, otlp exporter would need at least gcc4.9 compiler, while api/sdk works with gcc4.8. Similarly, ETW exporter only works for windows. And document this dependency matrix.
• Have separate build action for api/sdk, and exporter based on this dependency matrix. In the case for otlp, we can have three different build actions for bazel (and cmake) build:
  • Legacy bazel build with gcc 4.8 compiling api/sdk and all exporters except otlp.
  • Legacy bazel build with gcc 4.9 compiling api/sdk with otlp exporter.
  • default Bazel build with latest and greatest compiler compiling api/sdk with all exporters (each for osx, linux, and windows)

cc @open-telemetry/cpp-approvers for comment here. Just to add, there was a previous try by @bogdandrutu to upgrade gRPC which failed for same reason ( #459 )

[maxgolov]

Looks good to me. I think you need to get a sign-off from Josh on Bazel changes.

@jsuereth

[lalitb]

Looks good to me. I think you need to get a sign-off from Josh on Bazel changes.

@jsuereth

@maxgolov @jsuereth - would need feedback on the approach to go ahead with this PR. The problem right now is that the latest versions of gRPC (grpc-1.37.0) require gcc4.9+ ( so legacy bazel build on gcc4.8 is failing). and the currently used version (grpc-1.28.0) doesn't compile using bazel 4.0.0. Please go over the discussions in this PR where the approach is discussed, and provide your suggestions? We can either bump up our gcc version in legacy cmake/bazel builds, and say that opentelemetry-cpp needs at least gcc4.9 to compile, or have separate dependencies for otlp-exporter

[owent]

The latest version of gRPC which can be compiled by gcc-4.8 is 1.33.2. It's also the last version of gRPC use 20200225.3.
But I'm not familiar with bazel, and I have tried to add --config=legacy and --cxxopt="-std=c++11" --conlyopt="-std=c11" but failed to find a way to replace the version of @com_google_protobuf in WORKSPACE only when using --config=legacy .

BTW: The official support of vcpkg require g++>=6 . Do we need a standalone cmake toolchain to support gcc-4.8?

[maxgolov]

I'm wondering if we can have no-vcpkg, CMake-only build. My understanding we do not necessarily require vcpkg. The full set of binaries can be built with just plain CMake, even for an older system such as ubuntu-14.04 with gcc-4.8.4 .

Or can we have an option to only build Bazel legacy with an older gRPC? I have hit a similar issue in another library before. Such as either Google Test or Google Benchmark, would only compile with cmake or classic gnu make, and not with more recent build system on older Ubuntu distro..

Also, in general , as we intake new versions of dependencies -- new versions of 3rd party deps are forcing us to upgrade to more recent compiler, and/or CMake tooling.

The latest version of gRPC which can be compiled by gcc-4.8 is 1.33.2

Can we capture - what old legacy versions worked well with an old legacy compiler, and maintain that as a separate build configuration - or do you think it is too much of a hassle to maintain? I had this build script (outside of standard ./ci/do_ci.sh script) that may be used for this purpose:

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/tools/build.sh

It was running well in a number of docker containers last year:

Older Ubuntu-14.04-LTS by default comes with gcc-4.8.4 , and it used to compile Okay with the above script.

We can probably add that as part of CI loop to verify the build with older set of deps. Such as gcc-4.8.4 + gRPC-1.33.2 ? And assume that we can still run the build on a system that old, with a compiler that old... My understanding is that several companies (LightStep, Envoy?) - actively asked us for gcc-4.8 support. And we originally committed to that support in 2020.

[maxgolov]

@jsuereth - I wonder why don't we just turn off CI / Bazel gcc 4.8 loop? Do you guys feel like it's needed? We can still have CMake + gcc-4.8 loop.

[lalitb]

@jsuereth - I wonder why don't we just turn off CI / Bazel gcc 4.8 loop? Do you guys feel like it's needed? We can still have CMake + gcc-4.8 loop.

@maxgolov - Currently, our CMake + gcc-4.8 is not building otlp exporter. Once we enable that and upgrade gRPC to the latest, it will start failing too.

As I suggested above, we should be splitting the CI into two parts ( for both cmake and bazel ): gcc-4.8 without otlp, and gcc-4.9 with otlp.

[ThomsonTan]

@jsuereth - I wonder why don't we just turn off CI / Bazel gcc 4.8 loop? Do you guys feel like it's needed? We can still have CMake + gcc-4.8 loop.

@maxgolov - Currently, our CMake + gcc-4.8 is not building otlp exporter. Once we enable that and upgrade gRPC to the latest, it will start failing too.

Instead of turning of bazel with GCC 4.8, could we turn off WITH_OTLP for bazel GCC 4.8 and make it on par with CMake? Then we could upgrade gRPC to the latest.

[lalitb]

• default Bazel build with latest and greatest compiler compiling api/sdk with all exporters (each for osx, linux, and windows)

Instead of turning of bazel with GCC 4.8, could we turn off WITH_OTLP for bazel GCC 4.8 and make it on par with CMake? Then we could upgrade gRPC to the latest.

Agree, that's what I am proposing too ( for both cmake/bazel): gcc-4.8 without otlp, gcc-4.9 with otlp, gcc-latest with otlp

[maxgolov]

gcc-4.8 without otlp

So are you folks saying that we cannot support OTLP with gcc-4.8 ? That's fine, as long as we spell it out. I think original expectation was that OTLP is the de-facto the main exporter, not having it with gcc-4.8 is probably the same as saying that we just don't support gcc-4.8 at all for any other exporter. And the min required is gcc-4.9 then.

[ThomsonTan]

gcc-4.8 without otlp

So are you folks saying that we cannot support OTLP with gcc-4.8 ? That's fine, as long as we spell it out. I think original expectation was that OTLP is the de-facto the main exporter, not having it with gcc-4.8 is probably the same as saying that we just don't support gcc-4.8 at all for any other exporter. And the min required is gcc-4.9 then.

I agree. We will not support gcc 4.8 + OTLP with the latest gRPC. This is decided by gRPC and its dependencies instead of us. As we discussed, the user could still specify a prior gRPC version which works with gcc 4.8 (we could document this).

[owent]

@jsuereth - I wonder why don't we just turn off CI / Bazel gcc 4.8 loop? Do you guys feel like it's needed? We can still have CMake + gcc-4.8 loop.

@maxgolov - Currently, our CMake + gcc-4.8 is not building otlp exporter. Once we enable that and upgrade gRPC to the latest, it will start failing too.

As I suggested above, we should be splitting the CI into two parts ( for both cmake and bazel ): gcc-4.8 without otlp, and gcc-4.9 with otlp.

I'm working to try to use cmake to build old grpc and all the dependency in my project. And I will try to intergrate opentelemetry-cpp
with it later.There are still a lot of system using centos 7/redhat 7 which use gcc 4.8 as their default compiler,it's better if we have official support here.Maybe I can move some build scripts of my project here to support gcc 4.8?

[maxgolov]

Maybe I can move some build scripts of my project here to support gcc 4.8?

Makes sense. Can you check out these scripts:

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/tools/build.sh
Sorry, this one is for Windows, but I can add the same script for *nix:
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/tools/build-docker.cmd
And the old Ubuntu-14.04 docker image.. that one can be used as a playground for gcc-4.8.x
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/docker/ubuntu14.04/Dockerfile

Maybe we can add a docker image for rhel7 .. I had something here in another project, which we may repurpose for OpenTelemetry C++ SDK build:
https://github.com/microsoft/cpp_client_telemetry/blob/master/tools/setup-buildtools.sh
https://github.com/microsoft/cpp_client_telemetry/blob/master/docker/centos7/Dockerfile

We could have also recommended something like scl enable devtoolset-7, which would install gcc-7 on older system.

[owent]

Maybe I can move some build scripts of my project here to support gcc 4.8?

Makes sense. Can you check out these scripts:

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/tools/build.sh
Sorry, this one is for Windows, but I can add the same script for *nix:
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/tools/build-docker.cmd
And the old Ubuntu-14.04 docker image.. that one can be used as a playground for gcc-4.8.x
https://github.com/open-telemetry/opentelemetry-cpp/blob/main/docker/ubuntu14.04/Dockerfile

Maybe we can add a docker image for rhel7 .. I had something here in another project, which we may repurpose for OpenTelemetry C++ SDK build:
https://github.com/microsoft/cpp_client_telemetry/blob/master/tools/setup-buildtools.sh
https://github.com/microsoft/cpp_client_telemetry/blob/master/docker/centos7/Dockerfile

We could have also recommended something like scl enable devtoolset-7, which would install gcc-7 on older system.

There are some binary SDK packages provided by upstream in our system, so I can not just upgrade toolchains for now.
I have rebase from main branch again, but the build scripts I mean are pure cmake implement.
In my understanding, to build opentelemetry-cpp with OTLP and gRPC, we should also build:

1. zlib
2. re2
3. protobuf
4. cares
5. abseil
6. libcurl
7. openssl(without go)/boringssl(with go)
8. upb(it's already included by grpc)
9. nlohmann_json

I have create a toolset on https://github.com/atframework/cmake-toolset to build above tools/libraries and it works on both Linux, macOS and Windows. It can works with gcc 4.8 and also be used to build iOS and Android targets. And even more, it allow me to use our local mirror repositories instead of pulling from github, and I can custom some building options when need.
I'm trying to add opentelemetry-cpp into it, but I wonder if it's OK to move some utility build scripts and ports above into opentelemetry-cpp?

[maxgolov]

@owent - can this PR be closed without merging, I think you solved it in your other PR?

[lalitb]

@maxgolov I don't think this is solved yet - we still can't upgrade to grpc 1.38.0 because of our gcc 4.8 support. We should either change our base version support to gcc 4.9 or don't support OTLP with gcc4.8 ( and keep supporting it for non-otlp customers ).

[owent]

@owent - can this PR be closed without merging, I think you solved it in your other PR?

#810 do not solve the problem of using latest gRPC.It just upgrade gRPC to a newer version than before to support bazel 4.