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.

Sign up for GitHub

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

Jump to bottom

Add codeowners tooling to this repository #11562

Closed
4 tasks
mx-psi opened this issue Oct 29, 2024 · 14 comments · Fixed by #11739
Closed
4 tasks

Add codeowners tooling to this repository #11562

mx-psi opened this issue Oct 29, 2024 · 14 comments · Fixed by #11739
Labels
ci-cd CI, CD, testing, build issues help wanted Good issue for contributors to OpenTelemetry Service to pick up

Comments

@mx-psi
Copy link
Member

mx-psi commented Oct 29, 2024

We have some tooling on the opentelemetry-collector-contrib that allows us to automate some actions related to codeowners:

We can add the tooling one by one. Some of these may need additional tooling (more issue templates?):

  • Ping code owners on PRs for a component
  • Ping code owners on new issues
  • Ping code owners on existing issues
  • Add code owners as reviewers in a PR
@mx-psi mx-psi added help wanted Good issue for contributors to OpenTelemetry Service to pick up ci-cd CI, CD, testing, build issues labels Oct 29, 2024
@mx-psi mx-psi mentioned this issue Oct 29, 2024
Merged
@mowies
Copy link
Member

mowies commented Nov 25, 2024

i'd like to work on this :)

@mowies
Copy link
Member

mowies commented Nov 25, 2024 •
edited
Loading

@mx-psi so after a quick review of the 4 links above, I noticed that probably only the issue related things will really work on this repo. That's because the codeowners file is not set up as granular here compared to the contrib repo so the PR related workflows, that would ping codeowners for specific folders would just generate a ton of notifications for the collector maintainers.

Update: the * is not specifically respected by the automations so if a specific component is not found in the codeowners file, no notification will be sent and the * will be ignored

@mowies mowies mentioned this issue Nov 25, 2024
Merged
@mx-psi
Copy link
Member Author

mx-psi commented Nov 25, 2024

@mowies That's fair. There will be a long transition period in which a lot of things do not have a codeowner. I think we should ignore the * line and only trigger automation if we are on a more specific line.

I am working on a PR that should be ready on Wednesday adding more specific codeowners, maybe after that is ready it's a good time to see how this would work in practice

@mowies
Copy link
Member

mowies commented Nov 26, 2024

The automations only look for specific labels as components anyways, so they already ignore the * case.

@mowies
Copy link
Member

mowies commented Nov 26, 2024

For some of the automations to work, I would also need to adjust the existing issue templates and probably mostly exchange them for the ones from the contrib repo. Should I add that to this PR or rather create a separate one?

@mx-psi
Copy link
Member Author

mx-psi commented Nov 26, 2024

The automations only look for specific labels as components anyways, so they already ignore the * case.

Ah, goes to show how little I know about this 😅 I don't understand the notifications issue you mention above. Could you explain with an example where we would get a lot of notifications?

Should I add that to this PR or rather create a separate one?

Let's create a separate one for this

@mowies
Copy link
Member

mowies commented Nov 26, 2024

I thought at first, that the * in the codeowners file of this repo would be respected by the automations and since that's (almost) the only entry in the file, that would trigger as a fallback for all components that are not specifically listed, but that's not actually the case after digging a little deeper. Instead, if the specific component is not found in the codeowners file, nobody will get notified. So that's good I guess in terms of spam 😅

@mx-psi
Copy link
Member Author

mx-psi commented Nov 26, 2024

Makes sense! Then yeah, the tooling will not be super useful until we have a more fleshed out list of codeowners, but it will be good as a start.

@mowies
Copy link
Member

mowies commented Nov 26, 2024

Yeah I think it's still good to have them in place. As soon as the issue templates are updated, we at least get labels for components, which is already something 😄
And since no notification spam will occur, I can also go ahead and add the PR related automations as well I think.

@mowies
Copy link
Member

mowies commented Nov 26, 2024 •
edited
Loading

some more ideas around this that i will extend over the course of implementing this:

  • i think a few tools and scripts could be moved to a central repo like https://github.com/open-telemetry/opentelemetry-go-build-tools so that they can be reused and don't need to be copy/pasted all over the place (i'm copying lots of things from contrib into core right now. for example pipelines, githubgen tool)
  • most of the automations that i'm adding now, are written in bash. i think it would make sense to rewrite those in a more accessible language like typescript or so while moving them to a central location
  • the githubgen tool really needs an exclusion list so that things like the test file here don't surface as errors (user some that exists but is completely unrelated to this and otel in general)
  • ...

EDIT:
filed an issue to move githubgen to the go-build-tools repo: open-telemetry/opentelemetry-collector-contrib#36691

@dmathieu
Copy link
Member

dmathieu commented Nov 26, 2024 •
edited
Loading

i think a few tools and scripts could be moved to a central repo

otel-go-contrib is also copying some of the scripts from collector-contrib, so that repository would be interested in that too.

@mowies mowies mentioned this issue Nov 27, 2024
Merged
@mowies
Copy link
Member

mowies commented Dec 4, 2024

first PR for this is ready for review: #11756

@mowies
Copy link
Member

mowies commented Dec 5, 2024 •
edited
Loading

@dmathieu could you please list a few tools that you'd like to see moved?
I'm currently creating an issue in otel-collector-contrib to move githubgen and I guess I could also create a parent issue to move more tools.

EDIT: issue to move githubgen: open-telemetry/opentelemetry-collector-contrib#36691

@dmathieu
Copy link
Member

dmathieu commented Dec 5, 2024

Right now, we're using request codeowners review:
https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/.github/workflows/request_codeowners_review.yml
Pinging codeowners on issues is also something we're interested in though.

github-merge-queue bot pushed a commit that referenced this issue Feb 6, 2025
@mowies
[chore] add githubgen to enhance codeowners and issue templates (#11756)
43087bd 
### This PR
- adds the githubgen tool as a dependency in internal/tools
- uses githubgen to generate codeowners and issue template files
- updates lots of metadata files by
- taking the existing codeowners file and feeding the info from there
back into the component metadata.yaml files or creating new
metadata.yaml files where none existed yet
- adds distributions.yaml as a basis the mostly already existing
`distributions:` keys in metadata.yaml files (needed for githubgen to
work correctly)
- adds relevant make commands to make the githubgen tool usage mostly
transparent to users

This change is a prerequisite to be able to ping codeowners reliably
with automated tooling as a next step.


Part of #11562

---------

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
github-merge-queue bot pushed a commit that referenced this issue Feb 12, 2025
@mowies
[chore] add automation for codeowner tooling (#11739)
d668e74 
This PR copies over automation from the collector-contrib repo and
slightly adjusts it where needed.
The PR includes 3 new workflows:
- add-labels: enables users to add labels to issues by commenting
`/label <the-label>` (although github org and other permissions apply)
- ping-codeowners-issues: ping codeowners of a component if the label
for that component was added to an issue
- ping-codeowners-on-new-issue: search the issue text for components and
add labels accordingly, then ping code owners


Fixes #11562

### Tests (done on my personal fork)
#### Adding/removing labels via comment
mowies#6

#### Pinging codeowners on new issues
mowies#12

#### Pinging codeowners when component label is added to issue
mowies#11

#### Adding codeowners to new PRs
only visible in workflow logs because other user is not a collaborator
of the repo, but the user would have been added

https://github.com/mowies/opentelemetry-collector/actions/runs/13266017136/job/37033580832

#### Pinging codeowners when component label is added to PR
only visible in workflow logs because other user is not a collaborator
of the repo, but the user would have been pinged

https://github.com/mowies/opentelemetry-collector/actions/runs/13264177723/job/37027389440?pr=13

---------

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ci-cd CI, CD, testing, build issues help wanted Good issue for contributors to OpenTelemetry Service to pick up
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[chore] add automation for codeowner tooling dynatrace-oss-contrib/opentelemetry-collector
3 participants
@dmathieu @mx-psi @mowies