Skip to content

Fix #856 - Add debug task #859

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix #856 - Add debug task #859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbafd0b
[Fix_#856] Add debug task
fjtirado May 27, 2024
de2f244
[Fix_#856] Ricardos comments
fjtirado May 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions ctk/features/debug.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
Feature: Debug Task
As an implementer of the workflow DSL
I want to ensure that debug tasks can be executed within the workflow
So that my implementation conforms to the expected behaviour

# Tests Debug with literal constant
Scenario: Debug with literal constant
Given a workflow with definition:
"""yaml
document:
dsl: 1.0.0-alpha1
namespace: default
name: debug
do:
printMessage:
debug: Hello world!
"""
And given the workflow input is:
"""yaml
"""
When the workflow is executed
Then the workflow should complete

# Tests Debug with expression
Scenario: Debug with expression
Given a workflow with definition:
"""yaml
document:
dsl: 1.0.0-alpha1
namespace: default
name: debug
do:
printMessage:
debug: ${Hello \(.name)!}
"""
And given the workflow input is:
"""yaml
name: John
"""
When the workflow is executed
Then the workflow should complete with output:
"""yaml
name: John
"""

17 changes: 17 additions & 0 deletions dsl-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- [Set](#set)
- [Try](#try)
- [Wait](#wait)
- [Debug](#debug)
Copy link
Member

Choose a reason for hiding this comment

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

I'm honestly wondering if this is a task or a call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Typically you will call debug within a composite task before and after invoking another task. so I think is easier if debug is a task itself.

Copy link
Member

Choose a reason for hiding this comment

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

It would probably be even better to call it via an extension.

Copy link
Member

@cdavernas cdavernas May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with Jb, imho it should not be a task nor a call.

Reasons are:

  1. It's irrelevant to authors of the workflow (but might be to implementers): there is no effect on the workflow
  2. There is no way to unify behavior accross runtimes (logs will have different format and features. Runtimes might enforce structured logs, too)
  3. You can achieve the same using shell+echo or, even better, outputting logs to a specific file
  4. Runtime specific: aside from 2, runtimes (ex: Synapse) might not want to offer the user the ability to write to logs. If It's up to the runtimes to get logs from Somewhere and do smtg with it, it does not belong to the DSL imo.

Copy link
Member

@cdavernas cdavernas May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

On a side note, the use case you described to @ricardozanini is covered by the sample logging extension (see dsl-reference.md)

Copy link
Member

@cdavernas cdavernas May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Bottom line is that I would instead recommend that "log" task to become the first one in the functions gallery. So instead of "forcing" runtimes to implement it, we create a 'run' task which uses shell to echo or output a potentially formatted message.
Wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

See https://github.com/serverlessworkflow/specification/blob/main/dsl.md#creating-a-custom-function

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JBBianchi I want to avoid different runtimes implementing logging in different ways.
In your extension example, you are invoking an URI to collect logging, but this task is intended to be able to call the runtime library (a logging library in Java, fmt package in go, Ilogger in C#) that actually logs in the console without losing portability (if any runtime implemenation support logging, the definition file is fully portable)

Copy link
Collaborator Author

@fjtirado fjtirado May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@cdavernas
Sample logging extension is an example of how to collect logs, it does not cover the case where you want to log a message to the standard console. I think using extension mechanism is superb if we have the log task, but without the log task, how do users logs to the console?
I think forcing people to write logs though a call to a shell (which actually assume the workflow is running with access to a shell, which is not true if we are using windows SO) is worse that forcing runtime to implement a log task (besides the performance difference of calling to the shell to write a message compared with the performance of a system call to print in the standard output)
As a side note, as I mentioned in the issue, this is not my personal preference, I actually have real feedback from users ot the current spec. They are using custom sysout function (sonataflow) a lot. This makes workflows not portable. Im trying to avoid losing portability and losing functionality (I cannot tell that users that they cannot log to the console any longer)

Copy link
Collaborator Author

@fjtirado fjtirado May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

2. There is no way to unify behavior accross runtimes (logs will have different format and features. Runtimes might enforce structured logs, too)

Any programming language which is used to implement the spec will have means to write to standar output (if there is not standard output, nothing is written), so implementation is trivial,
The message is just the string, the format is up to the implementation (which is fine as far as the message is part of the log). Implementations might provide extra configuration to format the log message (up to the implementation) The point is that the workflow definition will still be portable and when executed in any implementation it will be provide some kind of feedback though the standard console (if available). Thats all.

+ [Flow Directive](#flow-directive)
+ [External Resource](#external-resource)
+ [Authentication](#authentication)
Expand Down Expand Up @@ -973,6 +974,22 @@ do:
seconds: 10
```

#### Debug

Allows workflows to print information to standart output. It accepts a string that can be a literal or an expr

##### Examples
```yaml
document:
dsl: '1.0.0-alpha1'
namespace: test
name: sample-workflow
version: '0.1.0'
do:
printMessage:
debug: Hello world!
```

### Flow Directive

Flow Directives are commands within a workflow that dictate its progression.
Expand Down
3 changes: 2 additions & 1 deletion dsl.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ The Serverless Workflow DSL defines several default [task](dsl-reference.md#task
- [Set](dsl-reference.md#set), used to dynamically set or update the [workflow](dsl-reference.md#workflow)'s data during the its execution.
- [Try](dsl-reference.md#try), used to attempt executing a specified [task](dsl-reference.md#task), and to handle any resulting [errors](dsl-reference.md#error) gracefully, allowing the [workflow](dsl-reference.md#workflow) to continue without interruption.
- [Wait](dsl-reference.md#wait), used to pause or wait for a specified duration before proceeding to the next task.
- [Debug](dsl-reference.md#debug), used to print information to standard output

To ensure they conform to the DSL, runtimes **should** pass all the feature conformance test scenarii defined in the [ctk](ctk/README.md).
To ensure they conform to the DSL, runtimes **should** pass all the feature conformance test scenario defined in the [ctk](ctk/README.md).

##### Secret

Expand Down
7 changes: 7 additions & 0 deletions schema/workflow.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ $defs:
- $ref: '#/$defs/switchTask'
- $ref: '#/$defs/tryTask'
- $ref: '#/$defs/waitTask'
- $ref: '#/$defs/debugTask'
callTask:
type: object
oneOf:
Expand Down Expand Up @@ -533,6 +534,12 @@ $defs:
description: The amount of time to wait.
required: [ wait ]
description: Allows workflows to pause or delay their execution for a specified period of time.
debugTask:
type: object
properties:
debug:
type: string
description: Print a message (a literal string or an expression) to standard output
authenticationPolicy:
type: object
oneOf:
Expand Down
Loading