Make `workflow show` details more readable

antlai-temporal commented 1 week ago

What was changed

workflow show details are displayed with json but now we pretty-print it, properly align it to the left, and use a clear separator to indicate the next row. Note that trying to align indented json within a (narrow) last column was not easy to read... Also, an unrelated typo in a confirmation message was corrected.

Progress:
  ID           Time                     Type                   Details       
    1  2024-06-27T23:42:00Z  WorkflowExecutionStarted    
{
  "workflowType": {
    "name": "example"
  },
  "taskQueue": {
    "name": "hello-world",
    "kind": "TASK_QUEUE_KIND_NORMAL"
  },
  "input": {
    "payloads": [
      {
        "metadata": {
          "encoding": "anNvbi9wbGFpbg=="
        },
        "data": "IlRlbXBvcmFsIg=="
      }
    ]
  },
  "workflowTaskTimeout": "10s",
  "originalExecutionRunId": "6b8b1127-af83-4329-a327-2a22ec88c4a7",
  "identity": "904724@antlai-tp1",
  "firstExecutionRunId": "6b8b1127-af83-4329-a327-2a22ec88c4a7",
  "attempt": 1,
  "firstWorkflowTaskBackoff": "0s",
  "header": {},
  "workflowId": "workflow-RdV-QSRE-Ls9zQosf80cw"
}
----------------------------
    2  2024-06-27T23:42:00Z  WorkflowTaskScheduled       
{
  "taskQueue": {
    "name": "hello-world",
    "kind": "TASK_QUEUE_KIND_NORMAL"
  },
  "startToCloseTimeout": "10s",
  "attempt": 1
}
----------------------------
    3  2024-06-27T23:42:00Z  WorkflowTaskStarted         
{
  "scheduledEventId": "2",
  "identity": "904134@antlai-tp1",
  "requestId": "836dd605-2c75-4f11-b9ec-22387ed3dbf9",
  "historySizeBytes": "285",
  "workerVersion": {
    "buildId": "@temporalio/worker@1.10.1+e89347260f1727cf484da18008718f6532352a9e08d46719a9df0aadd063ac30"
  }
}
----------------------------

Checklist

Closes
546

cretz commented 1 week ago

I am not sure this is the right approach to just put the values on a separate line. This is meant to be a table, so the items should remain in the table and continue to look like a table with columns and such. I think we should compare with CLI 0.11. Can you show the output here of CLI 0.11 so we can see/compare?

The previous CLI had an entire table formatting approach for trimming long JSON into multiple lines inside a table column (and therefore some table rows had multiple lines). For complexity reasons it was not brought over when initially developed but probably does need to be brought over or developed similarly.

antlai-temporal commented 1 week ago

Can you show the output here of CLI 0.11 so we can see/compare?

Progress:
  [95mID[0m  [95m        Time        [0m  [95m           Type           [0m  [95m                                                     Details                                                      [0m  
   1  2024-06-27T23:42:00Z  WorkflowExecutionStarted    {WorkflowType:{Name:example},                                                                                       
                                                        ParentInitiatedEventId:0, TaskQueue:{Name:hello-world,                                                              
                                                        Kind:Normal}, Input:["Temporal"],                                                                                   
                                                        WorkflowTaskTimeout:10s, Initiator:Unspecified,                                                                     
                                                        OriginalExecutionRunId:6b8b1127-af83-4329-a327-2a22ec88c4a7,                                                        
                                                        Identity:904724@antlai-tp1,                                                                                         
                                                        FirstExecutionRunId:6b8b1127-af83-4329-a327-2a22ec88c4a7,                                                           
                                                        Attempt:1, FirstWorkflowTaskBackoff:0s,                                                                             
                                                        ParentInitiatedEventVersion:0,                                                                                      
                                                        WorkflowId:workflow-RdV-QSRE-Ls9zQosf80cw}                                                                          
   2  2024-06-27T23:42:00Z  WorkflowTaskScheduled       {TaskQueue:{Name:hello-world,                                                                                       
                                                        Kind:Normal},                                                                                                       
                                                        StartToCloseTimeout:10s,                                                                                            
                                                        Attempt:1}                                                                                                          
   3  2024-06-27T23:42:00Z  WorkflowTaskStarted         {ScheduledEventId:2,                                                                                                
                                                        Identity:904134@antlai-tp1,                                                                                         
                                                        RequestId:836dd605-2c75-4f11-b9ec-22387ed3dbf9,                                                                     
                                                        SuggestContinueAsNew:false,                                                                                         
                                                        HistorySizeBytes:285}                                                                                               
   4  2024-06-27T23:42:00Z  WorkflowTaskCompleted       {ScheduledEventId:2, StartedEventId:3, Identity:904134@antlai-tp1,                                                  
                                                        WorkerVersion:{BuildId:@temporalio/worker@1.10.1+e89347260f1727cf484da18008718f6532352a9e08d46719a9df0aadd063ac30,  
                                                        UseVersioning:false}, SdkMetadata:{CoreUsedFlags:[2,...1 more], LangUsedFlags:[]},                                  
                                                        MeteringMetadata:{NonfirstLocalActivityExecutionAttempts:0}}

antlai-temporal commented 1 week ago

@cretz Frankly, I didn't find it very readable the json output of the old cli, as you see above. Also displaying the json in a narrow column is just a mess, you don't really get advantage of the width of the console. I also like that it is much easier to grep the output and extract values of attributes if they are in separate lines...

cretz commented 1 week ago

I find the format you pasted from old CLI to be much more legible than the format in the description (ignoring the unicode artifacts in the header that are pasted here to misalign the headers). The text output isn't meant to necessarily have all values I don't think (if you're wanting to grep the raw JSON might as well use JSON format). If we want to move away from the row/column model of a table that can be discussed more generally. May be better to get more opinions on ideal format here as I admit I don't use CLIs much.

dnr commented 1 week ago

As someone who does use it a bunch: I'm 100% with @antlai-temporal. The old table format is hideous and unreadable. The json is much nicer. My only nitpick would be to indent the json by a few spaces (4?) so it looks like it's "inside" the line above it. (Also colorize it like jq but that might be getting too fancy)

cretz commented 1 week ago

@dnr - to confirm, you are ok with table columns wrapping back to beginning of line under the other column? Would like to get one or two more opinions from others that usually have them concerning CLI output (@dandavison, @josh-berry). If all agree, we can merge this (I don't have a super strong opinion).

dnr commented 1 week ago

Sure, that's fine. Just indent it a little more so the boundaries between events are more visible.

I don't think there's anything sacred about "tables". We should present information in the most useful and readable way and not be constrained by rules that we made up. In the specific case of history: tables make the most sense for a list of homogeneous items. History events have a few fields in common but are basically heterogeneous. This PR keeps the table format for the homogeneous fields (id, type, time) and json for the heterogeneous ones, which is about as good as we can do for this data.

cretz commented 1 week ago

I don't think there's anything sacred about "tables". We should present information in the most useful and readable way and not be constrained by rules that we made up.

Well tables are kinda a thing that exists, doesn't make them sacred. If we don't want to present this in a table that is fine too. But it's not unreasonable for someone to expect columns to remain within columns on tables. Columns are not something we made up. I personally think "as good as we can do" was what it was before (there is always JSON format if you like the full JSON contents), but I don't mind this way too. I just want to confirm others are ok with this new column approach.

dandavison commented 1 week ago

Here are my thoughts.

First of all, this is for-humans output (i.e. it is not --json). As such, we can and should make incremental improvements whenever people get the time to do so without any concept of "breaking" the output.

I'm a bit hesitant about this mixture of table and JSON -- I find it a bit visually unappealing (same for the original version with the squashed JSON in the final column). Is the idea that a user would copy the JSON and parse it? If so, a major concern with including JSON in "for-humans" output is that I don't want us to start feeling that we can't change the output.

An alternative would be to have a fields option controlling which fields to include as table columns. Another alternative is to offer the different formats that database CLI clients offer, i.e. each row is rendered in transposed form as a two-column mini-table with the fields down the left, and then some sort of divider between these "mini-table" rows. Also, we should note that people can always pipe to less -S and scroll right to see arbitrarily long lines. I don't mind if we pass through this JSON form as an intermediate stage of evolution, as long as we're not committed to keeping it.

My other suggestions are:

The event type should come before (to the left of) the timestamp. People only occasionally care about the timestamps. But you always have to immediately know the event type to understand the event.
The timestamp that is given its own column should be a time duration relative to workflow start. The (ugly) absolute timestamp should be relegated to the JSON details data.
We should use color in the CLI. Specifically here, we should introduce a color scheme to distinguish certain event types. Among other things this color scheme should give an intuitive visual color similarity to the *Scheduled, *Started, *Completed task sequences. Or possibly some combination of color and unicode characters.
Dividing lines should probably extend to the current terminal width. There's no problem with detecting that reliably in the majority of modern terminal emulators (CLI applications that do so do not receive many issues from users saying it's not working)
Dividing lines should probably use non-ascii characters such as the Unicode box-drawing set.
Adornments such as color and fancy characters can be disabled when output is not a TTY (i.e. when being piped or redirected to file)

dandavison commented 1 week ago

Something like this is another possibility for non-JSON output (with colored event types etc). A nice feature this has is that it is greppable (like gron output).

────────────────────────────────────────────────────────
ID: 1
Type: WorkflowExecutionStarted
RelativeTime: 0.00
Time: 2024-06-27T23:42:00Z
workflowType.name: example
taskQueue.name: hello-world
taskQueue.kind: TASK_QUEUE_KIND_NORMAL
...
────────────────────────────────────────────────────────
ID: 2
Type: WorkflowTaskScheduled
RelativeTime: 0.01
Time: 2024-06-27T23:42:00Z
...

antlai-temporal commented 1 week ago

Using gron for comparison:

ID           Time                     Type                   Details       
    1  2024-06-27T23:42:00Z  WorkflowExecutionStarted 
json = {};
json.attempt = 1;
json.firstExecutionRunId = "6b8b1127-af83-4329-a327-2a22ec88c4a7";
json.firstWorkflowTaskBackoff = "0s";
json.header = {};
json.identity = "904724@antlai-tp1";
json.input = {};
json.input.payloads = [];
json.input.payloads[0] = {};
json.input.payloads[0].data = "IlRlbXBvcmFsIg==";
json.input.payloads[0].metadata = {};
json.input.payloads[0].metadata.encoding = "anNvbi9wbGFpbg==";
json.originalExecutionRunId = "6b8b1127-af83-4329-a327-2a22ec88c4a7";
json.taskQueue = {};
json.taskQueue.kind = "TASK_QUEUE_KIND_NORMAL";
json.taskQueue.name = "hello-world";
json.workflowId = "workflow-RdV-QSRE-Ls9zQosf80cw";
json.workflowTaskTimeout = "10s";
json.workflowType = {};
json.workflowType.name = "example";
----------------------------
    2  2024-06-27T23:42:00Z  WorkflowTaskScheduled 
son = {};
json.attempt = 1;
json.startToCloseTimeout = "10s";
json.taskQueue = {};
json.taskQueue.kind = "TASK_QUEUE_KIND_NORMAL";
json.taskQueue.name = "hello-world";
-------------------------------------
  3  2024-06-27T23:42:00Z  WorkflowTaskStarted         
json = {};
json.historySizeBytes = "285";
json.identity = "904134@antlai-tp1";
json.requestId = "836dd605-2c75-4f11-b9ec-22387ed3dbf9";
json.scheduledEventId = "2";
json.workerVersion = {};
json.workerVersion.buildId = "@temporalio/worker@1.10.1+e89347260f1727cf484da18008718f6532352a9e08d46719a9df0aadd063ac30";
----------------------

cretz commented 1 week ago

Using gron for comparison:

From internal discussion I would 1) remove the json. prepend, 2) remove the quotes around strings, 3) omit anything empty, 4) use : instead of =, 5) remove ;, 6) see why payload JSON shorthand is not applying, and 7) not use a table anymore (i.e. those three cols are just the first three fields). So it'd look more like:

eventId: 1
time: 2024-06-27T23:42:00Z
name: WorkflowExecutionStarted 
attempt: 1
firstExecutionRunId: 6b8b1127-af83-4329-a327-2a22ec88c4a7
firstWorkflowTaskBackoff: 0s
identity: 904724@antlai-tp1
input.payloads[0]: Temporal
originalExecutionRunId: 6b8b1127-af83-4329-a327-2a22ec88c4a7
taskQueue.kind: TASK_QUEUE_KIND_NORMAL
taskQueue.name: hello-world
workflowId: workflow-RdV-QSRE-Ls9zQosf80cw
workflowTaskTimeout: 10s
workflowType.name: example
----------------------------
eventId: 2
time: 2024-06-27T23:42:00Z
name: WorkflowTaskScheduled 
attempt: 1
startToCloseTimeout: 10s
taskQueue.kind: TASK_QUEUE_KIND_NORMAL
taskQueue.name: hello-world
-------------------------------------
eventId: 3
time: 2024-06-27T23:42:00Z
name: WorkflowTaskStarted
historySizeBytes: 285
identity: 904134@antlai-tp1
requestId: 836dd605-2c75-4f11-b9ec-22387ed3dbf9
scheduledEventId: 2
workerVersion.buildId: @temporalio/worker@1.10.1+e89347260f1727cf484da18008718f6532352a9e08d46719a9df0aadd063ac30

Sushisource commented 6 days ago

I also agree that the properly indented output is 10x better than the barf from before. The columns I don't think are relevant to anyone really - they're neither very parsable nor particularly helpful to a reader (though, in @antlai-temporal 's original example, I'd probably just get rid of the "details" header since it's not doing much).

I think @cretz 's latest example is pretty nice (though, name should be type for event types).

Do that, keep colorized output for different event types, seems real nice to me.

cretz commented 6 days ago

@Sushisource - based on internal discussions, we have come up with https://github.com/temporalio/cli/issues/546#issuecomment-2203553067. Basically it's clear that table + details don't mix. Can tweak it as needed (feel free to start internal discussion on any feedback to that comment so we can get it correct).

Sushisource commented 6 days ago

Cool I like that

antlai-temporal commented 6 days ago

Closing this PR since the main approach has changed...

temporalio / cli

Make `workflow show` details more readable #608

What was changed

Checklist

546