[BUILD] Bazel //ext:headers exposes headers the CMake install excludes

[thc1006]

The two build systems do not agree on which ext headers a consumer is allowed to include.

CMake installs four, by name, and excludes the detail and server directories outright, in ext/CMakeLists.txt#L37-L51:

  FILES_MATCHING
  PATTERN
  "http_client.h"
  PATTERN
  "http_client_factory.h"
  PATTERN
  "http_client_factory_curl.h"
  PATTERN
  "url_parser.h"
  PATTERN
  "detail"
  EXCLUDE
  PATTERN
  "server"
  EXCLUDE)

Bazel takes all ten, in ext/BUILD#L10:

cc_library(
    name = "headers",
    hdrs = glob(["include/**/*.h"]),

The six a Bazel consumer gets and a CMake consumer does not:

ext/include/opentelemetry/ext/http/client/curl/http_client_curl.h
ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h
ext/include/opentelemetry/ext/http/client/curl/http_time_util.h
ext/include/opentelemetry/ext/http/client/detail/default_factory.h
ext/include/opentelemetry/ext/http/server/http_server.h
ext/include/opentelemetry/ext/http/server/socket_tools.h

Both halves compile. On b4257f0a, with a package whose only deps are //api and //ext:headers:

$ bazel build //extprobe:server_consumer   # #include ".../ext/http/server/http_server.h"
INFO: Build completed successfully

$ bazel build //extprobe:curl_consumer     # #include ".../ext/http/client/curl/http_operation_curl.h"
INFO: Build completed successfully

The comment above the CMake block says why each exclusion is there: detail/default_factory.h is an internal helper, the two concrete curl headers are reached through http_client_factory_curl.h, and the server headers are for tests and the http example. None of that is enforced on the Bazel side, so the curl Session and HttpOperation layouts are reachable by a Bazel consumer today, and changing either is a break for them even though the install tree says they are not part of the surface.

I ran into this while looking at whether making a member of HttpOperation atomic was an ABI question. Under CMake it plainly is not. Under Bazel I could not say the same.

Two directions, and I do not think the choice is mine to make:

1. Narrow //ext:headers to the four CMake installs, and give the rest their own target for the in-tree tests and examples that need them.
2. Treat the Bazel glob as the real surface and widen the CMake install to match, which contradicts the reasoning already written into ext/CMakeLists.txt.

#4332 proposes moving the server headers somewhere internal, which removes two of the six either way, but not the curl four.

The open decisions in #4458 record the same disagreement, since whichever shape the transport rewrite takes has to say what happens to a Bazel consumer that includes a concrete header.

[github-actions]

This issue is available for anyone to work on. Make sure to reference this issue in your pull request.
✨ Thank you for your contribution! ✨

[ParthibanRajasekaran]

Looking at this, I think Option A (narrowing Bazel) is the better path. The CMakeLists.txt comments clearly explain why each header is excluded, it's intentional API design, not accidental omission.

Narrowing ext/BUILD to match the four CMake headers would:

• Respect the existing design intent documented in ext/CMakeLists.txt
• Keep the public API stable for CMake consumers who already rely on those four headers
• Avoid locking implementation details into a stable API surface
• Work cleanly with Move the embedded HTTP server headers out of ext/include #4332 if server headers move to test_common

The six headers Bazel currently exposes fall into three categories: curl implementation internals (http_client_curl, http_operation_curl, http_time_util), internal helpers (detail/default_factory), and test infrastructure (http_server, socket_tools). None are meant for direct consumer use.

Could a maintainer confirm this direction before we put together a fix?

[thc1006]

Thanks for picking this up. Your reading of the CMake comments matches mine: they explain each exclusion, so the install list looks deliberate rather than accidental.

One thing worth knowing before you size the change. detail/default_factory.h is not only reachable by outside consumers, it is included by three shipped exporter sources:

exporters/otlp/src/otlp_http_client.cc
exporters/elasticsearch/src/es_log_record_exporter.cc
exporters/zipkin/src/zipkin_exporter.cc

So narrowing //ext:headers to the four installed headers is not only a matter of relocating test infrastructure. Those three are production code in this repo and would need something to depend on. The server headers have the same shape in a smaller way: examples/http/server.h and exporters/otlp/test/otlp_http_exporter_test.cc include http_server.h, and exporters/zipkin/test/zipkin_exporter_test.cc includes http_client_curl.h.

Measured on f77c1a5c, thirteen targets across eight packages name //ext:headers today:

ext/src/http/client/curl/BUILD:33
ext/test/http/BUILD:15, :28
ext/test/w3c_tracecontext_http_test_server/BUILD:22
test_common/src/http/client/nosend/BUILD:20
examples/custom_http_client/BUILD:17
examples/http/BUILD:24, :41
exporters/otlp/BUILD:118, :168, :394, :652
exporters/elasticsearch/BUILD:30

That is the set a narrowing has to keep building, which I think makes the shape roughly: the four installed headers stay in //ext:headers, the curl headers and detail/default_factory.h move to a target the exporters and ext's own sources can depend on, and the server headers go wherever #4332 decides to put them.

I am not working on this, so it is yours if you want it. I opened it because I could not tell which of the two surfaces was the intended one, and that part is still the question a maintainer needs to answer rather than something the code settles.

[ParthibanRajasekaran]

Got it. Keep the 4 installed headers in //ext:headers, move the curl headers and detail/default_factory to a new internal target for the exporters, server headers per #4332. I can take this on.

[thc1006]

Thanks for taking it. One correction to what I wrote, before it costs you time: I said the curl headers and detail/default_factory.h could share one internal target. That would hand the exporters the curl headers as well, which is the exposure being removed. They want to be separate.

What each dependent actually includes, measured file by file on f77c1a5c:

package	file	needs from the six
ext/src/http/client/curl	http_client_curl.cc, http_client_factory_curl.cc, http_operation_curl.cc, http_time_util.cc	http_client_curl.h, http_operation_curl.h, http_time_util.h
ext/test/http	curl_http_test.cc	http_client_curl.h, http_operation_curl.h, http_server.h
ext/test/http	socket_tools_test.cc	socket_tools.h
ext/test/w3c_tracecontext_http_test_server	main.cc	http_client_curl.h, http_server.h
examples/http	server.h	http_server.h
exporters/otlp	src/otlp_http_client.cc	default_factory.h
exporters/otlp	test/otlp_http_exporter_test.cc	http_server.h
exporters/elasticsearch	src/es_log_record_exporter.cc	default_factory.h
exporters/zipkin	src/zipkin_exporter.cc	default_factory.h
exporters/zipkin	test/zipkin_exporter_test.cc	http_client_curl.h
examples/custom_http_client		nothing from the six
test_common/src/http/client/nosend		nothing from the six

http_time_util.h has no consumer outside ext/src/http/client/curl, so it can sit with the curl implementation and never be visible elsewhere.

Three groups rather than two: the curl implementation headers, detail/default_factory.h on its own, and the server pair. Note that the exporter libraries want only detail/default_factory.h, while their tests want the server header, so the visibility for those two differs.

One consumer is easy to miss. exporters/zipkin/BUILD never names //ext:headers, yet zipkin_exporter.cc includes default_factory.h and zipkin_exporter_test.cc includes http_client_curl.h. It reaches them through intermediaries: the library through //ext/src/http/client/curl:http_client_curl, the test through that and //test_common/src/http/client/nosend:http_client_nosend, both of which do name //ext:headers. Whether zipkin survives the narrowing depends on whether those intermediaries carry the new targets forward, which I have not tried, so it is worth checking on purpose rather than assuming the thirteen //ext:headers entries across eight BUILD files cover everything.

The reassuring part is that the split is possible at all. The four installed headers are a closed set: http_client_factory_curl.h includes only http_client_factory.h, that includes only http_client.h, and url_parser.h includes nothing. None of the four reaches into the six. Going the other way, default_factory.h includes both public factory headers, http_client_curl.h includes http_operation_curl.h, and http_server.h includes socket_tools.h, so those pairs move together.

Worth a line in your description: examples/custom_http_client needs nothing outside the public four. It is the in-tree example of writing a custom client, so the surface CMake installs is already enough to do that, which is a decent argument that the four are the right four.

Edited. Two rows of the first table were wrong and I would rather you did not build on them. ext/test/http did not need http_time_util.h, and exporters/otlp also needs http_server.h in its test. Both came from a grep for #include that does not match # include, which is how otlp_http_exporter_test.cc:64 writes it. The table above is derived per file with a pattern that tolerates the space. I also said zipkin would break; I have not run that, and it is stated as what needs checking instead.