open-telemetry / oteps

OpenTelemetry Enhancement Proposals
https://opentelemetry.io
Apache License 2.0
334 stars 163 forks source link

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

Open alolita opened 3 years ago

alolita commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

A couple of thoughts are

alolita commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

Action items: