Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Add support for using task execution ID fields in log URI templates #372

Merged
merged 23 commits into from
Jul 15, 2023

Conversation

jeevb
Copy link
Contributor

@jeevb jeevb commented Jul 13, 2023

This PR adds a new templating scheme, TaskExecution, that supports the following task execution metadata variables in the log link templates:

{{ .taskID }}
{{ .taskVersion }}
{{ .taskProject }}
{{ .taskDomain }}
{{ .taskRetryAttempt }}
{{ .nodeID }}
{{ .executionName }}
{{ .executionProject }}
{{ .executionDomain }}
{{ .subtaskExecutionIndex }}
{{ .subtaskParentName }}
{{ .subtaskRetryAttempt }}
{{ .subtaskParentRetryAttempt }}

Usage:

plugins:
  logs:
    kubernetes-enabled: false
    templates:
      - displayName: Task Logs
        templateUris:
          - https://flyte.corp.net/console/projects/{{.executionProject}}/domains/{{.executionDomain}}/executions/{{.executionName}}/nodeId/{{.nodeID}}/taskId/{{.taskID}}/attempt/{{.taskRetryAttempt}}/view/logs
        messageFormat: 2
        scheme: TaskExecution
  k8s-array:
    logs:
      config:
        kubernetes-enabled: false
        templates:
          - displayName: Task Logs
            templateUris:
              - https://flyte.corp.net/console/projects/{{.executionProject}}/domains/{{.executionDomain}}/executions/{{.executionName}}/nodeId/{{.nodeID}}/taskId/{{.taskID}}/attempt/{{ .subtaskParentRetryAttempt }}/mappedIndex/{{ .subtaskExecutionIndex }}/mappedAttempt/{{ .subtaskRetryAttempt }}/view/logs
            messageFormat: 2
            scheme: TaskExecution

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Merging #372 (d9c71de) into master (3d2525a) will increase coverage by 1.31%.
The diff coverage is 82.86%.

❗ Current head d9c71de differs from pull request most recent head 06b5e94. Consider uploading reports for the commit 06b5e94 to get more accurate results

@@            Coverage Diff             @@
##           master     #372      +/-   ##
==========================================
+ Coverage   62.92%   64.23%   +1.31%     
==========================================
  Files         153      154       +1     
  Lines       12901    10570    -2331     
==========================================
- Hits         8118     6790    -1328     
+ Misses       4170     3167    -1003     
  Partials      613      613              
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
go/tasks/logs/config.go 0.00% <ø> (ø)
...s/pluginmachinery/tasklog/templatescheme_enumer.go 0.00% <0.00%> (ø)
go/tasks/logs/logging_utils.go 89.70% <100.00%> (+1.47%) ⬆️
go/tasks/pluginmachinery/tasklog/template.go 98.60% <100.00%> (+1.30%) ⬆️
go/tasks/plugins/array/k8s/subtask.go 30.35% <100.00%> (-0.14%) ⬇️
go/tasks/plugins/array/k8s/subtask_exec_context.go 83.18% <100.00%> (+3.98%) ⬆️
go/tasks/plugins/k8s/dask/dask.go 86.12% <100.00%> (+3.24%) ⬆️
.../plugins/k8s/kfoperators/common/common_operator.go 70.06% <100.00%> (+6.12%) ⬆️
go/tasks/plugins/k8s/kfoperators/mpi/mpi.go 75.00% <100.00%> (+2.42%) ⬆️
...o/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go 79.86% <100.00%> (+1.17%) ⬆️
... and 3 more

... and 123 files with indirect coverage changes

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
@jeevb jeevb marked this pull request as ready for review July 14, 2023 20:36
@jeevb jeevb merged commit 7ddd993 into master Jul 15, 2023
6 of 7 checks passed
@jeevb jeevb deleted the jeev/tasklog-tmpl-vars branch July 15, 2023 00:10
@fg91
Copy link
Member

fg91 commented Jul 19, 2023

Nice, thanks, very useful :)
Could you please document these new variables here?

eapolinario pushed a commit that referenced this pull request Sep 6, 2023
…372)

* Refactor task log templates to support extra vars

* plumbing for k8s

* Cleanup use of providers

* cleanup

* more cleanups

* more cleanups

* more cleanups

* Plumb through task context for plugins

* fix

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>

* move task execution identifier into Input struct

* use pointer

* tests

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>

* fix linting

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>

* cleanups

* fix

* revert to using regex replacements for performance

* Readd benchmark

* Split templating scheme (#373)

* Use a split templating scheme

* add enum for TemplateScheme

* cleanup comment

* fix linting issues

* add unit tests for templateVarsForScheme

* update for consistency

---------

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants