Feat: Add pod start and finish time in RFC3339 time format to logging link templating variables #minor

[fg91]

TL;DR

Currently, the the pod start and finish time in unix time format (seconds since 1970) can be used to template logging links.
This time format cannot be used in combination with google cloud logging. Instead, GCP logging requires timestamps in RFC3339 format:

timestamp >= "2016-11-29T23:00:00Z"
timestamp <= "2016-11-29T23:30:00Z"

This PR adds the pod start and finish time in this format to the variables available for templating logging links.

Type

• Bug Fix
• Feature
• Plugin

Are all requirements met?

• Code completed
• Smoke tested
• Unit tests added
• Code documentation added: Feat: Add pod start and finish time in RFC3339 time format to logging link templating variables flytesnacks#998
• Any pending items have an associated Issue

Complete description

NA

Tracking Issue

NA

Follow-up issue

NA

[codecov]

Codecov Report

Merging #360 (0021c97) into master (2a7a6f9) will increase coverage by 1.37%.
The diff coverage is 100.00%.

❗ Current head 0021c97 differs from pull request most recent head 6382a66. Consider uploading reports for the commit 6382a66 to get more accurate results

@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
+ Coverage   62.62%   64.00%   +1.37%     
==========================================
  Files         152      152              
  Lines       12789    10375    -2414     
==========================================
- Hits         8009     6640    -1369     
+ Misses       4168     3123    -1045     
  Partials      612      612              

Flag	Coverage Δ
unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
go/tasks/logs/logging_utils.go	90.00% <100.00%> (+2.50%)	⬆️
go/tasks/pluginmachinery/tasklog/template.go	97.80% <100.00%> (+0.83%)	⬆️

... and 131 files with indirect coverage changes

[hamersaw]

Fantastic! Should be pretty easy to add ISO 8601 format as well right? Would wrap up this issue.

[fg91]

Fantastic! Should be pretty easy to add ISO 8601 format as well right? Would wrap up this issue.

Unfortunately, go appears to not have the time.ISO8601 equivalent to time.RFC33339, see here.

Users on stackoverflow recommend to use rfc3339 as a proxy for 8601 (see reply to this answer):

From what I could tell, RFC3339 is a stricter version of ISO 8601. So it is probably safe to use the RFC format if a system expects the ISO.

However, this answer suggests that not everything that is allowed by RFC3339 is allowed by ISO 8601 and vice versa.

That being said, a RFC3339 timestamp propeller generates based on this PR is e.g. 2023-06-13T18:28:16Z. According to this ISO 8601 timestamp format validator (first google result), it is a match.

So it seems that time.Unix(startTime, 0).Format(time.RFC3339) doesn't use any peculiarities that ISO 8601 doesn't allow.

We could use our own ISO8601 format string (see here) even though I couldn't confirm this format string is correct.

Alternatively, we could leave it with RFC3339 and in the documentation explain that it is valid also for ISO 8601. This would be my suggestion. What do you think @hamersaw ?

[hamersaw]

Alternatively, we could leave it with RFC3339 and in the documentation explain that it is valid also for ISO 8601. This would be my suggestion.

Sounds great to me! Thanks for the depth.