generated clients sketch

[ahl]

This is an example of a trivial generated client. I'm looking for early feedback. Some things I'd note:

• progenitor forces consumers to pull in a few dependencies. Some of them seem fine (e.g. it can require chrono and uuid) but I think it would be nice to wrap these up more elegantly

• I don't love the use of anyhow here. I think the client could be more precise about its errors

• The use of reqwest seems permissible, but I'd love to hear feedback there

• Note that agent.request_share becomes agent.api_request_share because that's what we call the actual endpoint function. I think we should just drop prefixes like "api" in actual endpoint functions, but we could alternatively 1. add an option to the endpoint macro to change the operationId or 2. maybe have some way to tell dropshot to drop prefixes generally, but I have no idea what that would look like.

• We lose logging. This seems problematic.

[david-crespo]

Very cool. On 4, I like changing the function name as well. We can wait and see if we have enough places we'd want to override the operation ID before adding that.

[jclulow]

The anyhow use in Progenitor is definitely just because I was lazy at the time. We should replace it with a thiserror thing that at a minimum allows one to extract the status code from an error.

[andrewjstone]

Slick

[smklein]

I'm echoing other comments, but:

• Removing API prefixes from endpoints sounds good - I don't think an ability for progenitor to "rename them" in the client bindings is at all necessary.
• Anyhow -> thiserror in libs is always good.

On the note of logging - couldn't we possibly generate this too? I know @bnaecker has been working on integrating dtrace probes into the server endpoints; I wonder if there's a way we could have some automated goop such that on each {client, server} invocation, we call {dtrace provider functions, slog logging functions}.

[ahl]

On the note of logging - couldn't we possibly generate this too? I know @bnaecker has been working on integrating dtrace probes into the server endpoints; I wonder if there's a way we could have some automated goop such that on each {client, server} invocation, we call {dtrace provider functions, slog logging functions}.

I've been thinking more about the code we generate, how consumers might customize that code generation, and how consumers might customize at the more generic HTTP client layer rather than at the API. For example, this uses reqwest today, but one could imagine a generated client using some trait instead. We could potentially put logging and tracing at that more generic layer rather than in the generated API code.

[davepacheco]

This all sounds good.

Regarding using reqwest as a client: @ahl and I discussed this offline but I figured I'd put this here, too. We're eventually going to want some sophisticated management of the underlying TCP connections. Most out-of-the-box clients (including reqwest, as far as I can tell) include a very simple pool of connections that I think will fall apart for us at scale. More sophisticated clients look more like what I described here. I think we should assume we're going to need the sophisticated thing eventually, though maybe not for v1.

This could influence the interface that these clients provide. But @ahl's and my conclusion is that the change described here is certainly a step in the right direction, and we can evolve it as needed when we figure out how we want to expose better connection management. Unless folks disagree, I think we should probably punt on that part for now.

[ahl]

I added support for logging. @davepacheco and @jclulow I'd particularly appreciate your feedback:

https://github.com/oxidecomputer/omicron/blob/1d36fcfd85a148d257247a1f6e55506250482e33/sled-agent/src/bootstrap/client.rs

generate_api!(
    "../openapi/bootstrap-agent.json",
    slog::Logger,
    |log: &slog::Logger, request: &reqwest::Request| {
        debug!(log, "client request";
            "method" => %request.method(),
            "uri" => %request.url(),
            "body" => ?&request.body(),
        );
    },
    |log: &slog::Logger, result: &Result<_, _>| {
        debug!(log, "client response"; "result" => ?result);
    },
);

In short, you can optionally specify a type, a pre-request hook, and a post-request hook. This would let us do USDT probes pretty nicely as well.

[davepacheco]

Looks pretty cool!