Skip to content
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

HTTP client API for exporter developers #6

Closed
maxgolov opened this issue Oct 14, 2019 · 6 comments · Fixed by #370
Closed

HTTP client API for exporter developers #6

maxgolov opened this issue Oct 14, 2019 · 6 comments · Fixed by #370

Comments

@maxgolov
Copy link
Contributor

Different OS may have alternate implementations of HTTP stack:

  • Windows: WinInet, Win 10 (WinRT) HTTP client, WinHttp
  • Linux: libcurl, poco, etc.
  • Mac OS X: libcurl, poco, Apple SDK URLRequest
  • Android: Java HttpRequest, Google HTTP client, OkHTTP
  • iOS: Apple SDK URLRequest
    etc.
  • Chromium HTTP client

An exporter that supports HTTP requires a minimum set of methods:

  • create an instance of HTTP client
  • the client must have ability to create a request with a set of custom headers, method and request body
  • it must provide async execution, with a callback that allows to obtain HTTP response state/result, status code, response headers and response body
  • the callback should, as a minimum, support ability to inspect individual HTTP state machine events (SSL handshake, attempt to connect, sending request, receiving response, etc.), as well as ability to receive the response body / response status code.

Const-correctness aside, common interface structure could define how an exporter developer may design and implement their own HTTP client.

HTTP client does not have intrinsic knowledge of the binary serialization protocol itself (be that gRPC, protobuf, JSON, MSFT Bond, etc.), but can be invoked by exporter implementation to trigger the data upload. The callback associated with each async HTTP request may further handling the retry policy in case if data collection end-point returns something else than 200 OK.

Basic set of interfaces needed to implement the HTTP client:

    class IHttpClient
    {
    public:
        IHttpRequest* CreateRequest();
        void SendRequestAsync(IHttpRequest* request, IHttpResponseCallback* callback);
        void CancelRequestAsync(std::string const& id);
        void CancelAllRequests();
    };

    class IHttpRequest
    {
    public:
        std::string& GetId();
        void SetMethod(std::string const& method);
        void SetUrl(std::string const& url);
        HttpHeaders& GetHeaders();
        void SetBody(std::vector<uint8_t>& body);
        std::vector<uint8_t>& GetBody();
    };

    class HttpHeaders : public std::multimap<std::string, std::string>
    {
    public:
	void set(std::string const& name, std::string const& value);
	void add(std::string const& name, std::string const& value);
	std::string const& get(std::string const& name);
	bool has(std::string const& name);
    }

    class IHttpResponse
    {
    public:
        std::string& GetId();
        HttpResult GetResult();
        unsigned GetStatusCode();
        HttpHeaders& GetHeaders();
        std::vector<uint8_t>& GetBody();
    };

    class IHttpResponseCallback
    {
    public:
        void OnHttpResponse(IHttpResponse* response);
        void OnHttpStateEvent(HttpStateEvent state, ...);
    };

@kyessenov
Copy link

Please consider adding nghttp2-based stacks (https://github.com/envoyproxy/envoy) to the list.

@SamuelMarks
Copy link

0x4b pushed a commit to 0x4b/opentelemetry-cpp that referenced this issue Jul 31, 2020
@jajanet
Copy link
Contributor

jajanet commented Aug 21, 2020

Was this addressed in #217? Maybe we can update this issue to needing to add thread-safety, optimizations, or other measures if so?

@lalitb
Copy link
Member

lalitb commented Aug 25, 2020

#217 brings only http server, we still are missing client part. So either we can modify it to work as thread-safe, optimised http client/server library, or else use header only c++11 cross platform library like https://github.com/yhirose/cpp-httplib. Anyway, I can own this if no one is working on it. We would anyway need it for zipkin support.

@kyessenov
Copy link

I think we want a portable interface but not an implementation. Consider the use case of compiling this library to Wasm and substituting regular POSIX calls with Wasm externs. Anything that talks over the network cannot be directly translated to Wasm.

@SamuelMarks
Copy link

I started working on a common interface last quarter, but it's very much a WiP:

libacquire

C89 License

The core for your package manager, minus the dependency graph components. Features: download, verify, and extract.

By default—for HTTP, HTTPS, and FTP—this uses libfetch on FreeBSD; wininet on Windows; and libcurl everywhere else. Override with -DUSE_LIBCURL or -DUSE_LIBFETCH.

By default—for MD5, SHA256, SHA512—this uses wincrypt on Windows; and OpenSSL everywhere else. Note that on macOS this uses the builtin CommonCrypto/CommonDigest.h header, and on OpenBSD it uses LibreSSL; however in both of these cases it's the OpenSSL API with different headers. Override with -DUSE_OPENSSL.

Mentioning here not so that you follow my implementation, but rather the reverse; the common interface you come up with I'll [likely follow and] implement for Solaris, Linux, BSD, macOS, and Windows.

(though maybe you'll be inspired by the idea of libacquire in defining said interface)

@maxgolov maxgolov linked a pull request Aug 31, 2020 that will close this issue
GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this issue Jun 21, 2024
[EXPORTER and SDK] Additional fixes after NOMINMAX removal on Windows…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants