Command process logs should display the error from process

NicolasMahe commented 4 years ago

The error from the processes are printed in the engine's log with module=orchestrator. The command process:logs should fetch them and display it to the user.

TODO:

[ ] Listen to the logs of docker (like the daemon:logs command)
[ ] Parse logs from text to object
[ ] Filter logs that match the module=orchestrator
[ ] Display the logs in the process:logs command

gitcoinbot commented 4 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 5500.0 MESG attached to it as part of the mesg-foundation fund.

If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
- Want to chip in? Add your own contribution here.
- Questions? Checkout Gitcoin Help or the Gitcoin Slack
- $69,232.36 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot commented 4 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 1 day, 18 hours ago. Please review their action plans below:

1) uivlis has been approved to start work.

I will update the process:logs command accordingly. 2) agbilotia1998 has been approved to start work.

1.) Install the project. 2.) See the logs 3.) Parse the logs 4.) Display in process:logs

Learn more on the Gitcoin Issue Details page.

gitcoinbot commented 4 years ago

@agbilotia1998 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] reminder (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

gitcoinbot commented 4 years ago

@agbilotia1998 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] reminder (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

uivlis commented 4 years ago

Hey @antho1404, although I'm not exactly sure this would solve the issue, there is a bug caused by the usage of nodeKey instead of stepID, a breaking change in @mesg/api. It seems that inside the process:logs command source file you use the new version of the IExecution interface, but import the old one from @mesg/api/lib/execution. Releasing a new version of the @mesg/api with the updated api/lib/typedef/execution.ts file might be a solution. Another one would be importing the file from the project, locally. And, finally, I could replace nodeKey back to stepId, what do you suggest?

antho1404 commented 4 years ago

I'm not sure to understand, the CLI should be updated with the latest @mesg/api version, just make sure to restart the daemon if you did an update of the CLI.

The issue to fix here is that we might have issues during the runtime of a process and we should be able to display the errors from the daemon related to the orchestrator.

Right now the daemon is displaying these errors in the logs that can be accessed with the command daemon:logs.

We need to display the logs of the daemon that match the module=orchestrator in the process:log command.

This way anytime an error occurs in a process, the CLI will be able to display this error immediately instead of having the use dig into the daemon logs.

Is this clearer?

gitcoinbot commented 4 years ago

@agbilotia1998 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] reminder (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

agbilotia1998 commented 4 years ago

Hi @antho1404 as per my understanding, the errors related to a process are currently displayed with daemon:logs but the same should be shown in process:logs PROCESS_HASH command of cli. To generate and test the case, I am using this process file

key: erc20-notification
steps:
  - type: trigger
    instance:
      src: https://github.com/mesg-foundation/service-ethereum-erc20
    eventKey: transfer

and compiling it to get the definition and then use the definition in create command. But probably this trigger doesn't have any error so mesg-cli daemon:logs| grep module=orchestrator does not output anything. Can you please let me know if I am going in right direction or there is some issue. Also please let me know any example process file where I can catch the errors.

antho1404 commented 4 years ago

Here is an example of a process file that has runtime error.

name: process-with-error
steps:
  - type: trigger
    instance:
      src: https://github.com/mesg-foundation/service-emit-event-interval
    eventKey: every_5_seconds
  - type: task
    instance:
      src: https://github.com/mesg-foundation/service-js-function
    taskKey: execute
    inputs:
      foo: bar

Inputs for the task are invalid here and every 5 seconds, the orchestrator will try to create a new execution with invalid inputs that will result in an error that needs to be displayed in the process:log command (and that is now only displayed in the mesg-cli daemon:logs | grep module=orchestrator command)

uivlis commented 4 years ago

Hey @antho1404,

I'm not sure to understand, the CLI should be updated with the latest @mesg/api version, just make sure to restart the daemon if you did an update of the CLI.

Yes, that is true, the CLI is updated with latest @mesg/api version. However, what I was saying is that this very latest version somehow does not contain the latest breaking change in the IExecution interface, specifically, it still has a stepID instead of a newer nodeKey. Again, I reapeat from my latest comment:

Releasing a new version of the @mesg/api with the updated api/lib/typedef/execution.ts file might be a solution. Another one would be importing the file from the project, locally. And, finally, I could replace nodeKey back to stepId, what do you suggest?

antho1404 commented 4 years ago

@uivlis I looked at the version on github of @mesg/api@0.2.0 and the type is valid. Also did a fresh install of the CLI and the installed api package is valid (with the right type). None of these actually have the stepID that doesn't exist anymore.

Could you try to re-install it or maybe let me know how you installed it because you shouldn't have a stepID and should run on the following versions

@mesg/cli: 0.2.0
@mesg/api: 0.2.0

gitcoinbot commented 4 years ago

@agbilotia1998 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] reminder (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

uivlis commented 4 years ago

@antho1404 First, my apologies for the delay in answering, been busy with the daily work. Second, my apologies, again, because you were right. After a clean clone of the project, things went neat (I think I had an older fork of the project). I will proceed further with the task at hand.

uivlis commented 4 years ago

Hey @antho1404 I managed to display inside process:logs the errors with module=orchestrator using the example process you provided above. However, I used the default name for the docker service: engine. Is there a way to get the name of the docker service from process:logs, knowing that the docker var is private inside docker-command.ts, or, perhaps, I could add as a parameter to the process:logs command the name of the docker service running the engine?

gitcoinbot commented 4 years ago

@agbilotia1998 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] reminder (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

antho1404 commented 4 years ago

Hi @uivlis, the process:log command inherits of the root-command but you could inherit from the docker-command this way you will have the flag from it and also access the current functions like the listServices. Also, could you publish your work in WIP like that I can help you better.

gitcoinbot commented 4 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 5500.0 MESG has been submitted by:

@uivlis

@mesg-bot please take a look at the submitted work:

PR by @uivlis

Learn more on the Gitcoin Issue Details page
Want to chip in? Add your own contribution here.
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$79,597.39 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot commented 4 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 5500.0 MESG attached to this issue has been approved & issued to @uivlis.

Learn more on the Gitcoin Issue Details page
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$80,947.52 more funded OSS Work available on the Gitcoin Issue Explorer

mesg-foundation / js-sdk

Command process logs should display the error from process #140