[BUG] Check what curl_global_init returns (#4434)

[Dyfintie]

Fixes #4434.

## Changes
- [x] Bug fix (non-breaking change which fixes an issue)

This PR addresses the unhandled return value of `curl_global_init(CURL_GLOBAL_ALL)` in `HttpCurlGlobalInitializer`:
- Stores the return `CURLcode` and tracks an `is_initialized_` boolean status.
- Logs an error with `OTEL_INTERNAL_LOG_ERROR` if `curl_global_init` fails.
- Skips calling `curl_global_cleanup()` in the destructor if initialization failed.
- Avoids creating `multi_handle_` in `HttpClient` if global init failed.
- Immediately signals `SessionState::CreateFailed` on `HttpClient::SendRequest()`, `HttpClientSync::Get()`, and `HttpClientSync::Post()`.
- Adds unit tests in `curl_http_test.cc` via `HttpClientTestPeer` covering both asynchronous and synchronous client behaviors on

initialization failure.

## Testing
- Added unit tests:
  - `BasicCurlHttpTests.GlobalInitFailureHttpClient`
  - `BasicCurlHttpTests.GlobalInitFailureHttpClientSync`
- Ran full test suite locally: all 28 tests in `curl_http_test` passed.
- Formatted with `clang-format`.

For significant contributions please make sure you have completed the following items:

* [ ] `CHANGELOG.md` updated for non-trivial changes
* [x] Unit tests have been added
* [x] Changes in public API reviewed

[thc1006]

Thanks for this, and for asking on the issue first. Two things I checked that I think are right, and one question.

The guard in Session::SendRequest is in the right place, and it is worth saying why, because it is easy to lose in a later edit. It is the first statement in the function, so it returns before http_client_.MaybeSpawnBackgroundThread(). That matters: the background loop calls curl_multi_perform, curl_multi_poll, curl_multi_wait and curl_multi_info_read on multi_handle_ with no null check, so the thread never starting is what keeps those safe rather than any check inside them.

Moving the curl_global_initializer_ declaration up in the header is also right, and necessary rather than cosmetic. Members initialise in declaration order and not in the order of the initialiser list, so without the header change the new multi_handle_ initialiser would read a member that had not been constructed yet.

The question. On main there are four more uses of multi_handle_ that are not null checked: curl_multi_add_handle around line 752, curl_multi_remove_handle around 819 and 863, and the curl_multi_cleanup and curl_multi_init pair around 913. Reading the code I could not reach any of them from a failed global init, because each sits behind either the background thread or a session that was already scheduled, and this PR stops both. curl_multi_wakeup is already guarded on main. Since this PR is what makes a null multi_handle_ possible in the first place, it seems worth confirming that rather than inferring it, and a case that constructs the client with a failed initializer and then calls CancelAllSessions() and FinishAllSessions() would cover the destructor path.

One overlap to be aware of rather than to act on. #4406 makes the client survive without a multi handle from the other direction, where curl_multi_init() itself fails, and it measured what the current code does in that state: 3.00 seconds of CPU in a 3 second window and 1,168,126 error lines from the IO thread. Same end state, different trigger. It is a draft and parked, so please do not wait for it. I will rebase onto whichever of us lands first.

Small thing: "a private constructor to to simulate" in the header has a doubled "to".

[Dyfintie]

Thank you so much for the thorough review and insights :)
I have updated the PR with the following changes:

1. Guard rationale comment: Added a comment in
   Session::SendRequest documenting that returning early before
   MaybeSpawnBackgroundThread() prevents the background IO loop
   from starting with an uninitialized or null multi_handle_.

2. Explicit test coverage: Extended BasicCurlHttpTests.
   GlobalInitFailureHttpClient to explicitly call client.
   CancelAllSessions() and client.FinishAllSessions(), ensuring the
   cancellation and destructor paths execute cleanly when
   multi_handle_ is null.

3. Comment typo fix: Fixed the doubled "to" in the header
   comment.:)

4. Also fixed implemented the linter fixes.

[thc1006]

All three are done, and I checked each rather than taking the summary for it.

The guard comment says the thing that matters, that the early return happens before MaybeSpawnBackgroundThread(). The test now injects the failed initializer, asserts IsValid() is false, sends a request, and then calls CancelAllSessions() and FinishAllSessions() with multi_handle_ null, and the client is a stack object so the destructor runs on the same path. That is what I was asking to see confirmed rather than inferred. The sync case covering Get and Post returning CreateFailed was not something I asked for and is a good addition. The doubled "to" is gone.

Worth flagging separately, because you cannot see it from your side and it is not something you did wrong: no CI has run on 2c5efd26. The pull request shows only the EasyCLA check, and the API says there are eight GitHub Actions check suites sitting at conclusion: action_required:

gh api repos/open-telemetry/opentelemetry-cpp/commits/2c5efd26/check-suites \
  --jq '.check_suites[] | "\(.app.name) \(.status) \(.conclusion)"'
GitHub Actions completed action_required     (x8)

action_required is the approval gate for a first contribution: workflows do not start until a maintainer presses "Approve and run workflows", and it applies again to each new push until you have something merged. CI did run on your earlier head, which is how I saw the MSVC red last week, so this started when you pushed the fixes above.

So the pull request is not being ignored, it is unevaluated. @marcalff or @lalitb, could one of you approve the workflow run on this one? The change is small, EasyCLA has passed, and it closes #4434.

[thc1006]

The workflows ran, and the 31 red checks are one compile error rather than 31 problems. It is a one line fix and you could not have hit it locally, which I will explain.

CI reports this:

http_client_curl.cc:56:29: error: invalid operands of types 'const char [61]' and 'int' to binary 'operator<<'

That is the second error, not the first. Reproducing it locally gives both:

error: invalid operands of types 'const char [61]' and 'int' to binary 'operator<<'
error: 'OTEL_INTERNAL_LOG_ERROR' was not declared in this scope

The macro is not declared, so the compiler reads OTEL_INTERNAL_LOG_ERROR(...) as a call and tries to evaluate its argument, and "a string" << an int on its own is what it then complains about. The << is fine.

Why it is not declared: the include sits inside the #else of a guard, near the top of the file.

#ifdef ENABLE_OTLP_COMPRESSION_PREVIEW
...
#else
#  include "opentelemetry/sdk/common/global_log_handler.h"
#endif

The one log call that existed before yours is at line 217, inside the same #else, so it never needed the header anywhere else. Your new call at line 55 is outside the guard, so with compression preview on the header is never included.

Moving the include out of the #else fixes it:

#ifdef ENABLE_OTLP_COMPRESSION_PREVIEW
...
#endif

#include "opentelemetry/sdk/common/global_log_handler.h"

Measured on your branch, building //ext/src/http/client/curl:http_client_curl:

	before	after
default	builds	builds
--copt=-DENABLE_OTLP_COMPRESSION_PREVIEW	2 errors	builds

And why your local build was green: compression preview is off by default, which is the half that already had the header. A single local configuration cannot see this one. The three IWYU jobs and the two clang-tidy jobs are red for the same reason, since neither tool can run on a translation unit that does not compile, though I would not promise they all clear until CI says so.

Sorry this cost you a round trip. The guard is genuinely easy to miss and it has caught me in this same file before.