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

Use custom abseil with bazel #1172

Merged
merged 5 commits into from
Jan 15, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,28 @@ jobs:
- name: run tests
run: ./ci/do_ci.sh bazel.test

bazel_with_abseil:
name: Bazel with external abseil
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
submodules: 'recursive'
- name: Mount Bazel Cache
uses: actions/cache@v2
env:
cache-name: bazel_cache
with:
path: /home/runner/.cache/bazel
key: bazel_test
- name: setup
run: |
sudo ./ci/setup_thrift.sh dependencies_only
sudo ./ci/setup_ci_environment.sh
sudo ./ci/install_bazelisk.sh
- name: run tests
run: ./ci/do_ci.sh bazel.with_abseil

bazel_valgrind:
name: Bazel valgrind
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Increment the:

* [SDK] Add LogLevel to internal_log ([#1147](https://github.com/open-telemetry/opentelemetry-cpp/pull/1147))
* [API/SDK] Logger: Propagating resources through LoggerProvider ([#1154](https://github.com/open-telemetry/opentelemetry-cpp/pull/1154))
* [API]: Allow to use external abseil for bazel targets ([#1172](https://github.com/open-telemetry/opentelemetry-cpp/pull/1172))

## [1.1.1] 2021-12-20

Expand Down
27 changes: 27 additions & 0 deletions api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,38 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")

package(default_visibility = ["//visibility:public"])

bool_flag(
name = "with_abseil",
build_setting_default = False,
)

cc_library(
name = "api",
hdrs = glob(["include/**/*.h"]),
defines = select({
":with_external_abseil": ["HAVE_ABSEIL"],
"//conditions:default": [],
}),
strip_include_prefix = "include",
tags = ["api"],
deps = select({
":with_external_abseil": [
"@com_google_absl//absl/base",
"@com_google_absl//absl/types:any",
"@com_google_absl//absl/types:variant",
"@com_google_absl//absl/types:span",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - do we need to add a dependency for absl::span?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@com_google_absl//absl/types:any and @com_google_absl//absl/types:span are removed. The failed jobs seem not be the problem of this PR.

"@com_google_absl//absl/numeric:bits",
"@com_google_absl//absl/hash:city",
],
"//conditions:default": [],
}),
)

config_setting(
name = "with_external_abseil",
flag_values = {":with_abseil": "true"},
)
4 changes: 2 additions & 2 deletions api/include/opentelemetry/nostd/variant.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ OPENTELEMETRY_END_NAMESPACE
# endif

# ifdef HAVE_ABSEIL
# include <absl/types/variant.h>
# include "absl/types/variant.h"
# else
# include "./absl/types/variant.h"
# include "./internal/absl/types/variant.h"
lalitb marked this conversation as resolved.
Show resolved Hide resolved
# endif

# ifdef _MSC_VER
Expand Down
11 changes: 11 additions & 0 deletions bazel/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ def opentelemetry_cpp_deps():
],
)

# Load abseil dependency(optional)
maybe(
http_archive,
name = "com_google_absl",
sha256 = "59b862f50e710277f8ede96f083a5bb8d7c9595376146838b9580be90374ee1f",
strip_prefix = "abseil-cpp-20210324.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abseil version doesn't match with what we use in CI: 20210324.0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The version of internal abseil in api/include/opentelemetry/nostd/absl is v2020-03-03#8 ,should it be upgraded?

Copy link
Member

@lalitb lalitb Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version of internal abseil in api/include/opentelemetry/nostd/absl is v2020-03-03#8, should it be upgraded?

One of the concerns I see with the periodic upgrade of the internal nostd::variant snapshot would be the ABI incompatibility issue if the instrumented library and application are built using different otel-cpp releases (having different nostd::variant snapshots).

urls = [
"https://github.com/abseil/abseil-cpp/archive/20210324.2.tar.gz",
],
)

# Load gRPC dependency
maybe(
http_archive,
Expand Down
4 changes: 4 additions & 0 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ elif [[ "$1" == "bazel.test" ]]; then
bazel $BAZEL_STARTUP_OPTIONS build $BAZEL_OPTIONS //...
bazel $BAZEL_STARTUP_OPTIONS test $BAZEL_TEST_OPTIONS //...
exit 0
elif [[ "$1" == "bazel.with_abseil" ]]; then
bazel $BAZEL_STARTUP_OPTIONS build $BAZEL_OPTIONS --//api:with_abseil=true //...
bazel $BAZEL_STARTUP_OPTIONS test $BAZEL_TEST_OPTIONS --//api:with_abseil=true //...
exit 0
elif [[ "$1" == "bazel.macos.test" ]]; then
bazel $BAZEL_STARTUP_OPTIONS build $BAZEL_MACOS_OPTIONS //...
bazel $BAZEL_STARTUP_OPTIONS test $BAZEL_MACOS_TEST_OPTIONS //...
Expand Down
2 changes: 1 addition & 1 deletion docs/dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Both these dependencies are listed here:
- [API](https://github.com/open-telemetry/opentelemetry-cpp/tree/v1.0.0-rc1/api) & [SDK](https://github.com/open-telemetry/opentelemetry-cpp/tree/v1.0.0-rc1/sdk):
- Uses Standard C++ library for latest features (std::string_view, std::variant, std::span, std::shared_ptr, std::unique_ptr) with C++14/17/20 compiler if `WITH_STL` cmake option is enabled or `HAVE_CPP_STDLIB` macro is defined. License: `GNU General Public License`
- For C++11/14/17 compilers, fallback to gsl::span if [GSL C++ library](https://github.com/microsoft/GSL) is installed. License: `MIT License`
- Uses Abseil C++ Library for `absl::variant` as default `nostd::variant` if `WITH_ABSEIL` cmake option is enabled. License: `Apache License 2.0`
- Uses Abseil C++ Library for `absl::variant` as default `nostd::variant` if `WITH_ABSEIL` cmake option or `--@io_opentelemetry_cpp/api:with_abseil=true` (aka `--//api:with_abseil=true`) bazel option is enabled. License: `Apache License 2.0`

- [OTLP/HTTP+JSON](https://github.com/open-telemetry/opentelemetry-cpp/tree/v1.0.0-rc1/exporters/otlp) exporter:
- [protobuf](https://github.com/protocolbuffers/protobuf): Library to serialize structured data.
Expand Down
4 changes: 3 additions & 1 deletion examples/otlp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ gRPC internally uses a different version of Abseil than OpenTelemetry C++ SDK.

One option to optimize your code is to build the SDK with system-provided
Abseil library. If you are using CMake, then `-DWITH_ABSEIL=ON` may be passed
during the build of SDK to reuse the same Abseil library as gRPC.
during the build of SDK to reuse the same Abseil library as gRPC. If you are
using Bazel, then `--@io_opentelemetry_cpp/api:with_abseil=true` may be passed
to reuse your Abseil library in your project.

If you do not want to pursue the above option, and in case if you run into
conflict between Abseil library and OpenTelemetry C++ `absl::variant`
Expand Down