opa icon indicating copy to clipboard operation
opa copied to clipboard

E2E mode for `opa bench`

Open tsandall opened this issue 2 years ago • 6 comments

Today opa bench measures the performance of the inner-most evaluation call. This is useful because it provides an upper-bound on performance for a particular policy by ignoring things like server overhead. However, in some cases, it's important to know how much additional overhead the server is going to introduce. To solve for this, we can introduce a new mode in the opa bench command (e.g., opa bench --e2e) that runs the benchmark via the HTTP server endpoint.

In terms of implementation, I'd expect bench to instantiate the server handler (e.g., POST v1/data) and then call it inside of the benchmark suite. The output for the e2e mode should include the overall latency as well as the eval latency so that users have an idea of how much time is spent where. The implementation should make sure to support the full server stack including:

  • Authentication and authorization handlers
  • Input deserialization
  • Decision logging (w/ masking applied if relevant policies are present)

TBD:

  • Should we support two e2e modes: one for the server and one for the SDK? E.g., --e2e-server and --e2e-sdk?

tsandall avatar Dec 08 '21 17:12 tsandall

Starting to look into this. Some first thoughts.

Authentication and authorization will be a bit of a hurdle I suspect, as we'd need to allow the user to provide either a token to use for authentication, or provide client certificate configuration if TLS authentication is chosen. We'd obviously need them to configure the server side as well. From what I can tell, we'd need to add at least the following flags to opa bench for that purpose alone:

# server
--authentication
--authorization
--tls-cert-file
--tls-private-key-file
--tls-ca-cert-file

# client
--token
--tls-client-cert-file

In addition to that, I suppose we'll need to add the --config-file flag to allow running with the same config as that used in development/production, and because that is used to configure decision logging. If the bundles are signed, we'd need to add flags to work with that as well 😅 The question is if we should draw a line somewhere as to how much this command should support. Either way, there's going to be a lot of flags that are only applicable when running e2e tests, and a lot that aren't.. I wonder if we would benefit from splitting this up into two distinct commands?

I'm also unsure about the query argument in this context, as having to provide something like data.policy.allow doesn't feel natural when querying the actual HTTP server, or does it? Related to that, should the input.json now need to wrap input too as that's what the HTTP server expects?

As for the actual implementation, how would we gather the actual metrics from the HTTP calls? Is appending ?metrics=true to the request and use the returned metrics good enough, or would we need another way to instrument this?

anderseknert avatar Feb 24 '22 10:02 anderseknert

Authentication and authorization will be a bit of a hurdle I suspect, as we'd need to allow the user to provide either a token to use for authentication, or provide client certificate configuration if TLS authentication is chosen. We'd obviously need them to configure the server side as well. From what I can tell, we'd need to add at least the following flags to opa bench for that purpose alone:

Agreed, we'll need to let people set those if they're interested. In this first cut, I wonder if we could skip this as authn/authz is only required if you're deploying OPA into an untrusted environment.

In addition to that, I suppose we'll need to add the --config-file flag to allow running with the same config as that used in development/production, and because that is used to configure decision logging. If the bundles are signed, we'd need to add flags to work with that as well 😅 The question is if we should draw a line somewhere as to how much this command should support. Either way, there's going to be a lot of flags that are only applicable when running e2e tests, and a lot that aren't.. I wonder if we would benefit from splitting this up into two distinct commands?

I'd be fine with two commands as long as the actual bench implementation is not repeated. It feels like this just requires a different bench setup. As far as signing goes, I'd ignore that for now. We don't provide signature verification in other dev commands today. As far as config goes, I'd include that if needed, but I'm not sure I can think of an example that requires it right now. Perhaps we could add that over time?

I'm also unsure about the query argument in this context, as having to provide something like data.policy.allow doesn't feel natural when querying the actual HTTP server, or does it?

I can see what you're saying... however, I think it would be fine. Someone can basically run:

opa bench --e2e -b bundle.tar.gz -i input.json data.policy.allow

And get back meaningful results. I'm not sure it's much of a problem. We'll want to implement a simple map function that turns data.policy.allow into /v1/data/policy/allow and rejects invalid inputs. That may already exist.

Related to that, should the input.json now need to wrap input too as that's what the HTTP server expects?

I wouldn't do that. If we need to support the full HTTP message body payload we could introduce a different CLI flag.

As for the actual implementation, how would we gather the actual metrics from the HTTP calls? Is appending ?metrics=true to the request and use the returned metrics good enough, or would we need another way to instrument this?

That seems like a good first-cut. The only thing that I could imagine doing differently is passing the metrics object through the context and re-using it in the server. However, that's going to create a different code path that could result in bugs.

tsandall avatar Feb 25 '22 15:02 tsandall

In this first cut, I wonder if we could skip this as authn/authz is only required if you're deploying OPA into an untrusted environment.

I'll hold off with authn/authz until I've got the rest up and working, and we can see then if it's worth doing now or if we should save it until requested.

I'd be fine with two commands as long as the actual bench implementation is not repeated.

Without authn/authz, I think this will be similar enough to fit inside of opa bench, so I'll aim for that in the first iteraton. Thanks!

As far as signing goes, I'd ignore that for now. We'll want to implement a simple map function that turns data.policy.allow into /v1/data/policy/allow and rejects invalid inputs.

If we need to support the full HTTP message body payload we could introduce a different CLI flag.

Not sure what you mean here. We'll obviously need to support the fullt HTTP message body payload, won't we? 🤔

That seems like a good first-cut. The only thing that I could imagine doing differently is passing the metrics object through the context and re-using it in the server. However, that's going to create a different code path that could result in bugs.

Cool, I'll try my options here and we'll see where it leads me.

anderseknert avatar Feb 27 '22 20:02 anderseknert

@anderseknert is this still in progress?

tsandall avatar Apr 04 '22 15:04 tsandall

My work stopped here to pick up some other stuff, and plan was to get back to this as soon as possible.

anderseknert avatar Apr 04 '22 18:04 anderseknert

Initial work for the e2e mode has been completed as part of this. Support for authn/authz and config file is not addressed in these changes.

ashutosh-narkar avatar Jun 22 '22 18:06 ashutosh-narkar

This issue captures the additional updates that can be added to opa bench with e2e mode enabled.

ashutosh-narkar avatar Aug 30 '22 23:08 ashutosh-narkar