-
Notifications
You must be signed in to change notification settings - Fork 417
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
#574: add instructions about how to build with Bazel #747
Changes from 1 commit
78bba1b
b9fdf2a
83b0044
9d32e8e
d130452
1d2d9bf
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 | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -134,4 +134,55 @@ target_link_libraries(foo PRIVATE ${OPENTELEMETRY_CPP_LIBRARIES}) | |||||||
|
||||||||
## Build instructions using Bazel | ||||||||
|
||||||||
TBD | ||||||||
### Prerequisites | ||||||||
|
||||||||
- A supported platform (e.g. Windows, macOS or Linux). | ||||||||
Refer to [Platforms Supported](./README.md#supported-development-platforms) | ||||||||
for more information. | ||||||||
- A compatible C++ compiler supporting at least C++11. | ||||||||
Major compilers are supported. | ||||||||
Refer to [Supported Compilers](./README.md#supported-c-versions) for more information. | ||||||||
- [Git](https://git-scm.com/) for fetching opentelemetry-cpp source code from repository. | ||||||||
To install Git, consult the [Set up Git](https://help.github.com/articles/set-up-git/) | ||||||||
guide on GitHub. | ||||||||
- [Bazel](https://www.bazel.build/) for building opentelemetry-cpp API, | ||||||||
SDK with their unittests. We use 3.7.2 in our build system. | ||||||||
|
||||||||
To install Bazel, consult the [Installing Bazel](https://docs.bazel.build/versions/3.7.0/install.html) guide. | ||||||||
|
||||||||
### Building as Standalone Bazel Project | ||||||||
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. Isn't this more useful for contributors, less so installation? E.g. I'd expect most bazel users to want to do something like pulling in OTel CPP via Not that this isn't useful, just not how I'd expect folks to consume. 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. e.g. https://github.com/GoogleCloudPlatform/opentelemetry-operations-cpp/blob/main/WORKSPACE#L46 Or you can see: https://github.com/googleapis/google-cloud-cpp/pull/1230/files for an example usage of OpenCensus C++ 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 is a good point. We can have a separate section (say) "Incorporating Into An Existing Project", which includes the above-mentioned steps, similar to how we document for |
||||||||
|
||||||||
1. Getting the opentelementry-cpp source: | ||||||||
|
||||||||
```console | ||||||||
# Change to the directory where you want to create the code repository | ||||||||
$ cd ~ | ||||||||
$ mkdir source && cd source | ||||||||
$ git clone https://github.com/open-telemetry/opentelemetry-cpp | ||||||||
Cloning into 'opentelemetry-cpp'... | ||||||||
... | ||||||||
Resolving deltas: 100% (3225/3225), done. | ||||||||
$ | ||||||||
``` | ||||||||
|
||||||||
2. Download the dependencies and build the source code: | ||||||||
|
||||||||
```console | ||||||||
$ bazel build //... | ||||||||
bazel build -- //... -//exporters/otlp/... -//exporters/prometheus/... | ||||||||
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. Technically the exclusion here
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. How about just invoke 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. Maybe we need to add a script for Linux then, like 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.
Suggested change
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. OTLP-gRPC takes forever to build though 😄 (293s total on my docker container with WSL2) |
||||||||
Extracting Bazel installation... | ||||||||
Starting local Bazel server and connecting to it... | ||||||||
INFO: Analyzed 121 targets (98 packages loaded, 3815 targets configured). | ||||||||
INFO: Found 121 targets... | ||||||||
INFO: From Compiling sdk/src/trace/tracer_context.cc: | ||||||||
... | ||||||||
|
||||||||
``` | ||||||||
|
||||||||
4. Once Bazel tests are built, run them with `bazel test //...` command | ||||||||
|
||||||||
```console | ||||||||
$ bazel test //... | ||||||||
``` | ||||||||
|
||||||||
5. The build artifacts will be located under `bazel-bin` |
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.
Can we somehow omit
3.7.2.
here. It's 10 seconds to follow the guidelines, even if we don't mention the version explicitly. Maybe suggesting to check what version is being used indirectly, by lookup up the file contents here:opentelemetry-cpp/.bazelversion
Line 1 in 414a731
Something like: Please install the exact version of bazel mentioned in
.bazelversion
?Even if someone messes it up like I did, it takes 10 seconds to correct it - it even tells how to correct / install the right version. I think we might be upgrading the Bazel soon anyways, so probably not worth it anchoring to specific version in the how-to doc.
@jsuereth
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.
Yeah, I think we should likely encourage folks to install/use Bazelisk, as it can auto-correct versions on the fly and leads to a much smoother developer experience when you move back/forth between different bazel projects.