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

add CI test with gRPC latest release

Open esigo opened this issue 3 years ago • 4 comments
trafficstars

Fixes #1607 (issue)

Changes

Adds a new CI which tests otlp exporters using the latest gRPC release.

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

esigo avatar Sep 10 '22 17:09 esigo

Codecov Report

Merging #1608 (b2e82b6) into main (5c180a1) will not change coverage. The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1608   +/-   ##
=======================================
  Coverage   85.12%   85.12%           
=======================================
  Files         159      159           
  Lines        4999     4999           
=======================================
  Hits         4255     4255           
  Misses        744      744           

codecov[bot] avatar Sep 10 '22 17:09 codecov[bot]

Thanks for the PR. A couple of nit comments

  • Instead of adding a new CI, should we use the latest gRPC for all cmake builds on ubuntu-latest/ubuntu-20.04/ubuntu-20.10. These virtual machine uses g++-9.3 as the default compiler, which builds in c++-14 mode.
  • Can we try using the latest gRPC for bazel builds too? Not sure if it will work, as it failed earlier in #1462. We can do it in separate PR if it takes time
  • Should we document the gRPC dependency here - https://github.com/open-telemetry/opentelemetry-cpp#supported-development-platforms. We already do it for gcc4.8 for gRPC here.

lalitb avatar Sep 11 '22 18:09 lalitb

Thanks for the PR. A couple of nit comments

  • Instead of adding a new CI, should we use the latest gRPC for all cmake builds on ubuntu-latest/ubuntu-20.04/ubuntu-20.10. These virtual machine uses g++-9.3 as the default compiler, which builds in c++-14 mode.
  • Can we try using the latest gRPC for bazel builds too? Not sure if it will work, as it failed earlier in Upgrade to gRPC 1.46.3 (#1459) #1462. We can do it in separate PR if it takes time
  • Should we document the gRPC dependency here - open-telemetry/opentelemetry-cpp#supported-development-platforms. We already do it for gcc4.8 for gRPC here.

One more item is for Windows build, could we also make the same update there?

ThomsonTan avatar Sep 12 '22 20:09 ThomsonTan

One more item is for Windows build, could we also make the same update there?

That can be challenging it seems. We use vcpkg to install windows dependencies as here and latest gRPC version in vcpkg is v1.41.0.

lalitb avatar Sep 12 '22 22:09 lalitb

@lalitb Windows CMake CI doesn't use gRPC. We use C++14 for building gRPC itself only so I believe we don't need to change our documentation. I would keep this PR as it is.

I raised another PR to bump vcpkg #1633. I created another issue for bazel #1634.

esigo avatar Sep 24 '22 09:09 esigo

meanwhile gRPC 1.49.1 released.

esigo avatar Sep 25 '22 10:09 esigo