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

Development monitoring #2045

Merged
merged 52 commits into from
Feb 12, 2024
Merged

Development monitoring #2045

merged 52 commits into from
Feb 12, 2024

Conversation

0oM4R
Copy link
Contributor

@0oM4R 0oM4R commented Jan 18, 2024

Description

This package provides service monitoring; currently, check if the service is alive

ServiceMonitor class

  • checkServiceAliveness : accepts array of IServiceAliveness and
    Creates an instance of ServiceMonitor.

    • services - An array of services to monitor.
    • interval - The interval, in minutes, between monitoring checks (default is 2 minutes)
    • retries - The number of retries in case a service is determined to be down (default is 2 retries).
    • retryInterval - The interval, in seconds, between retries (default is 2 seconds).
      This class have three public functions
    1. monitorService : Monitors the services at a regular interval and returns a function to exit and disconnect the monitoring.
    2. pingService: check the liveness of the services and disconnect
    3. disconnect: disconnect the services that have disconnect handler

ILivenessChecker Interface

each instance of the classes that implements ILivenessChecker interface have its own logic to create the connection and
check the service's aliveness

Evenets

we expose an instance of MonitorEventEmitter that make emitting events more easier

Screenshot from 2024-01-22 16-39-21

Changes

added new package monitoring

Related Issues

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

@@ -0,0 +1,56 @@
import { events } from "../helpers/events";
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't uderstand why we didn't do it like this

class ServiceMonitor {

constructor(services: Array<IService>, interval) {
    ...
}
private checkLivenessOnce {

}
private checkHealthOnce {

}
monitor() {
    withInterval do your checkLivenessOnce and checkHealthOnce 
}

}

@0oM4R 0oM4R marked this pull request as draft January 28, 2024 16:09
@0oM4R 0oM4R marked this pull request as ready for review January 30, 2024 10:05
Copy link
Contributor

@mohamedamer453 mohamedamer453 left a comment

Choose a reason for hiding this comment

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

  • Graphql module is not exported in index.ts and thus it cannot be used.

    image

new GridProxyMonitor("<FakeURL>"),
new TFChainMonitor("wss://tfchain.dev.grid.tf/ws", "mnemonic", "sr25519"),
new RMBMonitor("wss://relay.dev.grid.tf", "wss://tfchain.dev.grid.tf/ws", "mnemonic", "sr25519"),
];
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be great if a check for graphql was added to the example.

Copy link
Contributor

@AhmedHanafy725 AhmedHanafy725 left a comment

Choose a reason for hiding this comment

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

I am afraid about disconnecting the clients cause it's only one static instance per url on the client, so if it will be used in the dashboard it might disconnect the client while deploying something.

@@ -0,0 +1,32 @@
{
"name": "monitoring",
Copy link
Contributor

Choose a reason for hiding this comment

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

any package's name added to this sdk should start with @threefold/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i notice the license also is missing, which license should we use ?

},
"dependencies": {
"axios": "^0.27.2",
"@polkadot/api": "^8.9.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to add this package if you are using tfchain client

private tfClient: TFClient;
constructor(tfChainUrl: string, mnemonic: string, keypairType: KeypairType) {
this.url = tfChainUrl;
this.tfClient = new TFClient({ url: this.url, mnemonicOrSecret: mnemonic, keypairType: keypairType });
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we using TFClient here? it requires a mnemonic and a keypair type. Instead, we can use the queryClient which requires only the url

- remove unnecessary package
- change package name to include @threefold/
- use queryClient in tfchain and update the example
@0oM4R 0oM4R requested a review from AhmedHanafy725 February 12, 2024 11:41
@AhmedHanafy725 AhmedHanafy725 merged commit 2e6d78e into development Feb 12, 2024
2 checks passed
@AhmedHanafy725 AhmedHanafy725 deleted the development_monitoring branch February 12, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants