[BUILD] Install an explicit list of ext headers instead of the whole tree

[thc1006]

Step 2 of the plan in #4315, and a contribution to #1980.

ext_common installed include/opentelemetry/ext with a *.h glob, so every header under that tree shipped whether or not it was meant to. This replaces the glob with a list of the headers the component installs. A header now reaches the package only when it is named, which is the signal you asked for when one is renamed or removed, and it makes each later decision about the installed surface a one line diff.

No longer installed

• client/detail/default_factory.h, an internal helper included by exporter sources rather than by any public header.
• client/curl/http_client_curl.h and client/curl/http_operation_curl.h, the curl implementation. These two go together, since the first includes the second. A consumer creates a curl client through http_client_factory_curl.h, which includes only the abstract http_client_factory.h.
• server/http_server.h and server/socket_tools.h, the embedded HTTP server.

The server and client/detail directories go with them. install(DIRECTORY)
creates a destination directory for every source directory it walks and applies
FILES_MATCHING to the files it finds there, so naming files alone still
installed both, empty. Excluding the two directories drops them from the walk:

with the directory excludes     ext/http/{client/curl, common}
                                empty directories: none

without them                    ext/http/{client/curl, client/detail,
                                          common, server}
                                empty directories: client/detail, server

All five headers are a breaking change for anyone who included them from an installed package, which is why the CHANGELOG names them. The embedded server in particular has shipped for a long time, so out of tree users of it lose it here rather than when it moves.

The embedded server, and what it took to drop it

examples/http/server.h includes http_server.h, and examples_test turned WITH_EXAMPLES_HTTP on whenever ext_http_curl was installed, so examples/http was built against the install prefix. The example is now disabled there unconditionally, per review.

That variable gates exactly one thing, add_subdirectory(http) in examples/CMakeLists.txt, so the coverage this gives up is the http example building against the install prefix, and nothing else. The ext headers that example uses are still exercised by install/test/src/test_ext_http.cc and test_ext_http_curl.cc.

Dropping the headers also made the ws2_32 link on the opentelemetry_ext interface wrong: the target no longer carries a header that calls Winsock, so a package consumer would inherit a link it has no use for. It now goes on the seven targets that compile the server, which is the full transitive closure of the two headers over the include graph on main:

curl_http_test, socket_tools_test, w3c_tracecontext_http_test_server, otlp_http_exporter_test, otlp_http_log_record_exporter_test, zipkin_exporter_test, and the http_server example. The http_client example is not among them; it uses the curl client only.

Two notes on that link. socket_tools.h carries #pragma comment(lib, "ws2_32.lib") under _WIN32, which MSVC and clang-cl honor and MinGW's GCC does not, so every Windows job in CI resolves Winsock through the pragma and none of them can tell the difference either way. And Bazel's //ext:headers still globs the whole include tree, so it still exposes the server and its -DEFAULTLIB:Ws2_32.lib linkopt is still load bearing. It stays.

The file move itself is #4332.

Guarding it

Every other case in the install tests compiles against headers that are present, so putting the glob back over include/opentelemetry/ext would leave all of them passing while the package quietly grew five headers again. install/test/cmake/CMakeLists.txt now fails when any of the five is found under the include directory the installed opentelemetry-cpp::api target points at, and separately when either pruned directory is present.

Checked both ways, for both guards. With the manifest it reports Checking <prefix>/include for headers that must not ship and configures. With the glob restored it stops at

CMake Error at CMakeLists.txt:90 (message):
  The ext_common component installed headers it is meant to leave out:

and with the manifest kept but the two directory excludes removed, at

CMake Error at CMakeLists.txt:109 (message):
  The ext_common component left directories behind in the install tree:

Verification

Installed from main and from this branch into separate clean prefixes and compared the whole trees, not just the headers:

Only in inst-main/include/opentelemetry/ext/http/client/curl: http_client_curl.h
Only in inst-main/include/opentelemetry/ext/http/client/curl: http_operation_curl.h
Only in inst-main/include/opentelemetry/ext/http/client/detail: default_factory.h
Only in inst-main/include/opentelemetry/ext/http/server: http_server.h
Only in inst-main/include/opentelemetry/ext/http/server: socket_tools.h

files only in main: 5    files only in pr: 0
differing content: 13    total files in pr: 631

The five are the intended removals, nothing is added, and the thirteen that differ are all static archives, which are not byte reproducible across builds. Every installed header and every CMake config file is identical.

The install tests then pass against that prefix, 100% tests passed, 0 tests failed out of 37, over api, sdk, ext_common, ext_http, ext_http_curl, the ostream and in memory exporters and zipkin.

As a control, flipping WITH_EXAMPLES_HTTP back on against the same prefix fails, which is the reason the line is there:

95% tests passed, 2 tests failed out of 37

The following tests FAILED:
	 36 - examples-build-test (Failed)
	 37 - examples-run-test (Failed)

examples/http/server.h:6:10: fatal error:
    opentelemetry/ext/http/server/http_server.h: No such file or directory

That run is a Linux subset of what CI covers, nine components rather than twenty one, and it cannot exercise the ws2_32 change at all, since the pragma covers every Windows configuration CI builds. The exhaustive enumeration above is what that part rests on.

Not in scope

Manifests for the other components, the FILE_SET and VERIFY_INTERFACE_HEADER_SETS work that needs CMake 3.24, and moving the embedded server out of ext, which is #4332.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.86%. Comparing base (e79ab1f) to head (b4cac29).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4327      +/-   ##
==========================================
+ Coverage   80.85%   80.86%   +0.02%     
==========================================
  Files         450      450              
  Lines       19215    19215              
==========================================
+ Hits        15535    15537       +2     
+ Misses       3680     3678       -2     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dbarker]

Thanks for the PR and clear description! Please see feedback and requested changes below.

[thc1006]

All six done, thanks for the clear list.

1. set(WITH_EXAMPLES_HTTP OFF) unconditionally in examples_test.
2. Both server patterns out of the manifest.
3. ws2_32 off the opentelemetry_ext interface and onto the seven targets that compile the server. That set is the transitive closure of the two headers over the include graph, so it covers the two OTLP HTTP tests and the zipkin test as well as the ext tests and the example. http_client is not in it.
4. First paragraph gone.
5. TODO added above the call.
6. Move the embedded HTTP server headers out of ext/include #4332.

One thing I deliberately did not change, in case it reads as an oversight: Bazel's //ext:headers keeps its -DEFAULTLIB:Ws2_32.lib. That target still globs the whole include tree, so it still exposes the server and the linkopt is still load bearing there. #4332 is where the two build systems line up again.

Worth knowing about the ws2_32 move: socket_tools.h has #pragma comment(lib, "ws2_32.lib") under _WIN32, which MSVC and clang-cl honor, and every Windows job in CI is one of those. So CI cannot fail either way, and the enumeration is what this rests on rather than a green run.

Verified by installing main and this branch into clean prefixes: five headers gone, none added, and the only files that differ are the thirteen static archives. Install tests are 37/37 against the manifest prefix, and forcing WITH_EXAMPLES_HTTP back on against that same prefix fails examples-build-test and examples-run-test on the missing http_server.h, which is the control for that one line. Numbers are in the description.

Last thing: #4283 and #4294 both touch ext/test/http/CMakeLists.txt, so whichever lands second needs a rebase. Neither needs the link, both new targets are if(NOT WIN32).

[dbarker]

Thanks for the changes. Looks good to me. Minor request below to the changelog entry.

[thc1006]

Your approval is recorded against f4573881 and four commits have landed since, so rather than let them ride on it:

• 6ade65d0 is the CHANGELOG breaking change call out you asked for.
• ed6f6d77 adds the negative case to the install test. Every existing case compiles against headers that are present, so putting the glob back would leave all of them passing while the package quietly grew five headers again.
• 9e187675 hardens that check. It reads INTERFACE_INCLUDE_DIRECTORIES off the imported api target, and an unset property yields <VAR>-NOTFOUND, which list(GET) returns without complaint, so a check whose only job is to fail loudly could have passed having looked at nothing.
• a75c0e53 closes something the file manifest alone did not. install(DIRECTORY) creates a destination directory for every source directory it walks and applies FILES_MATCHING to what it finds there, so naming only file names still installed include/opentelemetry/ext/http/server/ and .../client/detail/, empty. Two directory excludes drop them from the walk.

Installed into a clean prefix both ways:

with the directory excludes     ext/http/{client/curl, common}
                                empty directories: none

without them                    ext/http/{client/curl, client/detail,
                                          common, server}
                                empty directories: client/detail, server

Both guards were checked in the failing direction too. With the glob restored the install test stops at The ext_common component installed headers it is meant to leave out:, and with the manifest kept but only the two directory excludes removed, at The ext_common component left directories behind in the install tree:.

The last commit is the one furthest from what you reviewed. Say the word and I will drop it and open it separately, or squash it in, whichever you prefer.

Issue for the file move is #4332.

[dbarker]

Please see the requested change below to revert the latest install test change.

[dbarker]

Thank you for the updates!