Repository: mlflow/mlflow
Pull request: Added start_time and duration to csv export
Signed-off-by: Marijn Valk <marijncv@hotmail.com>

## What changes are proposed in this pull request?

This PR adds the start time and duration of each run to the csv export. closes #4850 

## How is this patch tested?

Use the csv export button and see the Start time and duration added. For runs in progress, the duration cell is empty

## Release Notes

### Is this a user-facing change?

- [ ] No. You can skip the rest of this section.
- [x] Yes. Give a description of this change to be included in the release notes for MLflow users.

Start time and duration have been added to the csv export in the experiment UI

### What component(s), interfaces, languages, and integrations does this PR affect?
Components 
- [ ] `area/artifacts`: Artifact stores and artifact logging
- [ ] `area/build`: Build and test infrastructure for MLflow
- [ ] `area/docs`: MLflow documentation pages
- [ ] `area/examples`: Example code
- [ ] `area/model-registry`: Model Registry service, APIs, and the fluent client calls for Model Registry
- [ ] `area/models`: MLmodel format, model serialization/deserialization, flavors
- [ ] `area/projects`: MLproject format, project running backends
- [ ] `area/scoring`: MLflow Model server, model deployment tools, Spark UDFs
- [ ] `area/server-infra`: MLflow Tracking server backend
- [ ] `area/tracking`: Tracking Service, tracking client APIs, autologging

Interface 
- [x] `area/uiux`: Front-end, user experience, plotting, JavaScript, JavaScript dev server
- [ ] `area/docker`: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
- [ ] `area/sqlalchemy`: Use of SQLAlchemy in the Tracking Service or Model Registry
- [ ] `area/windows`: Windows support

Language 
- [ ] `language/r`: R APIs and clients
- [ ] `language/java`: Java APIs and clients
- [ ] `language/new`: Proposals for new client languages

Integrations
- [ ] `integrations/azure`: Azure and Azure ML integrations
- [ ] `integrations/sagemaker`: SageMaker integrations
- [ ] `integrations/databricks`: Databricks integrations

<!--
Insert an empty named anchor here to allow jumping to this section with a fragment URL
(e.g. https://github.com/mlflow/mlflow/pull/123#user-content-release-note-category).
Note that GitHub prefixes anchor names in markdown with "user-content-".
-->
<a name="release-note-category"></a>
### How should the PR be classified in the release notes? Choose one:

- [ ] `rn/breaking-change` - The PR will be mentioned in the "Breaking Changes" section
- [ ] `rn/none` - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
- [x] `rn/feature` - A new user-facing feature worth mentioning in the release notes
- [ ] `rn/bug-fix` - A user-facing bug fix worth mentioning in the release notes
- [ ] `rn/documentation` - A user-facing documentation change worth mentioning in the release notes


Problem statement:
[FR] Include start time and run duration in "Download CSV" exported data
## MLflow Roadmap Item

This is an MLflow Roadmap item that has been prioritized by the MLflow maintainers. We're seeking help with the implementation of roadmap items tagged with the `help wanted` label.

For requirements clarifications and implementation questions, or to request a PR review, please tag @dbczumar in your communications related to this issue.

## Proposal Summary

Include the start time for each run entry in the CSV exported by the "Download CSV" button in the MLflow Experiment UI. Additionally, for runs that have completed, include the duration.

## Motivation
- What is the use case for this feature? Run duration is often a meaningful heuristic in ML training, serving as a proxy for the quality of a model or illuminating issues in the training process (e.g. poor convergence).
- Why is this use case valuable to support for MLflow users in general? ^
- Why is this use case valuable to support for your project(s) or organization? ^
Why is it currently difficult to achieve this use case? Start time and duration information is not available at all in the exported CSV file.

### What component(s), interfaces, languages, and integrations does this feature affect?
Components 
- [ ] `area/artifacts`: Artifact stores and artifact logging
- [ ] `area/build`: Build and test infrastructure for MLflow
- [ ] `area/docs`: MLflow documentation pages
- [ ] `area/examples`: Example code
- [ ] `area/model-registry`: Model Registry service, APIs, and the fluent client calls for Model Registry
- [ ] `area/models`: MLmodel format, model serialization/deserialization, flavors
- [ ] `area/projects`: MLproject format, project running backends
- [ ] `area/scoring`: MLflow Model server, model deployment tools, Spark UDFs
- [ ] `area/server-infra`: MLflow Tracking server backend
- [ ] `area/tracking`: Tracking Service, tracking client APIs, autologging

Interfaces
- [X] `area/uiux`: Front-end, user experience, plotting, JavaScript, JavaScript dev server
- [ ] `area/docker`: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
- [ ] `area/sqlalchemy`: Use of SQLAlchemy in the Tracking Service or Model Registry
- [ ] `area/windows`: Windows support

Languages 
- [ ] `language/r`: R APIs and clients
- [ ] `language/java`: Java APIs and clients
- [ ] `language/new`: Proposals for new client languages

Integrations
- [ ] `integrations/azure`: Azure and Azure ML integrations
- [ ] `integrations/sagemaker`: SageMaker integrations
- [ ] `integrations/databricks`: Databricks integrations

Hints from issue discussion:
cc @harupy @ianshan0915

Commit message: Fix timestamp timezone issue

Signed-off-by: Marijn Valk <marijncv@hotmail.com>

Patch to review:
diff --git a/mlflow/server/js/src/common/utils/Utils.js b/mlflow/server/js/src/common/utils/Utils.js
index 168e453c0e9aa..07d3913856bb1 100644
--- a/mlflow/server/js/src/common/utils/Utils.js
+++ b/mlflow/server/js/src/common/utils/Utils.js
@@ -191,7 +191,7 @@ class Utils {
    * @param endTime in milliseconds
    */
   static getDuration(startTime, endTime) {
-    return startTime && endTime ? this.formatDuration(endTime - startTime) : null;
+    return startTime && endTime ? this.formatDuration(endTime - startTime) : '';
   }
 
   static baseName(path) {
diff --git a/mlflow/server/js/src/common/utils/Utils.test.js b/mlflow/server/js/src/common/utils/Utils.test.js
index 6c4cfa135f1cc..ef7ad43e9896a 100644
--- a/mlflow/server/js/src/common/utils/Utils.test.js
+++ b/mlflow/server/js/src/common/utils/Utils.test.js
@@ -71,12 +71,12 @@ test('formatDuration', () => {
 });
 
 test('getDuration', () => {
-  expect(Utils.getDuration(1, null)).toEqual(null);
-  expect(Utils.getDuration(1, undefined)).toEqual(null);
-  expect(Utils.getDuration(null, 1)).toEqual(null);
-  expect(Utils.getDuration(undefined, 1)).toEqual(null);
-  expect(Utils.getDuration(undefined, undefined)).toEqual(null);
-  expect(Utils.getDuration(null, null)).toEqual(null);
+  expect(Utils.getDuration(1, null)).toEqual('');
+  expect(Utils.getDuration(1, undefined)).toEqual('');
+  expect(Utils.getDuration(null, 1)).toEqual('');
+  expect(Utils.getDuration(undefined, 1)).toEqual('');
+  expect(Utils.getDuration(undefined, undefined)).toEqual('');
+  expect(Utils.getDuration(null, null)).toEqual('');
   expect(Utils.getDuration(1, 11)).toEqual('10ms');
   expect(Utils.getDuration(1, 501)).toEqual('0.5s');
   expect(Utils.getDuration(1, 901)).toEqual('0.9s');
diff --git a/mlflow/server/js/src/experiment-tracking/components/ExperimentView.js b/mlflow/server/js/src/experiment-tracking/components/ExperimentView.js
index 10d0dd35c01e3..3486415bb61a9 100644
--- a/mlflow/server/js/src/experiment-tracking/components/ExperimentView.js
+++ b/mlflow/server/js/src/experiment-tracking/components/ExperimentView.js
@@ -1253,6 +1253,8 @@ export class ExperimentView extends Component {
     tagsList,
   ) {
     const columns = [
+      'Start Time',
+      'Duration',
       'Run ID',
       'Name',
       'Source Type',
@@ -1266,6 +1268,8 @@ export class ExperimentView extends Component {
 
     const data = runInfos.map((runInfo, index) => {
       const row = [
+        Utils.formatTimestamp(runInfo.start_time),
+        Utils.getDuration(runInfo.start_time, runInfo.end_time),
         runInfo.run_uuid,
         Utils.getRunName(tagsList[index]), // add run name to csv export row
         Utils.getSourceType(tagsList[index]),
diff --git a/mlflow/server/js/src/experiment-tracking/components/ExperimentView.test.js b/mlflow/server/js/src/experiment-tracking/components/ExperimentView.test.js
index 85c03e3be8a0b..7ef2edf8d1f2e 100644
--- a/mlflow/server/js/src/experiment-tracking/components/ExperimentView.test.js
+++ b/mlflow/server/js/src/experiment-tracking/components/ExperimentView.test.js
@@ -191,10 +191,17 @@ describe('Download CSV', () => {
 
   const blobOptionExpected = { type: 'application/csv;charset=utf-8' };
   const filenameExpected = 'runs.csv';
+  const startTimeStringExpected = Utils.formatTimestamp(
+    getDefaultExperimentViewProps().runInfos[0].start_time,
+  );
   const saveAsSpy = jest.spyOn(FileSaver, 'saveAs');
+  const blobSpy = jest.spyOn(global, 'Blob').mockImplementation((content, options) => {
+    return { content, options };
+  });
 
   afterEach(() => {
     saveAsSpy.mockClear();
+    blobSpy.mockClear();
   });
 
   test('Downloaded CSV contains tags', () => {
@@ -206,16 +213,14 @@ describe('Download CSV', () => {
       }),
     ];
     const csvExpected = `
-Run ID,Name,Source Type,Source Name,User,Status,batch_size,acc,a,b
-run-id,name,LOCAL,src.py,user,FINISHED,512,0.1,0,1
+Start Time,Duration,Run ID,Name,Source Type,Source Name,User,Status,batch_size,acc,a,b
+${startTimeStringExpected},0ms,run-id,name,LOCAL,src.py,user,FINISHED,512,0.1,0,1
 `.substring(1); // strip a leading newline
 
     const wrapper = getExperimentViewMock({ tagsList });
     wrapper.instance().onDownloadCsv();
-    expect(saveAsSpy).toHaveBeenCalledWith(
-      new Blob([csvExpected], blobOptionExpected),
-      filenameExpected,
-    );
+    expect(saveAsSpy).toHaveBeenCalledWith(expect.anything(), filenameExpected);
+    expect(blobSpy).toHaveBeenCalledWith([csvExpected], blobOptionExpected);
   });
 
   test('Downloaded CSV does not contain unchecked tags', () => {
@@ -227,8 +232,8 @@ run-id,name,LOCAL,src.py,user,FINISHED,512,0.1,0,1
       }),
     ];
     const csvExpected = `
-Run ID,Name,Source Type,Source Name,User,Status,batch_size,acc,a
-run-id,name,LOCAL,src.py,user,FINISHED,512,0.1,0
+Start Time,Duration,Run ID,Name,Source Type,Source Name,User,Status,batch_size,acc,a
+${startTimeStringExpected},0ms,run-id,name,LOCAL,src.py,user,FINISHED,512,0.1,0
 `.substring(1);
 
     const wrapper = getExperimentViewMock({ tagsList });
@@ -240,25 +245,21 @@ run-id,name,LOCAL,src.py,user,FINISHED,512,0.1,0
     });
     // Then, download CSV
     wrapper.instance().onDownloadCsv();
-    expect(saveAsSpy).toHaveBeenCalledWith(
-      new Blob([csvExpected], blobOptionExpected),
-      filenameExpected,
-    );
+    expect(saveAsSpy).toHaveBeenCalledWith(expect.anything(), filenameExpected);
+    expect(blobSpy).toHaveBeenCalledWith([csvExpected], blobOptionExpected);
   });
 
   test('CSV download succeeds without tags', () => {
     const tagsList = [createTags(mlflowSystemTags)];
     const csvExpected = `
-Run ID,Name,Source Type,Source Name,User,Status,batch_size,acc
-run-id,name,LOCAL,src.py,user,FINISHED,512,0.1
+Start Time,Duration,Run ID,Name,Source Type,Source Name,User,Status,batch_size,acc
+${startTimeStringExpected},0ms,run-id,name,LOCAL,src.py,user,FINISHED,512,0.1
 `.substring(1);
 
     const wrapper = getExperimentViewMock({ tagsList });
     wrapper.instance().onDownloadCsv();
-    expect(saveAsSpy).toHaveBeenCalledWith(
-      new Blob([csvExpected], blobOptionExpected),
-      filenameExpected,
-    );
+    expect(saveAsSpy).toHaveBeenCalledWith(expect.anything(), filenameExpected);
+    expect(blobSpy).toHaveBeenCalledWith([csvExpected], blobOptionExpected);
   });
 });
 
diff --git a/mlflow/server/js/src/experiment-tracking/components/RunView.js b/mlflow/server/js/src/experiment-tracking/components/RunView.js
index 587f4d5f5c166..f6d38202a2757 100644
--- a/mlflow/server/js/src/experiment-tracking/components/RunView.js
+++ b/mlflow/server/js/src/experiment-tracking/components/RunView.js
@@ -308,7 +308,7 @@ export class RunViewImpl extends Component {
           >
             {Utils.getUser(run, tags)}
           </Descriptions.Item>
-          {duration !== null ? (
+          {duration !== '' ? (
             <Descriptions.Item
               label={this.props.intl.formatMessage({
                 defaultMessage: 'Duration',


Code review comments:
diff_hunk: @@ -191,7 +191,7 @@ class Utils {
    * @param endTime in milliseconds
    */
   static getDuration(startTime, endTime) {
-    return startTime && endTime ? this.formatDuration(endTime - startTime) : null;
+    return startTime && endTime ? this.formatDuration(endTime - startTime) : '';
path: mlflow/server/js/src/common/utils/Utils.js
text: @user1:
I think returning `null` here is semantically clearer than returning the empty string. Can we maintain the `null` return type and handle the mapping from `null` to empty string in the CSV export routine?

@author:
Agreed. Reverted back to returning `null` here. 

For consistency, should this PR refactor the other getter functions in [Utils](https://github.com/mlflow/mlflow/blob/master/mlflow/server/js/src/common/utils/Utils.js) (getRunName, getSourceName etc.) to return `null` instead of empty strings? Let me know!