Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): Support Unix epoch timestamps in links #5666

Merged
merged 6 commits into from
Apr 15, 2021

Conversation

dinever
Copy link
Member

@dinever dinever commented Apr 13, 2021

Fixes #5557

This PR enables us to use ${status.startedAtEpoch} and ${status.finishedAtEpoch} in Argo links.

This is very useful for users who want to link to a logging facility that only supports epoch timestamps as URL parameters.

For example, by setting the following link:

  links: |
    - name: Grafana
      scope: workflow
      url: https://play.grafana.org/d/000000012/grafana-play-home?namespace=${metadata.namespace}&workflowName=${metadata.name}&from=${status.startedAtEpoch}&to=${status.finishedAtEpoch}
    - name: DataDog
      scope: workflow
      url: https://app.datadoghq.com/logs?live=false&from_ts=${status.startedAtEpoch}&to_ts=${status.finishedAtEpoch}

We can link to a Grafana / DataDog dashboard by the timestamp.

Showcase:

timestamp

Docs:

Screen Shot 2021-04-15 at 3 32 29 PM

Would ❤️ to hear some feedback. Thanks!

@codecov
Copy link

codecov bot commented Apr 13, 2021

Codecov Report

Merging #5666 (feba601) into master (71dfc79) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head feba601 differs from pull request most recent head 815f3d3. Consider uploading reports for the commit 815f3d3 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5666      +/-   ##
==========================================
- Coverage   47.12%   47.10%   -0.02%     
==========================================
  Files         242      242              
  Lines       15135    15135              
==========================================
- Hits         7133     7130       -3     
- Misses       7095     7096       +1     
- Partials      907      909       +2     
Impacted Files Coverage Δ
workflow/controller/operator.go 70.84% <0.00%> (-0.27%) ⬇️
workflow/metrics/server.go 16.66% <0.00%> (+4.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71dfc79...815f3d3. Read the comment docs.

@alexec alexec changed the title feat(ui): Support expression evaludaiton in links feat(ui): Support expression evaluation in links Apr 13, 2021
ui/src/app/shared/utils.ts Outdated Show resolved Hide resolved
ui/src/app/shared/utils.ts Outdated Show resolved Hide resolved
Signed-off-by: Peixuan Ding <dingpeixuan911@gmail.com>
Signed-off-by: Peixuan Ding <dingpeixuan911@gmail.com>
# Adds a button to the workflow page but the timestamps are converted to Unix epoch time instead.
- name: Example Workflow Link
scope: workflow
url: "http://logging-facility?namespace=${metadata.namespace}&workflowName=${metadata.name}&startedAt=${status.startedAt | date: '%s'}&finishedAt=${ status.finishedAt | date: '%s' | default: 'now' }"
Copy link
Member Author

@dinever dinever Apr 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an example to demonstrate filters.

window.open(url, '_blank');
} else {
document.location.href = url;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like archived workflow links don't support opening with a new tab before. So fixed that.

Signed-off-by: Peixuan Ding <dingpeixuan911@gmail.com>
@@ -99,6 +100,7 @@ export const EventSourceLogsViewer = ({
}}
/>
)}
<Links scope='event-source-logs' object={eventSource} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like event source logs were not actually added. We have this in the example configmap tho. So have to fix this.

@@ -34,7 +34,7 @@ export const Links = ({scope, object, button}: {scope: string; object: {metadata
};

const openLink = (url: string) => {
if ((window.event as MouseEvent).ctrlKey) {
if ((window.event as MouseEvent).ctrlKey || (window.event as MouseEvent).metaKey) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In MacOS we have to use command + left click to open in a new tab.

docs/links.md Outdated
| `${status.startedAt}` | Start timestamp of the workflow/pod, in the format of `2021-01-01T10:35:56Z` |
| `${status.finishedAt}` | End timestamp of the workflow/pod, in the format of `2021-01-01T10:35:56Z`. If the workflow/pod is still running, this variable will be an empty string. |

> v3.1 and after
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see 3.1 branch created yet so I assume this change will go in 3.1 release (if merged)?

@dinever
Copy link
Member Author

dinever commented Apr 15, 2021

Thanks for the review @alexec!

I've done some investigation.
Seems like there isn't a way to limit the scope of eval or Function() and make it safe in JavaScript.
So I looked for some template engine that we can use. The following are the ones don't work:

  • mustache: doesn’t support utility functions/filters
  • ejs: doesn’t support custom tags like ${...}
  • dotjs: context is only accessible under it variable, which is not backward compatible with what we have right now
  • handlerbars: doesn’t support custom tags like ${...}
  • nunjucks: not safe to use due to no sandbox execution

Luckily, I found LiquidJS which is safe and lightweight with useful built-in filters like date and default.

For example, to convert startedAt to epoch, we can do:

${ status.startedAt | date: '%s' }

I've pushed all my changes. Let me know what you think. Thanks!

@dinever dinever force-pushed the fix/5557 branch 2 times, most recently from 7f1cae2 to e3af497 Compare April 15, 2021 05:53
Signed-off-by: Peixuan Ding <dingpeixuan911@gmail.com>
ui/yarn.lock Outdated Show resolved Hide resolved
Signed-off-by: Peixuan Ding <dingpeixuan911@gmail.com>
Signed-off-by: Peixuan Ding <dingpeixuan911@gmail.com>
@alexec alexec merged commit a018523 into argoproj:master Apr 15, 2021
@alexec alexec added this to the v3.1 milestone Apr 15, 2021
@alexec
Copy link
Contributor

alexec commented Apr 15, 2021

Great stuff! 🚢

@dinever dinever deleted the fix/5557 branch April 15, 2021 18:38
@dinever dinever changed the title feat(ui): Support expression evaluation in links feat(ui): Support Unix epoch timestamps in links Apr 15, 2021
@simster7 simster7 mentioned this pull request Apr 19, 2021
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for epoch time in links
2 participants