-
Notifications
You must be signed in to change notification settings - Fork 438
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
bump to gRPC v1.48.1 for bazel CIs #1786
Changes from 2 commits
4c3161f
d3a902a
d92975a
8e8a82a
139d7bc
238fe46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
# include "opentelemetry/sdk/trace/tracer_provider.h" | ||
# include "opentelemetry/trace/provider.h" | ||
|
||
# include <google/protobuf/message_lite.h> | ||
# include <gtest/gtest.h> | ||
# include "gmock/gmock.h" | ||
|
||
|
@@ -466,6 +467,7 @@ TEST_F(OtlpHttpExporterTestPeer, ExportJsonIntegrationTestSync) | |
TEST_F(OtlpHttpExporterTestPeer, ExportJsonIntegrationTestAsync) | ||
{ | ||
ExportJsonIntegrationTestAsync(); | ||
google::protobuf::ShutdownProtobufLibrary(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is protobuf library loaded explicitly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes it is optional, here we need it to prevent the leak report though. |
||
} | ||
# endif | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes #1787, gRPC with C++11 needed to be defined in the main branch first.