IWF-512: Add start timestamp field to all events

[lwolczynski]

Description

Checklist

• Code compiles correctly
• Tests for the changes have been added
• All tests passing
• This PR change is backwards-compatible
• This PR CONTAINS a (planned) breaking change (it is not backwards compatible)

Related Issue

Closes #issue_number

[lwolczynski]

@samuel27m let me know if this is the right approach. I wasn't 100% sure if these are the timestamp values you needed

[codecov]

Codecov Report

Attention: Patch coverage is 91.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 65.89%. Comparing base (8852a80) to head (1674796).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
service/interpreter/activityImpl.go	75.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #541      +/-   ##
==========================================
+ Coverage   65.80%   65.89%   +0.08%     
==========================================
  Files          62       62              
  Lines        6849     6872      +23     
==========================================
+ Hits         4507     4528      +21     
- Misses       2063     2065       +2     
  Partials      279      279              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mixydubs]

why use the WorkflowStartTime here? How does that relate to the time the workflow failed?

[samuel27m]

@samuel27m let me know if this is the right approach. I wasn't 100% sure if these are the timestamp values you needed

I was thinking something like this, however, I also thinking that the FAIL events should include the Endtimestamp too:

Start events should only contain the StartTimestampInMs:

event.Handle(iwfidl.IwfEvent{
    EventType:          iwfidl.WORKFLOW_START_EVENT,
    WorkflowType:       input.IwfWorkflowType,
    WorkflowId:         provider.GetWorkflowInfo(ctx).WorkflowExecution.ID,
    WorkflowRunId:      provider.GetWorkflowInfo(ctx).WorkflowExecution.RunID,
    SearchAttributes:   persistenceManager.GetAllSearchAttributes(),
    StartTimestampInMs: ptr.Any(provider.GetWorkflowInfo(ctx).WorkflowStartTime.UnixMilli()),
})

But then FAIL events should also include the EndTimestampInMs alongside the StartTimestampInMs

event.Handle(iwfidl.IwfEvent{
    EventType:          iwfidl.WORKFLOW_FAIL_EVENT,
    WorkflowType:       input.IwfWorkflowType,
    WorkflowId:         provider.GetWorkflowInfo(ctx).WorkflowExecution.ID,
    WorkflowRunId:      provider.GetWorkflowInfo(ctx).WorkflowExecution.RunID,
    SearchAttributes:   sas,
    StartTimestampInMs: ptr.Any(provider.GetWorkflowInfo(ctx).WorkflowStartTime.UnixMilli()), // the start time, its the exact same time as in the WORKFLOW_START_EVENT event
    EndTimestampInMs:   ptr.Any(provider.Now(ctx).UnixMilli()), // the end time of the workflow
})

Which would be the same implementation in timestamps as the COMPLETE events have:

event.Handle(iwfidl.IwfEvent{
     EventType:          iwfidl.WORKFLOW_COMPLETE_EVENT,
     WorkflowType:       input.IwfWorkflowType,
     WorkflowId:         provider.GetWorkflowInfo(ctx).WorkflowExecution.ID,
     WorkflowRunId:      provider.GetWorkflowInfo(ctx).WorkflowExecution.RunID,
     StartTimestampInMs: ptr.Any(provider.GetWorkflowInfo(ctx).WorkflowStartTime.UnixMilli()),
     EndTimestampInMs:   ptr.Any(provider.Now(ctx).UnixMilli()),
     SearchAttributes:   sas,
})

Let me know if I'm missing something here, but this is what I was thinking

[lwolczynski]

@samuel27m let me know if this is the right approach. I wasn't 100% sure if these are the timestamp values you needed

I was thinking something like this, however, I also thinking that the FAIL events should include the Endtimestamp too:

Start events should only contain the StartTimestampInMs:

event.Handle(iwfidl.IwfEvent{
    EventType:          iwfidl.WORKFLOW_START_EVENT,
    WorkflowType:       input.IwfWorkflowType,
    WorkflowId:         provider.GetWorkflowInfo(ctx).WorkflowExecution.ID,
    WorkflowRunId:      provider.GetWorkflowInfo(ctx).WorkflowExecution.RunID,
    SearchAttributes:   persistenceManager.GetAllSearchAttributes(),
    StartTimestampInMs: ptr.Any(provider.GetWorkflowInfo(ctx).WorkflowStartTime.UnixMilli()),
})

But then FAIL events should also include the EndTimestampInMs alongside the StartTimestampInMs

event.Handle(iwfidl.IwfEvent{
    EventType:          iwfidl.WORKFLOW_FAIL_EVENT,
    WorkflowType:       input.IwfWorkflowType,
    WorkflowId:         provider.GetWorkflowInfo(ctx).WorkflowExecution.ID,
    WorkflowRunId:      provider.GetWorkflowInfo(ctx).WorkflowExecution.RunID,
    SearchAttributes:   sas,
    StartTimestampInMs: ptr.Any(provider.GetWorkflowInfo(ctx).WorkflowStartTime.UnixMilli()), // the start time, its the exact same time as in the WORKFLOW_START_EVENT event
    EndTimestampInMs:   ptr.Any(provider.Now(ctx).UnixMilli()), // the end time of the workflow
})

Which would be the same implementation in timestamps as the COMPLETE events have:

event.Handle(iwfidl.IwfEvent{
     EventType:          iwfidl.WORKFLOW_COMPLETE_EVENT,
     WorkflowType:       input.IwfWorkflowType,
     WorkflowId:         provider.GetWorkflowInfo(ctx).WorkflowExecution.ID,
     WorkflowRunId:      provider.GetWorkflowInfo(ctx).WorkflowExecution.RunID,
     StartTimestampInMs: ptr.Any(provider.GetWorkflowInfo(ctx).WorkflowStartTime.UnixMilli()),
     EndTimestampInMs:   ptr.Any(provider.Now(ctx).UnixMilli()),
     SearchAttributes:   sas,
})

Let me know if I'm missing something here, but this is what I was thinking

This makes sense! Added now 👍

[samuel27m]

Thank you! 🎉