Proposal for efficient maintenance of contrib repo

[srprash]

Introduction

I'm opening this issue to discuss and derive concensus on processes and practices required to maintain the openetelemetry-dotnet-contrib repo efficiently. The scope of maintainance of the repo includes defining a structure for the repo and its components, assigning approvers and maintainers, establishing practices for code reviews, and outlining release process of components to nuget.

Goals

• Rigid structure for any contributed project under /src/ path. eg: List of approvers, Readme, Changelog, etc. This can be called out in the CONTRIBUTING.md
• Assign approvers for each sub folder under /src/ and /test/ folders, and maintainers for the contrib repo.
• The master branch of the contrib repo is building successfully at all the times.
• All the test projects under /test/ folder are running successfully and coverage report is generated for each PR submitted.
• Since the contrib repo has decoupled components as projects, the ability to release any particular project as a nuget package on demand.
• There is a proper channel for approvers to request the maintainers for a release.

Current state and limitations

• Single source of OpenTelemetry package version applied to all the projects in contrib repo. This hinders a single project to update the OpenTelemetry version independently.
• Single continuous build and release workflow. This workflow releases all the projects within the repository with the same version number.
• If any one of the project breaks, the complete build fails and none of the projects are built and released.
• Current maintainers may not be subject matter experts (SME) on all the components and therefore PR moves slow. This can discourage willing contributors from contributing new important features and improvements.

Proposed Changes & Processes

Managing responsibilities

• A set of approvers to be appointed to each of the components under /src/ and its corresponding tests within /test/ folders. These approvers are voluntary and are SMEs on the given component. The job of an approver is to address issues, review pull requests, and maintain a high bar for their components.
• A set of maintainers to be appointed for the comlete repo. The maintainer is responsible for maintaining the overall quality of the repo, merge in pull requests already approved by an approver, and do releases.
• If an issue is reported on the repo for a component, the maintainer should tag and engage the approver assigned for the component.
• If a PR is opened for a component, the relevant approver should review it in detail. Once the PR looks good, the approver should approve it, engage a maintainer to get their approval and merge the PR. No PR should be merged in without a maintainer's approval.
• Refer to the CODEOWNERS in opentelemetry-collector-contrib repo.

OpenTelemetry version targeting

• Each project should declare its dependency on a minimum version of OpenTelemetry package in its .csproj file. example:

  <PackageReference Include="OpenTelemetry" Version="1.0.0" />

• The OpenTelemetry package version declared in the root /build/Common.props should always be the latest. This way any project not specifying its own OpenTelemetry package dependency will always be built with the latest one.
  Caution: If there are breaking changes introduced in new version of OpenTelemetry, the depending component may break.

Continuous build

• Ideally, if each component targets its own OpenTelemetry version, the build for the complete solution should always be passing.
• The CI should be triggered on each PR and merge to master branch as it does today through a GH workflow for linux and windows.

Release on demand

• The ability to release a component is solely possessed by a maintainer of the repo.
• If a new version of a component needs to be released, the approver needs to do the following:
  • Update the CHANGELOG of the component.
  • Open an issue requesting the release. Mention the full component name (eg: OpenTelemetry.Contrib.Extensions.AWSXRay), the version number, and link to changelog.
  • Tag a maintainer.
• The maintainer will then review the issue and trigger the release workflow.
• The current release workflow does a nightly build, package, and upload to MyGet. This workflow can be modified to run on demand with inputs as the project name and version. The workflow will then build, test, package, and release just that component to Nuget.
• There is no need for a full contrib repo release. Why? 1. Not all components may have changes ready to release. 2. Not all components may follow same versioning strategy.

[srprash]

@cijothomas @SergeyKanzhelev @eddynaka @alolita @anuraaga @willarmiros
Feel free to include anyone else on this.

[anuraaga]

Since the contrib repo has decoupled components as projects, the ability to release any particular project as a nuget package on demand.

Each project should declare its dependency on a minimum version of OpenTelemetry package in its .csproj file. example:

Practices in .NET package management may be different, but from what I've seen in other languages, these would not be desired for a monorepo. To decouple builds, we'd generally have separate repositories, features like GitHub releases become confusing if parts of a repo can be released separately. So we'd expect the dependencies to all be shared and the full build to pass all the time - it works fine in java instrumentation and collector-contrib repo thanks to the efforts of the maintainers, and I presume it can happen here too.

[cijothomas]

If an issue is reported on the repo for a component, the maintainer should tag and engage the approver assigned for the component

This can be automatically done, by using issue templates as done in spec repo. https://github.com/open-telemetry/opentelemetry-specification/tree/master/.github/ISSUE_TEMPLATE

[cijothomas]

If a PR is opened for a component, the relevant approver should review it in detail. Once the PR looks good, the approver should approve it, engage a maintainer to get their approval and merge the PR. No PR should be merged in without a maintainer's approval.

Not sure what a maintainer is supposed to do to approve. Its already assumed that maintainers may not have enough expertise t mark approval. If approver approvers it, only job of maintainer should be to click the "merge" button.

[eddynaka]

If a PR is opened for a component, the relevant approver should review it in detail. Once the PR looks good, the approver should approve it, engage a maintainer to get their approval and merge the PR. No PR should be merged in without a maintainer's approval.

Not sure what a maintainer is supposed to do to approve. Its already assumed that maintainers may not have enough expertise t mark approval. If approver approvers it, only job of maintainer should be to click the "merge" button.

If that is the case, we could enable auto-merge and the maintainer would not need to click merge :)

[willarmiros]

From the Github docs:

The people you choose as code owners must have write permissions for the repository

This came up when discussing this at the JS sig. I'm not actually sure how narrowly you can scope "write permissions" on GitHub, but it seems like a CODEOWNER of an individual component would still need to get some kind of repo-wide access. IMO we can address this by making sure there is branch protection in place, and ensuring code owners who are not also maintainers have the lowest level of write access.

I'd be interested to hear how they tackle this in collector-contrib if @anuraaga or anyone else knows.

EDIT: It looks like this is how you can add an outside (non open-telemetry) member to the repo with write access and this describes exactly what write permission allows. It does not involve any ability to force-push code changes (assuming branch protection is in place) or do anything else destructive.

[srprash]

It does not involve any ability to force-push code changes (assuming branch protection is in place) or do anything else destructive.

Yeah, looks fine to me. The only thing which I'm a little concerned about is the ability to "Create, edit, run, re-run, and cancel GitHub Actions workflows". We would want to restrict release workflows to maintainers only.

[SergeyKanzhelev]

We can polish details as we go. I think the proposal is something that will allow us to scale what we have now in -contrib while collaboratively help each other. Next step as @anuraaga pointed is splitting repositories.

One question that still bothers me - will we be able to find owners for things like .NET Remoting. And if somebody will sign up to support it and then will stop, will we remove the component? Or have a "attic" of those in some separate repository when they degraded?

[srprash]

@SergeyKanzhelev Do you mean that there will be a repo for each instrumentation/extension/exporter component under opentelemetry org? That would surely guarantee continuous build and independent releases for each component but wouldn't that be too much to maintain? It may also make it difficult for users to find and/or contribute to the OS projects for instrumentations under opentelemetry.
I believe what @anuraaga suggests is that we maintain this monorepo and work together to make sure the repo build is always passing with the latest shared dependency like the openetelemetry SDK.

[anuraaga]

Discussed a bit with @srprash offline, to clarify, I think there are two reasonable options - a monorepo with everything, and they all build together, something that the maintainers would need to ensure keeps going for example by applying SDK dependency updates. Or a separate repo for each item to give them truly independent lifecycle which fits general GitHub practices. I strongly discourage a single repo with different folders released separately - it's too confusing to users to find many artifacts in the same place, but only some work with certain versions of OTel and not others for example.

@SergeyKanzhelev seems to be proposing going with the second option of separate repo per item. If this is what the dotnet maintainers want, then it's ok, and perhaps it's inline with a desire to have .net libraries instrumented natively instead of maintaining instrumentation out-of-band here. It's not as confusing for users as a single split repo as the repository will make clear its versioning, but it does have the potential issue of cruft repositories and a likely -attic organization to take them.

[SergeyKanzhelev]

@SergeyKanzhelev seems to be proposing going with the second option of separate repo per item

only long term when we will grow enough

[srprash]

Based on feedbacks from @anuraaga and @SergeyKanzhelev , I believe that the initial proposal of separate build workflows in the same repo is not desirable. So we have the following options:

1. Split the contrib repo into separate repos.

   • Each repo can host a single component or a set of components which are to be built and released together.
   • The repos can be named opentelemetry/opentelemetry-dotnet-contrib-<component-name>
   • The repos can choose to do releases at their own pace and with own semantic versioning.
   • To ensure continuous compatibility with the OTel SDK dependency, each repo must have a workflow that does a complete build on each commit of the core repo.
   • A list of active contrib repos can be maintained on the core repo page for users to find easily.
   • If a repo is not keeping up with the latest version of the OTel SDK, it may be designated as unmaintained.

2. Maintain the contrib repo as is but fix the components broken due to core changes.

   • This needs more number of maintainers who can focus on the core and the contrib repo equally.
   • The contrib repo should have a workflow which builds the contrib repo against the latest version of OTel SDK. For dotnet, this would mean making changes to the OTel SDK API which can then enable the upgrade of dependency in the contrib repo.
   • If a change in the core or contrib repo breaks the build, the maintainer should take up or assign people to fix the build as soon as possible.
   • The releases of the OTel SDK and the contrib can be synced up on the same versioning.

For the current state of the dotnet SDK, option 1 makes more sense as the contrib repo does not have enough dedicated maintainers to fix the broken components promptly (or until SDK GA). As mentioned by @anuraaga option 2 works well in Java and Collector where there hasn't been a major breaking changes between the core and the contrib repos and the has good maintianer support too.
If we decide to go with separate repos, @SergeyKanzhelev would you be able to help me set those up?

[alolita]

In my discussions with @srprash I think option 1 makes more sense for the .Net contrib components to ensure that contributed components are maintained regularly and not blocked when another component is not maintained anymore. The AWS team is interested in stepping up and taking more responsibilities to not only maintain the .Net AWS components but also help core maintainers with other .Net components, builds, releases, bug fixes. @cijothomas @SergeyKanzhelev please let us know how we can move forward.

[srprash]

@alolita In yesterday's SIG meeting, it was decided that for now we can have separate builds and releases for components in the contrib repo (as was the initial proposal) and as we scale we can look into splitting the repository into component based repos. I'll be working with @cijothomas and @SergeyKanzhelev towards workflows which can facilitate individual builds.
We understood that this approach is more focused towards maintainability of the repo and could cause some confusion with users as mentioned by @anuraaga in this comment. We would need good documentation in the release notes to provide the users as much clarity as possible.

[reyang]

Not sure what a maintainer is supposed to do to approve. Its already assumed that maintainers may not have enough expertise t mark approval. If approver approvers it, only job of maintainer should be to click the "merge" button.

We should be able to make it easier by combining the CODEOWNERS file with branch policy and auto-merge feature.

[srprash]

Closing this one out since we now have a working independent build structure.
A few things that we still need to do:

• A consice CONTRIBUTING doc for contributors and RELEASING doc for maintainers.
• Determine codeowners, approvers and maintainers for the projects.
• Automate the release workflows as much as possible.

Thanks everyone for your inputs on this!