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

Proposal: Custom transport for decision/status log plugins #3047

Closed
gshively11 opened this issue Jan 6, 2021 · 6 comments
Closed

Proposal: Custom transport for decision/status log plugins #3047

gshively11 opened this issue Jan 6, 2021 · 6 comments

Comments

@gshively11
Copy link
Contributor

gshively11 commented Jan 6, 2021

Right now, the decision and status log plugins are hardcoded to use HTTP to deliver logs to service endpoints. This is excellent from an interop standpoint. However, we find ourselves in a situation where having a different transport mechanic has the potential to save us significant effort and money. Background:

We run on AWS and have hundreds of accounts that need to own an endpoint capable of receiving decision/status logs from OPA. There is a cost associated with standing up a service capable of receiving logs over HTTP and then shipping them to something like Kinesis for further processing/shipping, and it becomes a significant cost at scale.

One way to avoid that cost is to ship logs directly from OPA to Kinesis, thereby eliminating something like API Gateway -> Lambda -> Kinesis.

Our proposal is to refactor the log/status plugins to support optional, custom transport implementations. There could be some mechanism to register your own transports, similar to custom builtins, and then have the decision/status log plugins select them via a config option. This would solve our problem while keeping the actual custom transport code out of OPA core.

We recognize the option to create our own decision/status plugins, however, we need the majority of the functionality in the existing plugins and only need to tweak the transport, so completely new plugins seems like overkill.

@gshively11
Copy link
Contributor Author

After digging into the code a bit, it looks like the concept of "Services" is tightly coupled with rest.Client. I think ideally, it would be great if Services could specify different underlying implementations, e.g. I want to ship decision logs directly to AWS Kinesis, so I create a custom Kinesis client, register it with OPA under a unique ID, and then reference it under the service configuration with something like service.type=kinesis.

However, changing Services to support something other than rest.Client is a pretty big change and is something I'd prefer not to tackle right now due to time constraints. A much quicker, but less elegant solution would be to add a property to the decisions_logs config that let's me override the meaning of decision_logs.service, something like decision_logs.custom_transport=true. The service key would then be used to retrieve a custom transport implementation (registered globally like custom builtins) and use it inside uploadChunk, instead of rest.Client.

@tsandall it looks like you implemented the decision logs plugin, so I'd love your thoughts on this one. Is there another option I'm missing? Would you be willing to accept the less elegant solution as a temporary bridge until services could be refactored (assuming that's even something you want to support?)

@gshively11
Copy link
Contributor Author

@tsandall
Copy link
Member

tsandall commented Apr 13, 2021

I'm not sure about supporting arbitrary transports for the service clients. I could imagine lots of subtlety that would end up leaking through the abstraction.

We already support custom decision log implementations that can replace the in-built HTTP logger. There's an example implementation of that here: https://www.openpolicyagent.org/docs/latest/extensions/#custom-plugins-for-opa-runtime. The plugin can be enabled via configuration. With this interface you would just implement the Log() function and send events to Kinesis. We've had people use this in the past to integrate w/ Kafka.

Adding support for custom status implementations that replace the in-built HTTP transport like we do for decision logs would be relatively easy. We would just add the same kind of extension machinery to the Status plugin and then call out here.

So, my recommendation in this case would be:

  • Add a custom decision logger that uses Kinesis w/ the existing decision_logs.plugin framework. You can implement buffering if you want. There's nothing sophisticated there.

  • Extend OPA to support custom status reports in the same way we support custom decision loggers (a PR would be very welcome there.)

@gshively11
Copy link
Contributor Author

gshively11 commented Apr 13, 2021

Wow, I totally overlooked the decision_logs.plugin feature. It's a bummer we can't hook into the existing code for buffering/encoding, but copying it isn't a huge deal I guess. I'll get started on a PR to add support for the status logger. Thanks for pointing me in the right direction!

tsandall pushed a commit that referenced this issue Apr 16, 2021
Added support for plugins in status plugin, similar to the pattern
employed in the decision logs plugin. By setting `status.plugin`
in configuration, you can override how the status plugin sends status
updates.

Related to #3047

Signed-off-by: Grant Shively <[email protected]>
@anderseknert
Copy link
Member

Anything more to do here, @gshively11?

@gshively11
Copy link
Contributor Author

Nope, closing, thanks for the reminder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants