[Core] Add TimeStampMillis to TimeStampedEvent

[mpkorstanje]

Summary

A timestamp is more useful when that timestamp is relative to the epoch.
Unlike System.currentTimeMillis, System.nanoTime is relative to some
arbitrary point in time.

Types of changes

• Bug fix (non-breaking change which fixes an issue).
• New feature (non-breaking change which adds functionality).
• Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

• I've added tests for my code.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[coveralls]

Coverage decreased (-0.3%) to 86.184% when pulling 0b7ff41 on add-timestampe-in-milliseconds-to-all-event into 6698a38 on master.

[zutshiy]

Loos pretty good :) I like that we are using the timeStampMillis in all the other places as well now, it would probably be super useful.

How do u feel about just sending the end and start times in the report and leaving the calculation of the duration to the system using the report instead? (since we have added start time anyway)

[aslakhellesoy]

A timestamp is more useful when that timestamp is relative to the epoch.

Useful for what exactly? There are two broad categories:

• Knowing how long something took (24 minutes)
• Knowing when something happened (12 Oct 1957)

Nano time is often best for duration, Millis time is often best for time. See https://stackoverflow.com/questions/351565/system-currenttimemillis-vs-system-nanotime

What are we trying to accomplish with these timestamps?

[aslakhellesoy]

I agree we don’t need the nano precision and happy to switch to milliseconds to measure elapsed time. I’m not sure we need to store those times as duration (relative to some start time) or absolute (time since epoch). I’d like to understand what the timestamps are used for first.

[zutshiy]

@aslakhellesoy The initial reason was to allow reports to know the concurrent start time of when a test case (or scenario) started, to allow them to accurately display the actual time it took to run a suite when running TCs in parallel (if we just give the duration, they do not know which TCs were run in parallel).

Hence the initial PR #1591

[mpkorstanje]

@aslakhellesoy

What are we trying to accomplish with these timestamps?

1. Correct the current implementation. Timestamps indicate a point in time. This makes nano time the wrong choice for use in TimeStampedEvent.

   In the long run we should replace TimeService with the java standard Clock and long timeStamp with Instant and long duration with Duration as these more accurately represent the concepts used here.

2. Allow cucumbers events to be correlated with other events in other systems. When analyzing test failures of a deployed system it is useful to know when an event happened. Most log systems allow you to filter by a time range. Currently this information is not accessible.

   @laxersaz (the author of Cluecumber) already expressed interest in including this into his reports.

3. Allows execution of multiple (concurrent) scenarios from different runners to be aggregated and displayed on a real time graph.

[mpkorstanje]

@gazler22

How do u feel about just sending the end and start times in the report and leaving the calculation of the duration to the system using the report instead? (since we have added start time anyway)

Would be nice but we have some existing systems to support. It's also kinda convenient.

[mpkorstanje]

I'm going to revert the use of milliseconds for duration. While test cases last milliseconds, it seems that individual steps are more granular then that.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.