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

Interceptor #67

Closed
wants to merge 18 commits into from
Closed

Interceptor #67

wants to merge 18 commits into from

Conversation

AlexAndrei98
Copy link
Contributor

@AlexAndrei98 AlexAndrei98 commented Jul 7, 2023

I wanted to create a new class that allows to push events that are happening inside the cloud agent to other backend processes.
It is similar to the notifier class but it is used to "notify" other web services.
I would like to notify the web services of the following event:
simple aid inception
simple aid rotation
simple aid delegation
multisig aid inception
multisig aid rotation
multisig aid delegation
multisig request
multisig signatures
acdc issuance
acdc revocation
acdc presentation

@pfeairheller
Copy link
Member

I agree this is good functionality to add. When we built kiwiing and the Keep we struggled with the need for "notifications" of agent activity (transient events that are only important if a client was currently running) and user notifications that needed to persist until the user acknowledged and dealt with them (for example, a credential was issued, a presentation was requested, a multisig group was proposed). I'd like us to make sure we don't conflate these two use cases in KERIA.

This functionality seems like a separate type of notification all together. Notifying some process controlled by the host of the KERIA service of all activity. Seems like KERIA doesn't need to worry about persistence of these notifications and only fire and forget if and only if someone is currently listening (no retries, etc).

Instead of configuring headers to be passed how about we just sign every message from the Agent that generates the message. That should be all the information a receiving process would need to authenticate the request and handle it.

Also, this feels a lot like instrumentation and metric processing for a KERIA service so should we consider using something like Prometheus to generate events and allow consumers to use all the tools available from that ecosystem to consume, process and present this data? That will make signing the data a little more challenging (and maybe not needed) so worth further discussion.

Regardless of path, we'll need to the tests to pass... 😉

@AlexAndrei98
Copy link
Contributor Author

AlexAndrei98 commented Jul 7, 2023

Adding Prometheus for the endpoints I believe is the easiest path to gather performance metrics! However, like you said, the interceptor is interested in pushing low level Keri events to an internal queue.

I think we can add signify headers but some backends still work with bearer tokens so that's my reasoning on why the headers would be needed.

Any recommendations for adding test for this? The way I am testing locally is by having another api running waiting for a post and printing them body out.

@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #67 (443e139) into main (afa1c9b) will decrease coverage by 0.37%.
The diff coverage is 56.86%.

@@            Coverage Diff             @@
##             main      #67      +/-   ##
==========================================
- Coverage   86.92%   86.56%   -0.37%     
==========================================
  Files          29       29              
  Lines        4491     4534      +43     
==========================================
+ Hits         3904     3925      +21     
- Misses        587      609      +22     
Impacted Files Coverage Δ
tests/app/test_agenting.py 96.56% <ø> (ø)
src/keria/app/agenting.py 78.54% <51.11%> (-2.75%) ⬇️
src/keria/app/aiding.py 71.30% <100.00%> (+0.29%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AlexAndrei98
Copy link
Contributor Author

@pfeairheller What are some good places to start putting some interceptor? At first I was thinking in the endpoint definition but then I realized that some async operations you would only find out if the client pulls so I started looking through the escrow porcesses like
here, or here and here . I see that the messages and attachments(like signagures) are bytearrays, can I just go ahead and base64 encode them or parse each of them and create a pretty() version of the object?

@@ -308,15 +312,15 @@ def __init__(self, hby, rgy, agentHab, agency, caid, **opts):
vry=self.verifier)

doers.extend([
Initer(agentHab=agentHab, caid=caid),
Initer(agentHab=agentHab, caid=caid, metrics = self.agency.metrics),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the list of doers, you have to include self.interceptor in this list to get the recur method to be called as a background process.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I not put it when I Initialize the agency super(Agency, self).__init__(doers=[self.interceptor], always=True) on Line 132 instead of having a logger per agent, I was thinking that since we have a queue per agency instead of agent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfeairheller I also tried passing self.agency.interceptor but it still not running. Does Interceptor need to be Just a Doer Since Agent is a DoDoer?

@m00sey m00sey marked this pull request as draft August 3, 2023 14:13
@pfeairheller
Copy link
Member

Closing this old PR due to lack of activity.

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

Successfully merging this pull request may close these issues.

2 participants