-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove unstable metrics dependency from the OTLP exporter #1827
Comments
👀 |
My ideia to remove the metrics dependency is to split the current exporters by signal and protocol. Creating 4 new client modules for gRPC metrics and traces, and HTTP metrics and traces. These clients would be passed to an Exporter that would wrap them and implement the sdk exporter interfaces.
Proposal usage example: // Unstable Metrics exporters
otlpmetrics.NewExporter(otlpgrpcmetrics.NewClient(clientOps...), exporterOpts...)
otlpmetrics.NewExporter(otlphttpmetrics.NewClient(clientOps...), exporterOpts...)
// Stable Traces Exporters
otlptraces.NewExporter(otlpgrpctraces.NewClient(clientOps...), exporterOpts...)
otlptraces.NewExporter(otlphttptraces.NewClient(clientOps...), exporterOpts...) Pros:
Cons:
reminder to self: proto-go also needs to be updated to have modules per signal. |
I was considering the "Cons" section above and agree about there being a lot of modules. I believe there will be users with legitimate reasons to want only one signal, and your solution works for them. Even when both are stable, this organization will probably hold its value. If we didn't have multiple dimensions of variation, this would be easier. The use of I support this plan. |
Is it good enough for these users to create one driver and share it between two exporters? The connection state will be shared -- there's not much other reason to combine the exporters, the data paths never cross. |
What do you think of making these modules internal so that should be wrapped? So that the client would not directly depend on this helper module. |
Would this resolve this issue, would the metrics dependency be removed if this approach was taken? I'm not sure I follow your suggestion here. Can you expand it? |
It would NOT make it possible to remove the dependency without making breaking API changes. This is the line I am concerned about. It is returning a struct for another module: |
EDIT: Got your point now, as I commented in the #1922, we should probably discuss that more in this week sig about the keeping the |
Talking about this in the SIG meeting today, this looks "done enough" to release. We should close this and capture the remaining tasks should moved to a new issue. |
Remaining tasks that I'll open issues for:
|
In order to release the OTLP exporter in our RC and subsequent 1.0 release it needs to not depend on any packages that will be experimental. Currently the exporter depends on the
metirc
,sdk/metric
, andsdk/export/metric
packages:opentelemetry-go/exporters/otlp/go.mod
Lines 14 to 17 in 70bc9eb
We need to find a way to factor these dependencies out while.
Proposed Solutions
The text was updated successfully, but these errors were encountered: