-
Notifications
You must be signed in to change notification settings - Fork 369
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
cleanup(otel): bazel flag is obsolete #14416
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14416 +/- ##
=======================================
Coverage 93.58% 93.58%
=======================================
Files 2313 2313
Lines 206884 206884
=======================================
+ Hits 193617 193619 +2
+ Misses 13267 13265 -2 ☔ View full report in Codecov by Sentry. |
build --@google_cloud_cpp//:enable_opentelemetry | ||
|
||
# OpenTelemetry versions < v1.16.0 are not compatible with Abseil |
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.
After me, nobody will read this comment block. doc/packaging.md
may be a better place, or the CHANGELOG.md
file.
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.
nobody will read this comment block.
Likely true. I was really banking on people finding the write up in the quickstart's README.
doc/packaging.md
may be a better place, or theCHANGELOG.md
file.
You are probably right that the more places we say words the better. I wrote up some words in the CHANGELOG. I also added a link (if you click enough times) from the packaging doc to the quickstart README.
In
v1.16.0
, OpenTelemetry started compiling with Abseil all the time with Bazel. See open-telemetry/opentelemetry-cpp#2679This means their
--io_opentelemetry_cpp//api:with_abseil
flag is now obsolete. Yay. That is good moving forward.They plan to remove the flag in the future.
The annoying this is that we cannot know which version of OpenTelemetry a downstream target is using. So we will not know if the flag is required or if the flag has been removed. Yuck.
I think our best path forward is to adapt to the change, and try to alert customers to configuration errors through documentation.
This change is