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

feat: Add event notifications #410

Merged
merged 25 commits into from
Nov 7, 2023

Conversation

bgins
Copy link
Contributor

@bgins bgins commented Nov 6, 2023

Description

This pull request implements the following changes:

  • Add event notifications
  • Add emit_event notification utility function
  • Add emit_receipt notification utility function
  • Move receipt notifications to event handler notifications
  • Add ConnnectionEstablished, ConnnectionClosed, ListeningOn, OutgoingConnectionError, and IncomingConnectionError network notifications
  • Test JSON event notification bytes roundtrip
  • Test JSON event notification string roundtrip
  • Integration test connection notifications with two Homestar nodes

Link to issue

Closes #407

Type of change

  • New feature (non-breaking change that adds functionality)

Test plan (required)

We have included unit tests to check roundtrip conversions between JSON bytes and strings. In addition, we have included an integration test that subscribes and listens for connection messages between Homestar nodes.

Includes:
  - re-purposing of feature flags
    * metrics is always a thing (on)
    * monitoring is the gated feature
    * websocket-server flag is gone, we only gate push notifications
  - jsonrpc setup and rpc method register
  - prometheus exposition format to json parser
  - Added todo around split networking config

Notes:
  - will not merge this in until example app has been restored with the client changes
@bgins bgins added networking Features, functionality involving networking dx Developer experience applications and improvements labels Nov 6, 2023
@bgins bgins force-pushed the bgins/libp2p-events-over-websocket branch from cb7e9c1 to 809bea6 Compare November 6, 2023 16:44
@bgins bgins force-pushed the bgins/libp2p-events-over-websocket branch from 809bea6 to 4830813 Compare November 6, 2023 17:34
@bgins bgins self-assigned this Nov 6, 2023
@bgins bgins marked this pull request as ready for review November 6, 2023 19:11
@bgins bgins requested a review from a team as a code owner November 6, 2023 19:11
const TIMESTAMP_KEY: &str = "timestamp";

/// Send notification as bytes.
pub(crate) fn send(
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: I wouldn't call this send, as that's typically tied to a channel itself. Anything better? Maybe emit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, sounds good. Called it emit_event so we can have a separate emit_receipt.

homestar-runtime/tests/network.rs Show resolved Hide resolved
Base automatically changed from zl/jsonrpc to main November 6, 2023 23:30
@bgins bgins changed the base branch from main to zl/jsonrpc November 6, 2023 23:44
@bgins bgins force-pushed the bgins/libp2p-events-over-websocket branch from f2e8364 to 80f0eec Compare November 7, 2023 18:59
@bgins bgins force-pushed the bgins/libp2p-events-over-websocket branch from 80f0eec to 794d3c7 Compare November 7, 2023 19:36
@zeeshanlakhani zeeshanlakhani merged commit ce815d7 into zl/jsonrpc Nov 7, 2023
21 of 23 checks passed
@zeeshanlakhani zeeshanlakhani deleted the bgins/libp2p-events-over-websocket branch November 7, 2023 19:54
zeeshanlakhani added a commit that referenced this pull request Nov 8, 2023
# Description

This pull request implements the following changes:

- [x] Add event notifications
- [x] Add `emit_event` notification utility function
- [x] Add `emit_receipt` notification utility function
- [x] Move receipt notifications to event handler notifications
- [x] Add `ConnnectionEstablished`, `ConnnectionClosed`, `ListeningOn`,
`OutgoingConnectionError`, and `IncomingConnectionError` network
notifications
- [x] Test JSON event notification bytes roundtrip
- [x] Test JSON event notification string roundtrip
- [x] Integration test connection notifications with two Homestar nodes

## Link to issue

Closes #407

## Type of change

- [x] New feature (non-breaking change that adds functionality)

## Test plan (required)

We have included unit tests to check roundtrip conversions between JSON
bytes and strings. In addition, we have included an integration test
that subscribes and listens for connection messages between Homestar
nodes.

---------

Co-authored-by: Zeeshan Lakhani <zeeshan.lakhani@gmail.com>
hugomrdias pushed a commit that referenced this pull request Nov 14, 2023
# Description

This pull request implements the following changes:

- [x] Add event notifications
- [x] Add `emit_event` notification utility function
- [x] Add `emit_receipt` notification utility function
- [x] Move receipt notifications to event handler notifications
- [x] Add `ConnnectionEstablished`, `ConnnectionClosed`, `ListeningOn`,
`OutgoingConnectionError`, and `IncomingConnectionError` network
notifications
- [x] Test JSON event notification bytes roundtrip
- [x] Test JSON event notification string roundtrip
- [x] Integration test connection notifications with two Homestar nodes

## Link to issue

Closes #407

## Type of change

- [x] New feature (non-breaking change that adds functionality)

## Test plan (required)

We have included unit tests to check roundtrip conversions between JSON
bytes and strings. In addition, we have included an integration test
that subscribes and listens for connection messages between Homestar
nodes.

---------

Co-authored-by: Zeeshan Lakhani <zeeshan.lakhani@gmail.com>
@bgins bgins mentioned this pull request Nov 16, 2023
20 tasks
zeeshanlakhani added a commit that referenced this pull request Nov 29, 2023
# Description

This pull request implements the following changes:

- [x] Add event notifications
- [x] Add `emit_event` notification utility function
- [x] Add `emit_receipt` notification utility function
- [x] Move receipt notifications to event handler notifications
- [x] Add `ConnnectionEstablished`, `ConnnectionClosed`, `ListeningOn`,
`OutgoingConnectionError`, and `IncomingConnectionError` network
notifications
- [x] Test JSON event notification bytes roundtrip
- [x] Test JSON event notification string roundtrip
- [x] Integration test connection notifications with two Homestar nodes

## Link to issue

Closes #407

## Type of change

- [x] New feature (non-breaking change that adds functionality)

## Test plan (required)

We have included unit tests to check roundtrip conversions between JSON
bytes and strings. In addition, we have included an integration test
that subscribes and listens for connection messages between Homestar
nodes.

---------

Co-authored-by: Zeeshan Lakhani <zeeshan.lakhani@gmail.com>
bgins added a commit that referenced this pull request Nov 29, 2023
## Description

Includes:
  - re-purposing of feature flags 
    * metrics is always a thing (on) 
    * monitoring is the gated feature 
    * The websocket-server flag is gone, we only gate push notifications
  - JSON-RPC setup and RPC method register
  - Prometheus exposition format to JSON parser

Other features and other fixes:

- [x] e2e testing of run workflow
- [x] #407
- [x] #410
- [x] #418
- [x] #424
- [x] #354 
- [x] #409
- [x] #425 
- [x] #426
- [x] #429 
- [x] #433
- [x] #435
- [x] #421
- [x] #436
- [x] #437
- [x] #444
- [x] #438
- [x] #390
- [x] #451 
- [x] #456

---------

Signed-off-by: Brian Ginsburg <7957636+bgins@users.noreply.github.com>
Signed-off-by: Zeeshan Lakhani <zeeshan.lakhani@gmail.com>
Co-authored-by: Brian Ginsburg <7957636+bgins@users.noreply.github.com>
Co-authored-by: Hugo Dias <hugomrdias@gmail.com>
bgins added a commit that referenced this pull request Nov 29, 2023
Includes:
  - re-purposing of feature flags
    * metrics is always a thing (on)
    * monitoring is the gated feature
    * The websocket-server flag is gone, we only gate push notifications
  - JSON-RPC setup and RPC method register
  - Prometheus exposition format to JSON parser

Other features and other fixes:

- [x] e2e testing of run workflow
- [x] #407
- [x] #410
- [x] #418
- [x] #424
- [x] #354
- [x] #409
- [x] #425
- [x] #426
- [x] #429
- [x] #433
- [x] #435
- [x] #421
- [x] #436
- [x] #437
- [x] #444
- [x] #438
- [x] #390
- [x] #451
- [x] #456

---------

Signed-off-by: Brian Ginsburg <7957636+bgins@users.noreply.github.com>
Signed-off-by: Zeeshan Lakhani <zeeshan.lakhani@gmail.com>
Co-authored-by: Brian Ginsburg <7957636+bgins@users.noreply.github.com>
Co-authored-by: Hugo Dias <hugomrdias@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Developer experience applications and improvements networking Features, functionality involving networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants