oteps icon indicating copy to clipboard operation
oteps copied to clipboard

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

Open alolita opened this issue 3 years ago • 12 comments

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

alolita avatar Nov 25 '20 05:11 alolita

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?

jkwatson avatar Nov 25 '20 05:11 jkwatson

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.

anuraaga avatar Nov 25 '20 05:11 anuraaga

@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.

alolita avatar Dec 03 '20 08:12 alolita

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.

iNikem avatar Dec 03 '20 08:12 iNikem

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.

anuraaga avatar Dec 03 '20 08:12 anuraaga

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?

jkwatson avatar Dec 08 '20 16:12 jkwatson

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.

Oberon00 avatar Dec 09 '20 11:12 Oberon00

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

anuraaga avatar Dec 09 '20 12:12 anuraaga

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?

iNikem avatar Dec 09 '20 13:12 iNikem

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.

carlosalberto avatar Dec 09 '20 22:12 carlosalberto

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.

dyladan avatar Dec 15 '20 17:12 dyladan

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)?

carlosalberto avatar Dec 15 '20 19:12 carlosalberto