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

refactor for dispatch style #21

Merged
merged 38 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
64cd994
refactor for dispatch style
msarahan Nov 5, 2024
752e336
plumb extra OTEL_RESOURCE_ATTRIBUTES through summarize
msarahan Nov 8, 2024
6182306
set default value for endpoint and protocol
msarahan Nov 8, 2024
961c10f
set defaults for SHARED_ACTIONS env vars
msarahan Nov 8, 2024
36c6347
re-export when stashing so that defaults take effect
msarahan Nov 8, 2024
881a7f8
move default shared actions repo to dispatch script
msarahan Nov 8, 2024
071b9bc
add default shared-actions repo and ref to last dispatch script
msarahan Nov 8, 2024
043229c
set default branch to telemetry-dispatch-actions in dispatch for now
msarahan Nov 8, 2024
d9ff2b9
rename base-env-vars to telemetry-env-vars for clarity of purpose
msarahan Nov 8, 2024
e638f91
syntax in setting attributes
msarahan Nov 8, 2024
31dbbe7
semicolon all the things
msarahan Nov 8, 2024
a5f9e34
fix default protocol
msarahan Nov 8, 2024
d85c7f9
replace cat | jq with plain file reads
msarahan Nov 8, 2024
d64dd46
trim leading comma from attributes
msarahan Nov 8, 2024
eaa996d
debugging output
msarahan Nov 8, 2024
a313c3f
shouldn't remove traceparent for job in summarize
msarahan Nov 8, 2024
4a974d7
parent span id for top-level traceparent
msarahan Nov 8, 2024
8a83c40
fix logic on top-level traceparent
msarahan Nov 8, 2024
3f7ceee
add start time to root workflow
msarahan Nov 11, 2024
8d7b889
trim leading comma from env vars when loading
msarahan Nov 11, 2024
5cc031f
use bash to remove leading commas
msarahan Nov 11, 2024
0d8f149
use bash to remove leading commas
msarahan Nov 11, 2024
4f01885
give up on bash and use sed
msarahan Nov 11, 2024
03a3140
tweak time formatting and debug
msarahan Nov 11, 2024
c702cfb
fix invalid braces in attributes
msarahan Nov 11, 2024
b0e6ea6
force empty parent span id for workflow root
msarahan Nov 11, 2024
8e98e7e
create root spans at start of summarize
msarahan Nov 12, 2024
b920f55
unset TRACEPARENT when setting root spans
msarahan Nov 12, 2024
8feefcc
comment root workflow start delay for now
msarahan Nov 12, 2024
2ac444d
Apply suggestions from code review
msarahan Nov 13, 2024
4a44423
upload job info if debug mode set
msarahan Nov 13, 2024
888f577
Update .github/workflows/test-telemetry-setup.yaml
msarahan Nov 13, 2024
25c4ea4
Update .github/workflows/test-telemetry-setup.yaml
msarahan Nov 13, 2024
fe2924b
remove test
msarahan Nov 13, 2024
59fc7ad
Merge branch 'telemetry-dispatch-actions' of github.com:rapidsai/shar…
msarahan Nov 13, 2024
559cbbc
improve name/description
msarahan Nov 13, 2024
7d3476e
remove debugging prints
msarahan Nov 13, 2024
955e274
comment about certs secrets not being set
msarahan Nov 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 0 additions & 51 deletions .github/workflows/test-telemetry-setup.yaml

This file was deleted.

15 changes: 15 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
# Copyright (c) 2024, NVIDIA CORPORATION.

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: trailing-whitespace
- id: check-added-large-files
- id: check-yaml
- id: end-of-file-fixer
- repo: https://github.com/rhysd/actionlint
rev: v1.7.4
hooks:
- id: actionlint-docker
99 changes: 68 additions & 31 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,82 @@
Contains all of the shared composite actions used by RAPIDS.

Actions that refer to each other assume that they have been checked out to the
./shared-actions folder. This assumption is what allow code reuse between
actions. Your general usage pattern for using these actions in other repos
should be:
./shared-actions folder. This *should* be the root of the GitHub Actions workspace.
This assumption is what allow code reuse between actions.

In general, we should try to never call "implementation actions" here. Instead,
we should prefer to create "dispatch actions" that clone shared-actions from a particular repo
at a particular ref, and then dispatch to an implementation action from that repo.
This adds complexity, but has other advantages:

* simplifies specifying a custom branch for actions for development and testing
* changes all shared-actions calls in a workflow at once, instead of changing each one
* allows reuse of shared-actions within the shared-actions repo. Trying to use these
without the clone and relative path would not otherwise keep the repo and ref
consistent, leading to great confusion over why changes aren't being reflected.

## Example dispatch action

```yaml
name: 'Example dispatch action'
description: |
The purpose of this wrapper is to keep it easy for external consumers to switch branches of
the shared-actions repo when they are changing something about shared-actions and need to test it
in their pipelines.

Inputs here are all assumed to be env vars set outside of this script.
Set them in your main repo's workflows.

runs:
using: 'composite'
steps:
- name: Clone shared-actions repo
uses: actions/checkout@v4
with:
repository: ${{ env.SHARED_ACTIONS_REPO}}
ref: ${{ env.SHARED_ACTIONS_REF}}
path: ./shared-actions
- name: Stash base env vars
uses: ./shared-actions/_stash-base-env-vars
```
...

In this action, the "implementation action" is the
`./shared-actions/_stash-base-env-vars`. You can have inputs in your
dispatch actions. You would just pass them through to the implementation action.
Environment variables do carry through from the parent workflow through the
dispatch action, into the implemetation action. In most cases, it is simpler
(though less explicit) to set environment variables instead of plumbing inputs
through each action.

Environment variables are hard-coded, not detected. If you want to pass a different
environment variable through, you need to add it to implementation stash action,
like `telemetry-impls/stash-base-env-vars/action.yml`. You do not need to
explicitly specify it on the loading side.

## Implementation action

These are similar to dispatch actions, except that they should not clone
shared-actions. They can depend on other actions from the shared-actions
repository using the `./shared-actions` relative path.

## Example calling workflow

The key detail here is that the presence of the SHARED_ACTIONS_REPO and/or
SHARED_ACTIONS_REF environment variables is what changes the shared-actions
dispatch. The `uses` line should not change.

```yaml
env:
SHARED_ACTIONS_REF: 'main'
# Change these in PRs
SHARED_ACTIONS_REPO: some-fork/shared-actions
SHARED_ACTIONS_REF: some-custom-branch

jobs:
actions-user:
runs-on: ubuntu-latest
steps:
- name: Checkout actions
uses: actions/checkout@v4
with:
repository: rapidsai/shared-actions
ref: ${{env.SHARED_ACTIONS_REF}}
path: ./shared-actions
- name: run script
uses: ./shared-actions/some-script-folder-name
with:
blah: yes
```

Instead of something like:

```
- name: Telemetry setup
id: telemetry-setup
uses: rapidsai/shared-actions/telemetry-traceparent@add-telemetry
```

This latter syntax is difficult because the branch info does not cascade
recursively into any checkouts that might be done in an action, and also because
this syntax does not support actions calling other actions with relative paths.

Note that the cloning/checkout order matters! The actions/checkout action wipes
the destination before cloning into it. That means that if you clone the shared-
actions repo in a folder, then clone the main repo without a path, the shared-
actions folder will be removed when you go looking for it. See https://github.com/actions/checkout/issues/1525#issuecomment-2076363261
# DO NOT change this in PRs
uses: rapidsai/shared-actions/dispatch-script@main
```
52 changes: 0 additions & 52 deletions github-actions-job-info/example-gha-job-log.json

This file was deleted.

57 changes: 0 additions & 57 deletions telemetry-create-span/action.yml

This file was deleted.

29 changes: 29 additions & 0 deletions telemetry-dispatch-load-base-env-vars/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: dispatch-load-base-env-vars
description: |
Wrapper that clones a specific branch/ref of the shared-actions repo, then
calls the `load-base-env-vars` action to download the base environment
variables and load them into the current environment.

This does not overwrite any environment variables that are already set.
inputs:
load_service_name:
description: |
If true, loads OTEL_SERVICE_NAME from the stashed env vars. This is used for top-level workflows.
Otherwise the telemetry service name is obtained from Github job metadata.
Getting the service name from Github job metadata is for child workflows.
default: 'false'

runs:
using: 'composite'
steps:
- name: Clone shared-actions repo
uses: actions/checkout@v4
with:
repository: ${{ env.SHARED_ACTIONS_REPO || 'rapidsai/shared-actions' }}
ref: ${{ env.SHARED_ACTIONS_REF || 'telemetry-dispatch-actions' }}
path: ./shared-actions
- name: Set OTEL_SERVICE_NAME from job if not loading from stash
if: ${{ inputs.load_service_name != 'true' }}
uses: ./shared-actions/telemetry-impls/set-otel-service-name
- name: Load base env vars
uses: ./shared-actions/telemetry-impls/load-base-env-vars
22 changes: 22 additions & 0 deletions telemetry-dispatch-stash-base-env-vars/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: dispatch-stash-base-env-vars
description: |
Clones a particular branch/ref of a shared-actions repo, then
call the stash-base-env-vars implementation script, which writes
some environment variables so that downstream jobs can refer to them.

Inputs here are all assumed to be env vars set outside of this script.
Set them in your main repo's workflows.

runs:
using: 'composite'
steps:
- name: Clone shared-actions repo
uses: actions/checkout@v4
with:
repository: ${{ env.SHARED_ACTIONS_REPO || 'rapidsai/shared-actions' }}
ref: ${{ env.SHARED_ACTIONS_REF || 'telemetry-dispatch-actions' }}
path: ./shared-actions
- name: Get traceparent representation of current workflow
uses: ./shared-actions/telemetry-impls/traceparent
- name: Stash base env vars
uses: ./shared-actions/telemetry-impls/stash-base-env-vars
31 changes: 31 additions & 0 deletions telemetry-dispatch-write-summary/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: dispatch-summarize
description: |
Clones a particular branch/ref of a shared-actions repo, then calls its telemetry summarize
action. The summarize action downloads and parses Github job metadata, and creates
OpenTelemetry spans from the job metadata. These are sent to the configured OTLP receiver/endpoint.
inputs:
cert_concat:
description: Concatenation of certs (CA;Client;ClientKey)
extra_attributes:
description:
Additional attributes to add to OTEL_RESOURCE_ATTRIBUTES.
See https://opentelemetry.io/docs/languages/sdk-configuration/general/#otel_resource_attributes

runs:
using: 'composite'
steps:
- name: Clone shared-actions repo
uses: actions/checkout@v4
with:
repository: ${{ env.SHARED_ACTIONS_REPO || 'rapidsai/shared-actions' }}
ref: ${{ env.SHARED_ACTIONS_REF || 'telemetry-dispatch-actions' }}
path: ./shared-actions
# This is necessary because this action will generally be run in a job separately from
# where the env vars are set
- name: Load base environment variables
uses: ./shared-actions/telemetry-impls/load-base-env-vars
- name: Run summarize action
uses: ./shared-actions/telemetry-impls/summarize
with:
cert_concat: ${{ inputs.cert_concat }}
extra_attributes: ${{ inputs.extra_attributes }}
Loading