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: introduce event hooks #1653

Open
marwan-at-work opened this issue Jul 26, 2020 · 2 comments · May be fixed by #1664
Open

Proposal: introduce event hooks #1653

marwan-at-work opened this issue Jul 26, 2020 · 2 comments · May be fixed by #1664
Labels
proposal A proposal for discussion and possibly a vote

Comments

@marwan-at-work
Copy link
Contributor

Motivation

Athens makes a number of decisions in a request's lifecycle. An administrator might want to hook into some of the functionality for automation or other reasons.

We already have one validation hook which can be used to block incoming requests.

However, there are more events that an Athens admin might be interested in automating:

  1. An event triggered when a new module is stored so they can do their own indexing.

  2. An event triggered when a module is blocked so they can know who is still requesting a certain module.

  3. An event triggered when a checksum request is made to a NoSumPattern so they know the rate of misconfiguration amongst their employees.

Proposal

I propose that we add an EventHook that an Athens admin can use to be able to hook into the Athens lifecycle.

The EventHook is a URL that Athens will make POST requests to for various events. We can start small and expand based on need for these events.

An Athens admin should be able to know (whether through documentation, code, or both) what events will be sent to their URL and what the shape of the data will look like.

Unlike the ValidationHook, the EventHook will not fail if the POST request returned an error. However, it should wait on the request to finish and log if the response is not 200.

Note that since the EventHook is blocking, it is the implementer's responsibility to make sure the hook does not cause time outs. For example, the hook server can accept requests and forward to a message broker like RabbitMQ or Kafka to process requests asynchronously.

Implementation

However the implementation might be, we should be able to provide an easy way for users to know what events are available and what the payload looks like.

Twirp and gRPC are natural fits here where we can automate client/server communication as well as documentation. However, they require additional build steps and tooling which could complicate the development process.

For simplicity we can use pure "net/http" and maintain our own documentation and client server code. I like this idea though it's a bit more work to maintain.

However, in both cases the user will just be able to import a package and get started so both will provide a good end user experience.

One thing to be aware of is that our semantic versioning so far has been based on the "server protocol" and not our code. Therefore, if we expect our users to import a library to use a hook, then our semver should take into account code-compatibility as well. One thing we can do is put the code in another repo to keep Athens semver based server protocol only. Or we can put the code in Athens and keep the semver promise on code optional.

@marwan-at-work marwan-at-work added the proposal A proposal for discussion and possibly a vote label Jul 26, 2020
@fedepaol
Copy link
Member

fedepaol commented Aug 3, 2020

Looks interesting!

Some thoughts:

  • even if it's the implementer's responsability not to block, I'd avoid relying on their good will / absence of bug for making sure athens is working. I think a (maybe long) timeout is better with that regard. Not sure if the originating call should fail or pass. Maybe we can have that as an option?

  • Rest vs grpc: if I understood correctly, you are proposing to implement either a rest hook plus a writing a "client library" (which in the end is a server library). If we go with this, we'll either have to have a different library per language (or just support go), whereas grpc will generate the client library for us. Another good point for grpc, is that we could have the client connect, and use the stream api to push events to the client.

@marwan-at-work
Copy link
Contributor Author

I think a (maybe long) timeout is better with that regard

Putting in a sane timeout is definitely a good idea. I hope we can make it long enough that it doesn't need to be configurable just to keep our config from getting any larger

If we go with this, we'll either have to have a different library per language (or just support go), whereas grpc will generate the client library for us

In the case of Athens, I'm comfortable keeping the client Go only since anyone interacting with Athens is probably using Go. Even if someone needed to use a different language, the EventHook should be incredibly simple: it will only be 1 endpoint with different payloads for each hook event. And most payloads will look exactly the same carrying the module@version and the event name.

Another good point for grpc, is that we could have the client connect, and use the stream api to push events to the client.

That does sound pretty cool, and might be a good performance boost. But I'm hoping to keep things pretty simple and the event hook server can always be a side car where making the http request should not be a huge overhead.

@marwan-at-work marwan-at-work linked a pull request Aug 11, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal A proposal for discussion and possibly a vote
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants