nf-tower icon indicating copy to clipboard operation
nf-tower copied to clipboard

Workflow metrics page

Open pditommaso opened this issue 5 years ago • 14 comments

The workflow page should include a workflow execution stats metrics similar to the ones produced by the nextflow execution report.

ideally, most of the code should be reused.

pditommaso avatar Jul 16 '19 10:07 pditommaso

I've managed to implement a workflow metrics component reusing most of the legacy code from the nextflow execution report.

screencapture-localhost-8000-metrics-2-2019-07-21-15_34_54

For now, it's rendered as an independent page at the url /workflow/metics/:id. Having this it should be straightforward to embed it as a card in the workflow page.

Notably, it uses Plotly JS library imported directly in the component:

https://github.com/seqeralabs/nf-tower/blob/3be2df6ceb365465960e7b578b7ce2273ef6e92b/tower-web/src/app/modules/main/component/workflow-metrics/metrics.component.ts#L17

@tcrespog does this makes the plotly javascript lazy-loaded, or it will be included in the main app bundle once we add this component in the main page?

pditommaso avatar Jul 21 '19 14:07 pditommaso

As long as the library isn't included in the package.json, then it shouldn't be eagerly loaded. So, your solution loads the script from the CDN when the workflow page is initialized (therefore, lazily). Why do you want to lazy load this specific library?

tcrespog avatar Jul 22 '19 05:07 tcrespog

Oh, now I see that you have added the plotly-1.34.0.min.js inside the component source directory, that seems strange to me. I guess this shouldn't be done if the script is being loaded from the CDN.

tcrespog avatar Jul 22 '19 05:07 tcrespog

That was needed to have the class Plotly accessible in the script. Quite neat actually.

I've realised only now that remained the loading from the CDN

https://github.com/seqeralabs/nf-tower/blob/3be2df6ceb365465960e7b578b7ce2273ef6e92b/tower-web/src/app/modules/main/component/workflow-metrics/metrics.component.ts#L36

This was an early tentative, but it was not working randomly likely due to the lazy loading latency. It's supposed to be removed.

pditommaso avatar Jul 22 '19 07:07 pditommaso

Reminder to myself, metrics entries display sequence should be consistent with the one displayed in the nextflow execution report. Likely an index attribute needs to be introduced to keep them ordered as they are created.

pditommaso avatar Jul 22 '19 07:07 pditommaso

That was needed to have the class Plotly accessible in the script. Quite neat actually.

Ok. Why not loading the dependency from package.json as the usual way?

tcrespog avatar Jul 22 '19 07:07 tcrespog

Because it's quite a big javascript file that's only used in that component. I would like to keep the app loading as fast as possible.

pditommaso avatar Jul 22 '19 07:07 pditommaso

Ok Paolo. But are you sure this solution achieves the lazy loading behavior that you want? Does it also work in the build form of the application?

tcrespog avatar Jul 22 '19 07:07 tcrespog

But are you sure this solution achieves the lazy loading behavior that you want?

No 😄

The main driver here was to reuse the existing code and html layout. The other main thing now is to embed this in the main workflow page showing it only when the workflow execution has completed.

I'll check if it works when building the app in prod mode.

pditommaso avatar Jul 22 '19 08:07 pditommaso

umm, I'm seeing this

chunk {0} runtime.465c2333d355155ec5f3.js (runtime) 1.41 kB [entry] [rendered]
chunk {1} main.83d2fdb9d124f70e3f9a.js (main) 2.79 MB [initial] [rendered]
chunk {2} polyfills.c53f45809f78efe47efe.js (polyfills) 43 kB [initial] [rendered]
chunk {3} polyfills-es5.6a9f5fde365a77ab7943.js (polyfills-es5) 69.8 kB [initial] [rendered]
chunk {4} styles.a23b78e918ba472ebd4e.css (styles) 313 kB [initial] [rendered]
chunk {scripts} scripts.eb0ddefb4dc782c7bc0e.js (scripts) 255 kB [entry] [rendered]
Date: 2019-07-22T08:05:13.009Z - Hash: 67e1cac15fba2ddf0084 - Time: 108839ms

WARNING in budgets, maximum exceeded for initial. Budget 2 MB was exceeded by 1.46 MB.

It's warning that the script bundle it's too big

pditommaso avatar Jul 22 '19 08:07 pditommaso

:smile: I ask this because as far as I know, I think the default behavior of the import statements is to lazy load the JavaScript modules. In fact, some JS libraries follow a pattern called "tree-shaking" which allows to load just the chosen exported modules without loading the full JS file in case of big libraries (RxJS or Lodash follow this pattern). So we might have to confirm this fact, but I think there wouldn't be any difference between importing the library in this way and including it in the pacakge.json file.

tcrespog avatar Jul 22 '19 08:07 tcrespog

Once compiled I don't see the plotly script loaded, therefore it must be included in the main js bundle

Screenshot 2019-07-22 at 10 13 36

I still prefer this import because makes it explicit that the lib is needed by this component.

Then we'll see how to lazy load it. It's not a priority.

pditommaso avatar Jul 22 '19 08:07 pditommaso

Ok, I'll leave this here as a future reference: https://github.com/plotly/angular-plotly.js/issues/50

tcrespog avatar Jul 22 '19 08:07 tcrespog

Related https://github.com/mohammedzamakhan/ngx-loadable

pditommaso avatar Dec 07 '19 07:12 pditommaso