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

Proposal: Non-core components like Exporters should live in contrib repos #142

Open
alolita opened this issue Nov 25, 2020 · 12 comments
Open
Assignees
Labels
release:required-for-ga Must be resolved before GA release, or nice to have before GA

Comments

@alolita
Copy link
Member

alolita commented Nov 25, 2020

Problem
As OpenTelemetry continues to grow, the number of PRs is expected to exceed the capacity for maintainers to provide timely code reviews. The maintainers are already spread thin across the language-core and language-contrib repos for their SDKs. This increases the backlog of pending code reviews. In addition, maintainers are typically focused on achieving feature completion and source stability and often de-prioritize review of non-core components.

Solution
We propose that non-core components, such as exporters, be moved into contrib repos so that maintainers for the core components are not overburdened and so that other developers can become maintainers for the non-core code. In addition to addressing maintainability, this solution helps make building and releasing artifacts easier by decoupling from core builds and schedules. For example, if we move the Prometheus exporters from the core sdk repos into the contrib repos they can be maintained by other developers without being impeded by core-sdk maintainer schedules and concerns. Furthermore, developers who do not have maintainer rights on the core repos can then be allowed to help maintain non-core components in the contrib repos. See table below for proposed relocations of Prometheus exporters from core to contrib repos.

Note: This issue has been discussed in the maintainer, language and Collector SIG meetings and has been generally agreed to. The intent of this issue is for formally recognize this solution going forward.

Proposed relocations of Prometheus exporters in OpenTelemetry

SDK Prometheus Exporter Type Current location Proposed location
C++ Pull Core Contrib
Python Pull Core Contrib
JavaScript Pull Core Contrib
Java Pull Core Contrib
Go Pull Core Contrib
Go Push Contrib Contrib
Collector Pull Core Contrib
Collector Push Core Contrib
DotNet Pull Core Contrib
Ruby N/A - -
Erlang N/A - -
Rust Pull Core Contrib
PHP Pull Core Contrib
@jkwatson
Copy link
Contributor

how do you bootstrap this? Who are the initial maintainers of these new repositories? Doesn't this just add additional overhead to the existing maintainers' workload?

@anuraaga
Copy link

A couple of thoughts are

  • Prometheus exporter is one of our most important exporters. It feels like a core component to me - I suspect the reason it may not be prioritized now is the focus on tracing, but with metrics ramping up it should get attention from maintainers.

  • Devs can contribute pull requests regardless of whether they're maintainers or not. @alolita Are you seeing a lot of stuck pull requests in the language repos related to Prometheus? I haven't seen any PRs in the Java repo yet. I'm not convinced this reorganization would actually help with velocity without seeing contributions from these proposed "contrib maintainers" in our current codebases first. If they contribute a lot, they can become core approvers / maintainers eventually as well, going too deeply into contrib-land encourages siloing and less ownership of the overall project from contributing organizations IMO.

@alolita
Copy link
Member Author

alolita commented Dec 3, 2020

@anuraaga responding to your first point - although Prometheus components are considered core in OTel, my experience in working to improve existing components such as the Prometheus receiver or adding Prometheus exporters in some of the language SDKs and Collector lead to this proposal. OTel core maintainers often have not had the bandwidth, priority and sometimes knowledge to review or take decisions on core spec changes or implementation changes. I'd like to see other engineers who are working actively on Prometheus protocol interoperability and implementation be able to help in triaging, reviewing and maintaining these components.The objective is to spread the wealth, distribute resources more evenly and avoid bottlenecks.

To your second point, developers are indeed able to contribute pull requests once they are members. But a lot of pull requests sit in queue waiting to be reviewed for days, sometimes weeks (e.g. .NET). I see developers just waiting patiently or frustrated unable to get reviews or get release artifacts with their merged changes since current maintainers are focused on GA prioritized tasks. Enabling components such as exporters to be hosted in contrib or in their own repos would enable that development to continue on the project with an easier path to add reviewers and maintainers for these essential but not core OTel components. Automating release processes to have nightly builds and regular releases may alleviate some of these issues but we still need to add more maintainers to every language and component. The objective here is to increase project modularity with multiple repos and while this may increase complexity of the project initially, it could provide better scalability in the long run.

@iNikem
Copy link
Contributor

iNikem commented Dec 3, 2020

I believe GitHub allows separate approvers for parts of the repository. This way we can continue to have all parts in the same repo but at the same time invite more approvers for those areas that require it.

@anuraaga
Copy link

anuraaga commented Dec 3, 2020

OTel core maintainers often have not had the bandwidth, priority and sometimes knowledge to review or take decisions on core spec changes or implementation changes

This is surprising given that Prometheus is arguably the main metrics export that we are targeting. Without it living in core, it seems like it would be difficult to actually make progress on metrics work, especially in a way that supports Prometheus - from what I understand, many of the issues with Prometheus export that have been found lately are directly related to the data modeling in the SDK itself. So if speed of improvements there is important, than I'd argue that keeping them close is precisely the thing to do. If projects are lacking expertise or focus on metrics, then we can use more help, eventually adding more maintainers. For example, @jkwatson has embarked on an arduous journey to rewrite the metrics implementation in Java. It's precisely this sort of change that a Prometheus expert can provide great insight on, and eventually make sure our core support for Prometheus is sound. If someone (maybe from AWS? :P) stepped up to help on this journey, helping us with tasks that aren't necessarily part of their OKRs but for the greater good, I wouldn't be surprised to give them approval / maintainer rights in core at some point, if not for the whole repo, then for the metrics parts as @iNikem suggests. In order to spread the help, it'd be great to have more hands in core itself I'd say to make it more lively :) I suspect this will encourage stronger community vs increasing the number of silos.

Also, there is a big technical hurdle with separating repos of version management - which version of the exporter is compatible with which SDK? for example. I would say there must be ways to improve the velocity of the projects process-wise before trying out a big hammer.

@jkwatson
Copy link
Contributor

jkwatson commented Dec 8, 2020

I'd like to understand the specific scope of this OTEP. "non-core components like Exporters" needs to be very carefully defined. Are propagators included in being "non-core"? What is included in "core"? W3C? B3? Jaeger? How do we draw those lines without being seen as playing favorites?

@Oberon00
Copy link
Member

Oberon00 commented Dec 9, 2020

I would say that only W3C is core for propagators. For trace exporters, I would say only OTLP (and maybe a logging exporter). Alternatively, one of Jaeger or Zipkin may be added (AFAIK Zipkin would have a bit broader backend support, but has no good way to export OTel resources) either in addition or instead of OTLP. I would like to keep the core very minimal in that regard.

@anuraaga
Copy link

anuraaga commented Dec 9, 2020

I think one thing to keep in mind is exporter can somehow work out by adding the collector. Even if core didn't support Zipkin collector can help.

With propagation, there is no choice but for the SDK to support it. So if core doesn't support a common propagation format, there is no hope for adoption because no one can migrate an entire fleet to w3c in one push, it's just not possible. So treating the two differently could make sense in a practical sense

@iNikem
Copy link
Contributor

iNikem commented Dec 9, 2020

I still don't understand why are we moving anything out at all. Initial concern was about lack of maintainers. We can add extra mantainers to specific folders in the main repo. Why is this bad solution to the original problem? Or is there anything else that we are trying to solve?

@carlosalberto
Copy link
Contributor

carlosalberto commented Dec 9, 2020

I would say that only W3C is core for propagators. For trace exporters, I would say only OTLP (and maybe a logging exporter).

If we go this route, I'd include Baggage too, and I'd also vow for B3, as it has been a de facto tracing standard, and could help porting code (that being said, I'd really like to have Jaeger there).

We can add extra mantainers to specific folders in the main repo.

This is slightly what happens with the Collector (which I like): have a contrib repo with a many different components, and have each one its CODEOWNERS.

Guess my opinion is that overall we should have the core stuff in the core repos too (Jaeger, Zipkin, Prometheus exporters). The rest, sure, that can go into a contrib repo.

@dyladan
Copy link
Member

dyladan commented Dec 15, 2020

While I don't want to unnecessarily complicate the process, I am increasingly seeing a tier of components emerging which for this purpose I'll call "official extensions," and a desire to keep these separate from core components like the providers and span processors. Things like prometheus-exporter, b3-propagator, jaeger-exporter, jaeger-propagator, et cetera which are required by the spec, but exist to help opentelemetry interoperate with other observability systems.

@carlosalberto
Copy link
Contributor

Also a follow up from the (brief) discussion in today's call:

  • There's a general feeling the officially supported exporters (OTLP, Jaeger, Zipkin, Prometheus) should live in the core repos, if possible. If a small part of the community wants to own, say, the Prometheus exporter, then they can effectively own it through CODEOWNERS (not entirely closed to have them in contrib repos though).
  • We may need more reviewers/maintainers (a problem I'd argue would still happen even after moving exporter code to a contrib repo). This was specially obvious recently having many interns posting PRs and not enough maintainer cycles for all the reviews (something we need to address the next year somehow).

Action items:

  • Consider having CODEOWNERS for specific exporters (for some repos)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:required-for-ga Must be resolved before GA release, or nice to have before GA
Projects
None yet
Development

No branches or pull requests

8 participants