split otlp/otlphttp/logging exporters #6343
Conversation
Codecov ReportBase: 91.76% // Head: 91.40% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #6343 +/- ##
==========================================
- Coverage 91.76% 91.40% -0.37%
==========================================
Files 236 236
Lines 13478 13478
==========================================
- Hits 12368 12319 -49
- Misses 880 935 +55
+ Partials 230 224 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
codeboten
changed the title
codeboten
force-pushed
the
codeboten/exporter-test
branch
from
b83b76e
to
3ac5a49
Compare
codeboten
force-pushed
the
codeboten/exporter-test
branch
from
3ac5a49
to
d56c71e
Compare
exporter/go.mod
Outdated
| go.opentelemetry.io/collector v0.0.0-00010101000000-000000000000 | ||
| go.opentelemetry.io/collector/pdata v0.62.1 | ||
| go.opentelemetry.io/otel v1.11.0 | ||
| go.opentelemetry.io/otel/sdk v1.11.0 |
There was a problem hiding this comment.
Not immediately problematic, but this direct dependency on the OTel Go SDK in an instrumented library that should only use the API is something we recommend avoiding. In the go-contrib repo we have done this by having a subordinate test module that can depend on the SDK tracetest packages and the like. It presents a good and reliable signal to consumers that an instrumentation library is not going to attempt to modify your SDK configuration when it doesn't require the SDK module at all.
There was a problem hiding this comment.
exporter/go.mod is no longer in this PR, but I see what you're saying. as the exporterhelper is back in the collector module (which depends on the sdk for different reasons), i think it's ok for now, but if the exporterhelper gets split into its own module, we could do what you're suggesting here to avoid taking on a dependency on the sdk for the tests alone.
codeboten
force-pushed
the
codeboten/exporter-test
branch
from
d56c71e
to
d0978c9
Compare
codeboten
force-pushed
the
codeboten/exporter-test
branch
from
3d6b857
to
94e383b
Compare
| @@ -18,6 +18,9 @@ module-sets: | |||
| modules: | |||
| - go.opentelemetry.io/collector | |||
| - go.opentelemetry.io/collector/cmd/builder | |||
| - go.opentelemetry.io/collector/exporter/loggingexporter | |||
There was a problem hiding this comment.
Was this added manually? Or is this the one that is managed by multimod?
There was a problem hiding this comment.
Manually to get multimod-verify to pass
|
How do we make contrib tests pass? I don't see a way out of it at the moment |
This may be a good reason to use Any thoughts @jpkrohling @bogdandrutu @dmitryax @Aneurysm9? @mx-psi the alternative is to update contrib in a follow up PR. |
IMO this is okay, but we would need a mechanism for skipping contrib tests then ( |
I didn't realize "Skip contrib" was a label we had :D |
This PR does the following creates separate modules for otlp/otlphttp/logging exporters Fixes open-telemetry#6194 open-telemetry#6195 open-telemetry#6196
Co-authored-by: Juraci Paixão Kröhling <juraci.github@kroehling.de>
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
codeboten
force-pushed
the
codeboten/exporter-test
branch
from
8c34655
to
2c5ad4a
Compare
This PR does the following creates separate modules for otlp/otlphttp/logging exporters
Fixes #6194 #6195 #6196