open-telemetry
[chore] RFC: Collector releases approvers group
Description
Proposal for creating a new collector-release-approvers group.
Announced at:
- #otel-collector-dev on 2024-10-30: https://cloud-native.slack.com/archives/C07CCCMRXBK/p1730307025302339
- Collector SIG on 2024-11-05 (TBD)
The stakeholders for this PR are:
- @open-telemetry/collector-approvers
- @open-telemetry/collector-contrib-approvers
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 91.67%. Comparing base (
c14d4f9) to head (92599da). Report is 38 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #11577 +/- ##
=======================================
Coverage 91.67% 91.67%
=======================================
Files 461 461
Lines 24749 24749
=======================================
Hits 22689 22689
Misses 1678 1678
Partials 382 382
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'd be interested to participate in this group, given my recent experience wrangling the pipelines in opentelemetry-collector-releases repo
Why is the builder there?
I see the builder as part of the pipeline to produce the artifacts, I think it fits naturally here. I am happy to take it off if it helps move this proposal forward, I think the important bit is to test this out and see if it helps retain more contributors.
I don't think we should have our own "approvers" with different rules, so better to have as a SIG directly.
Why not? We already have multiple sets of approvers within the Collector SIG (core approvers and contrib approvers). Is the concern not having maintainers as well?
Outside of the Collector SIG, we also have precedent on the semantic-conventions repository (e.g. the container semconv approvers is a separate approvers group that does not have a separate SIG or WG) and on the operator we used to have the target allocator handled separately IIRC
I see the builder as part of the pipeline to produce the artifacts, I think it fits naturally here. I am happy to take it off if it helps move this proposal forward, I think the important bit is to test this out and see if it helps retain more contributors.
I may miss something here. How does this "retain more contributors"?
Why not? We already have multiple sets of approvers within the Collector SIG (core approvers and contrib approvers). Is the concern not having maintainers as well?
The contrib is like a SIG has all the roles not just approvers. So if you want the release repo to have its set of approvers/maintainers I am happy with that.
I may miss something here. How does this "retain more contributors"?
See the rationale section:
Release engineering requires a different set of skills and interests than developing the Collector codebase. As such, the set of contributors for the Collector releases has overlap with but is different from the set of contributors for the Collector codebase. We are missing out on retaining people who are more interested in these aspects
This is my primary goal here: to help support a community of people around Collector releases that is not necessarily involved in the rest of the Collector.
The contrib is like a SIG has all the roles not just approvers. So if you want the release repo to have its set of approvers/maintainers I am happy with that.
What are your concerns with not having maintainers? Do you think the semantic conventions repository has any downsides? Also, could you make a more specific proposal about maintainers? Would the initial set be the contrib maintainers?
What are your concerns with not having maintainers? Do you think the semantic conventions repository has any downsides?
I don't know if they have or not, but they don't follow the community rules (or at least the standard rules). I am not ok to break that, unless GC will say that it is ok to have it. Also does that approver role match with the https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md#approver? If yes, do we have same requirements? If no what are the requirements?
I also see that you added a responsibilities section, which is not exactly like https://github.com/open-telemetry/community/blob/main/guides/contributor/membership.md#responsibilities-and-privileges-2, so what is the difference?
Also, could you make a more specific proposal about maintainers? Would the initial set be the contrib maintainers?
Happy to start with who is now maintainer to continue to be.
I don't know if they have or not, but they don't follow the community rules (or at least the standard rules). I am not ok to break that, unless GC will say that it is ok to have it
Could you point out what rule we would be breaking if we were to follow this proposal? I am happy to bring this up on the community repository. I just thought of another example of this pattern: the localization approvers for the different languages in opentelemetry.io, which also don't have a separate SIG/WG.
Also does that approver role match with the open-telemetry/community@main/guides/contributor/membership.md#approver? If yes, do we have same requirements? If no what are the requirements?
Yes, this would be the same requirements, but limited to the opentelemetry-collector-releases repository.
I also see that you added a responsibilities section, which is not exactly like open-telemetry/community@main/guides/contributor/membership.md#responsibilities-and-privileges-2, so what is the difference?
The responsibilities of the new group would that of being approvers on the opentelemetry-collector-releases repository. They would also be listed as codeowners for two components. This is a common situation, for example:
- The opentelemetry-collector approvers are listed as code owners for the examples folder in the opentelemetry-collector-contrib repository
- The Helm chart and Operator approvers are listed as code owners for the k8s distro folder in the opentelemetry-collector-releases repository
- The Go instrumentation approvers are listed as the owners of instrgen in opentelemetry-go-contrib
All these groups are approvers, and they also are code owners of a different folders. As to what code owner means in this repository, this is what #11557 tries to make crystal clear, I am happy to wait until that PR is merged to move forward with this one.
I made the following changes:
- Moved the 'Rationale' section to the top and renamed as 'Problem statement'
- Added a 'Prior art' section documenting what we talked about above about this pattern existing before
- Dropped the proposal for this team to own OCB for now
- Clarified responsibilities section to make sure it is clear what is in scope (approvers of -releases and code owners releases workflows)
@bogdandrutu Could you double check and see if https://github.com/open-telemetry/opentelemetry-collector/pull/11577#issuecomment-2450667089 still applies? Is there anything to clarify on the community repo or is everything clear now?
It does seem like a natural fit to have ocb under the purview of the new releases group(s) since it is used to release the distributions and images published under the releases repo, but I agree that it doesn't need to happen just yet.
This PR was marked stale due to lack of activity. It will be closed in 14 days.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'd be happy to take part in this group if looking for additional approvers.
I raised open-telemetry/community/pull/2480 to try to unblock this PR by explicitly clarifying this pattern is okay
@bogdandrutu We discussed and merged https://github.com/open-telemetry/community/pull/2480 on the last GC call. Could you take a look and let me know if your concerns are addressed?
This PR was marked stale due to lack of activity. It will be closed in 14 days.
@bogdandrutu can you take a look and see if your request for change has been addressed
This is entering final comment period, I will merge this on or after 2025-01-23 if there are no further comments