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.

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

Upgrade to gRPC 1.46.3 (#1459) #1462

Closed
wants to merge 8 commits into from

Conversation

marcalff
Copy link
Member

Fixes #1459

Changes

Upgrade to gRPC 1.46.3

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

@marcalff marcalff requested a review from a team June 21, 2022 14:43
@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #1462 (384f8d3) into main (39d6a22) will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1462      +/-   ##
==========================================
- Coverage   85.02%   84.96%   -0.06%     
==========================================
  Files         156      156              
  Lines        4977     4977              
==========================================
- Hits         4231     4228       -3     
- Misses        746      749       +3     
Impacted Files Coverage Δ
ext/src/http/client/curl/http_client_curl.cc 80.31% <0.00%> (-1.13%) ⬇️

@marcalff
Copy link
Member Author

The Bazel build fails with the following message:

2022-06-21T14:51:15.8227347Z ERROR: /home/runner/.cache/bazel/e6b8d6759295e14b76bc8cb98b604748/external/com_github_grpc_grpc/BUILD:964:16: Compiling src/core/lib/config/core_configuration.cc failed: (Exit 1): gcc failed: error executing command /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer '-std=c++0x' -MD -MF ... (remaining 48 argument(s) skipped)

File src/core/lib/config/core_configuration.cc is part of gRPC, so this looks like when building gRPC itself.

The gcc error message is not printed, making this hard to investigate.

Any idea anyone ?

@lalitb
Copy link
Member

lalitb commented Jun 21, 2022

The gcc error message is not printed, making this hard to investigate.

Looks like if the error data is larger than 1mb, bazel doesn't print it to stderr :
stderr (/home/runner/.cache/bazel/e6b8d6759295e14b76bc8cb98b604748/execroot/io_opentelemetry_cpp/bazel-out/_tmp/actions/stderr-2073) exceeds maximum size of --experimental_ui_max_stdouterr_bytes=1048576 bytes; skipping
It does print the filename containing the error message. Probably running locally, and error from file gives more information ?

@esigo esigo enabled auto-merge (squash) June 21, 2022 19:31
@marcalff
Copy link
Member Author

marcalff commented Jun 21, 2022

Hi @lalitb

I never used Bazel before, so I am really struggling with it.

On a local build on a linux machine, I get:

malff@localhost.localdomain:opentelemetry-cpp> bazel build //...
ERROR: /home/malff/CODE/MARC_GITHUB/opentelemetry-cpp/exporters/jaeger/BUILD:111:11: @bazel_tools//platforms:osx is not a valid configuration key for //exporters/jaeger:jaeger_thrift_gencpp
ERROR: Analysis of target '//exporters/jaeger:jaeger_thrift_gencpp' failed; build aborted: @bazel_tools//platforms:osx is not a valid configuration key for //exporters/jaeger:jaeger_thrift_gencpp
INFO: Elapsed time: 0.035s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded, 0 targets configured)

which seems even worse.

@marcalff
Copy link
Member Author

marcalff commented Jun 21, 2022

gRPC 1.46.3 depends on:
third_party/abseil-cpp (20211102.0)

The gRPC contains uses of

src/core/lib/channel/channel_args.h: decltype(ChannelArgTypeTraits<absl::remove_cvref_t<
src/core/lib/channel/channel_args.h: absl::remove_cvref_t<decltype(*store_value)>>::VTable()));

using remove_cvref_t in abseil.

auto-merge was automatically disabled June 21, 2022 20:55

Head branch was pushed to by a user without write access

@marcalff marcalff marked this pull request as draft June 21, 2022 20:58
@marcalff marcalff force-pushed the fix_upgrade_grpc_1459 branch from 977f612 to e43cff8 Compare June 22, 2022 17:17
@lalitb
Copy link
Member

lalitb commented Jul 20, 2022

Hi @lalitb

I never used Bazel before, so I am really struggling with it.

On a local build on a linux machine, I get:

malff@localhost.localdomain:opentelemetry-cpp> bazel build //...
ERROR: /home/malff/CODE/MARC_GITHUB/opentelemetry-cpp/exporters/jaeger/BUILD:111:11: @bazel_tools//platforms:osx is not a valid configuration key for //exporters/jaeger:jaeger_thrift_gencpp
ERROR: Analysis of target '//exporters/jaeger:jaeger_thrift_gencpp' failed; build aborted: @bazel_tools//platforms:osx is not a valid configuration key for //exporters/jaeger:jaeger_thrift_gencpp
INFO: Elapsed time: 0.035s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded, 0 targets configured)

which seems even worse.

Sorry I missed this message. Will have a look to bazel build.

@owent
Copy link
Member

owent commented Aug 23, 2022

BTW: Why don't we upgrade gRPC to the latest version of gRPC(v1.48.0)?

@lalitb
Copy link
Member

lalitb commented Aug 23, 2022

BTW: Why don't we upgrade gRPC to the latest version of gRPC(v1.48.0)?

Yes. we can try upgrading to gRPC v1.48.0, and/or bazel v4.2.2. Saw somewhere that the segmentation fault: 11 we are observing in MacOS is fixed in the latest version of bazel.

@lalitb
Copy link
Member

lalitb commented Aug 24, 2022

BTW: Why don't we upgrade gRPC to the latest version of gRPC(v1.48.0)?

Yes. we can try upgrading to gRPC v1.48.0, and/or bazel v4.2.2. Saw somewhere that the segmentation fault: 11 we are observing in MacOS is fixed in the latest version of bazel.

Ok, I realize it won't be straightforward to upgrade to 1.48.0, as it needs C++14 to build as discussed here - #1449.

@marcalff
Copy link
Member Author

Change superseded by PR #1608

Abandoning this branch.

@marcalff marcalff closed this Sep 19, 2022
@marcalff marcalff deleted the fix_upgrade_grpc_1459 branch July 4, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dependencies] Upgrade to gRPC version 1.46.3
3 participants