Closed dmathieu closed 2 months ago
cc @mx-psi
If we do this, there will be the question of deprecating/removing the current interfaces. Since those packages haven't reached 1.0, it should be doable.
How would this work for connectors, which depend on pairs of signal types?
It looks like connectors have a specific stability per signal/signal (https://github.com/open-telemetry/opentelemetry-collector/blob/main/connector/connector.go#L108-L118). So maybe that module could remain as it is. I don't know how it would work once it reaches 1.0 though.
For consumer, I think it should be easy to do the split and that we should go ahead with it. That way we avoid changing import paths once we move profiles to stable. I think it is also more pressing to decide this for consumer, since it's the one that is closest to 1.0.
For the component types, I think we have more time to think about those, so it may be good if the profiling folks can do an RFC for this part or something similar. There also seem to be several independent pieces that we want to answer here:
<componentkind>.Factory
? (leaning towards no here since it's too much work for only a small benefit, but we need to figure out the way of adding profiles support, maybe with an optional interface?)<componentkind>.Builder
struct? To be honest here, maybe we should not have Builder
at all as part of the public API, this seems mostly only useful for otelcol
/service
.On the 2024-05-22 meeting, we discussed this but reached no consensus. A summary of the discussion:
pprofile
struct and we promise not to break anything else.
How to move from here:
@mx-psi some things to discuss/understand:
Because of this I am not sure we should rush to add it yet.
The reason I am working on this is that one of the requirements from the profiling SIG to accept the Elastic profiling agent donation is to have it running as a collector receiver. We can't do that unless the collector has the proper interfaces. Also, while I agree the proto is still extremely experimental, the pdata is already within the collector (which is the part that could have breaking changes), under its own module so it has its own versioning model until it becomes stable.
Also, being able to receiver/forward/store profiles in OTLP is going to be a big part in validating that protocol.
The goal of this refactoring here is to make it easier to have experimental/beta packages within the collector, which would be very hard if we have to modify stable package APIs.
Since any experimental signal will eventually end in the service package, does it mean service will never be stable API? Does it mean that our whole collector will not be stable? I think we have to adopt something like gRPC, even though I understand that is not ideal, is the only option we have for things like service, unless we are willing to copy code and have also an "experimentalservice", and go deeper.
@bogdandrutu
I think the two goals that we should have here are:
Assuming we solve for these two at the lower levels, for Service
I can think of a few alternatives that are not the grpc-go approach:
Service
constructors (click to expand)service.New
return an error if you use any profiles related functionality without enabling a dedicated feature gate. We would have to adda Profiles
field to the pipelines struct and commit to supporting this (I feel like that's fine)Setting
for enabling profiles (click to expand)Do any of these work for you? Ultimately, I am more concerned about modules below service, since those are the ones that I would expect to cause more trouble with the builder in terms of build errors, so I am not entirely against just labeling the fields as experimental at the service
level.
What is so bad of asking users to do a import rename for something experimental, the idea of experimental is to not be in this state for a long time, and ideally not too many external dependencies are taking a dependency on this.
Not a big deal, I think the two points above are more important. The avoiding renames part is a nice-to-have.
Sorry for the delay, I discussed this with @dmathieu already in private, but so that it is more 'official' and everybody else can participate, this is what the Collector Stability Working Group discussed on 2024-05-29:
As next steps, (speaking personally for this part), I would like to see a design document that lays out the proposal from the Profiling SIG on how to do this, possibly supported by a PoC like the one @dmathieu is working on over at #10307. We don't have a formal structure for Collector-specific design documents but you can check this as a recent example. We don't need to figure out every detail on the RFC; the aim should be to have a high level vision of the proposal in a way that allows us to consider pros and cons.
Closed by #10375, we decided not to split by signal but instead do separate modules for experimental signals only
Is your feature request related to a problem? Please describe.
As we're planning the addition of profiles into the collector, we will need to add new interfaces into several packages (
consumer
,receiver
,processor
,connector
). Because the profiles signals is experimental, we would like to avoid making changes to those APIs that would be making them unstable.Describe the solution you'd like
Split each of the packages that have interfaces specific to signals into their own module/package. We would start with
consumer
, as that API is the closest to 1.0. But each package that has a signal-specific implementation would be impacted.We could do this two ways:
traces/consumer
,traces/receiver
.consumer/traces
,receiver/traces
.With this change, any new signal added in the future could much more easily implement its own interfaces even though the main packages are stable.
Describe alternatives you've considered
The alternatives could be to: