Display skipped executions warning for metrics

[kathy-t]

Description
Corresponding PRs:

• Record number of skipped executions for aggregated metrics dockstore#5822
• Record number of skipped executions during aggregation dockstore-support#486
• Add 1.16 migrations compose_setup#251

This PR displays a warning icon if the aggregated metric had executions that were skipped during aggregation. The tool tip displays the number of executions skipped.

Old screenshot


Review Instructions
Login as a curator, admin or platform partner and verify that this workflow https://qa.dockstore.org/workflows/github.com/kathy-t/ghapps-single-workflow:master?tab=metrics has skipped executions warnings for the CPU, Memory, and Cost metrics.

Logout and view the same workflow. You should not see any skipped executions warnings.

Issue
SEAB-6046

Security
If there are any concerns that require extra attention from the security team, highlight them here.

Please make sure that you've checked the following before submitting your pull request. Thanks!

• Check that your code compiles by running npm run build
• Ensure that the PR targets the correct branch. Check the milestone or fix version of the ticket.
• If this is the first time you're submitting a PR or even if you just need a refresher, consider reviewing our style guide
• Do not bypass Angular sanitization (bypassSecurityTrustHtml, etc.), or justify why you need to do so
• If displaying markdown, use the markdown-wrapper component, which does extra sanitization
• Do not use cookies, although this may change in the future
• Run npm audit and ensure you are not introducing new vulnerabilities
• Do due diligence on new 3rd party libraries, checking for CVEs
• Don't allow user-uploaded images to be served from the Dockstore domain
• If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.
• Check whether this PR disables tests. If it legitimately needs to disable a test, create a new ticket to re-enable it in a specific milestone.

[codecov]

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 40.32%. Comparing base (8a1faa1) to head (4b0208b).

Files	Patch %	Lines
...pp/workflow/executions/executions-tab.component.ts	20.00%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1938      +/-   ##
===========================================
+ Coverage    40.28%   40.32%   +0.03%     
===========================================
  Files          398      398              
  Lines        11899    11903       +4     
  Branches      2879     2880       +1     
===========================================
+ Hits          4794     4800       +6     
+ Misses        4827     4826       -1     
+ Partials      2278     2277       -1     

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

[denis-yuen]

This PR displays a warning icon if the aggregated metric had executions that were skipped during aggregation. The tool tip displays the number of executions skipped.

If I understand correctly, I feel like the warning is a bit too prominent for the following reasons

1. Once we start skipping executions, even if new executions in the future are fixed, the older skipped executions will remain and be skipped in the future
2. Even one skipped execution in thousands of runs could cause the icon to pop up
3. The icon isn't really actionable for end-users, just the platform owner

Brainstorming:
Not sure how to display though, ideas

• "Skipped executions" with a tooltip next to aborted executions without the icon?
• Info icon, not warning icon?
• Display only to admins, curators, and platform owners?

[denis-yuen]

technically, looks ok
think warning is a bit too prominent and/or hard to interpret for most users, so may need to consider alternatives

[svonworl]

The term "skipped" makes me think of the run not being executed because the workflow engine decided not to or something like that. I wonder if there's another term or a different way of phrasing it that makes it clearer that we skipped it during the analysis phase.

[coverbeck]

Random thoughts, just brainstorming:

• I'm wary of too much information being in a tooltip; tooltips should supplement info displayed, not be a substitute, because most users don't know to look for them, IMO.
• How frequently are executions skipped in our current data set? Is this a once in a blue moon sort of thing, or will it be fairly common? If it's reasonably common, I would be tempted to just have another column with the number.
• Does a user need to know in detail that, for example, the memory metric aggregation was skipped 8 times, but the CPU metric was skipped 11 times? I would argue not.
• What if we captured the total number of skipped aggregations at the top along with Total/Successful/Failed/Aborted? I guess the skipped is at different level though, a single execution could have more than one skipped, right?
• Digression, I think the top could be improved; instead of repeating the word "Executions" 4 times, we could have it once.

[denis-yuen]

Does a user need to know in detail that, for example, the memory metric aggregation was skipped 8 times, but the CPU metric was skipped 11 times? I would argue not.

I'm actually toying with the idea that users with the platform partner role may want to know about skipped executions, I'm not sure regular user types should

[kathy-t]

* How frequently are executions skipped in our current data set? Is this a once in a blue moon sort of thing, or will it be fairly common? If it's reasonably common, I would be tempted to just have another column with the number.

Not frequently -- I don't think we have any skipped executions right now because we've been ingesting data for the platforms. Ideally, the webservice should not let any invalid data get in, which is improved with dockstore/dockstore#5822. So I think the chances are low that there are skipped executions because the webservice would throw an error first.

* Does a user need to know in detail that, for example, the memory metric aggregation was skipped 8 times, but the CPU metric was skipped 11 times? I would argue not.

I included it so the user knows how many executions were used to aggregate the metric. For example, an average over 100 executions is different from an average over 1 execution where the other 99 were skipped.

* What if we captured the total number of skipped aggregations at the top along with Total/Successful/Failed/Aborted? I guess the skipped is at different level though, a single execution could have more than one skipped, right?

Yeah, it's scoped to the metric level. So for a single execution, it could be skipped for multiple metrics.

[kathy-t]

Updated the PR so that the skipped executions icon is the info icon and only admins, curators, and platform partners can see it. Note that for a platform partner, they can see the warning for other platforms as well. I think this is okay since the warning could also exist at the ALL level and that platform's skipped execution count would be included there.

Also addressed Charles feedback:

Digression, I think the top could be improved; instead of repeating the word "Executions" 4 times, we could have it once.

See updated screenshots in description.

[sonarqubecloud]

Quality Gate failed

Failed conditions
20.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud