Remove go.opentelemetry.io/otel/sdk dependency from instrumentation
#381
Conversation
- Create separate go.mod for intrumentation examples - Update and clean-up go.mod files for instrumentation - Use replace directive to on-disk location where appropriate
- replace SDK with internal trace test helper package
matej-g
requested review from
Aneurysm9,
MrAlias,
XSAM,
evantorrie,
jmacd and
lizthegrey
as code owners
| "github.com/gocql/gocql" | ||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| mockmeter "go.opentelemetry.io/contrib/internal/metric" |
There was a problem hiding this comment.
I'm aware that that these test helpers are now available in the main OTEL repo (related to discussion in #190 (comment)).
I wanted to open a ticket to replace these internal helpers with the ones from the OTEL repo, however I noticed that opentelemetry-go-contrib has also own internal helpers for tracer, even though these existed already in the OTEL repo.
Is there a reason for this duplicity or should tracer test helpers be replaced by the helpers from OTEL repo as well?
There was a problem hiding this comment.
I wanted to open a ticket to replace these internal helpers with the ones from the OTEL repo, however I noticed that
opentelemetry-go-contribhas also own internal helpers for tracer, even though these existed already in the OTEL repo.Is there a reason for this duplicity or should tracer test helpers be replaced by the helpers from OTEL repo as well?
No, we should unify on a single implementation in the otel repo IMO. I think the duplicity came from the concurrent development of both testing libraries more than an explicit choice. Unifying will avoid drift in implementation and avoid uncertainty by future development on which package to use.
MrAlias
left a comment
MrAlias
left a comment
There was a problem hiding this comment.
Looks good overall. Excited to see all the cleanup in the instrumentation dependencies!
Just some bookkeeping issues with the go.mod file replace declarations.
instrumentation/github.com/gorilla/mux/otelmux/example/go.mod
Show resolved
Hide resolved
instrumentation/github.com/Shopify/sarama/otelsarama/example/go.mod
Show resolved
Hide resolved
| "github.com/gocql/gocql" | ||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| mockmeter "go.opentelemetry.io/contrib/internal/metric" |
There was a problem hiding this comment.
I wanted to open a ticket to replace these internal helpers with the ones from the OTEL repo, however I noticed that
opentelemetry-go-contribhas also own internal helpers for tracer, even though these existed already in the OTEL repo.Is there a reason for this duplicity or should tracer test helpers be replaced by the helpers from OTEL repo as well?
No, we should unify on a single implementation in the otel repo IMO. I think the duplicity came from the concurrent development of both testing libraries more than an explicit choice. Unifying will avoid drift in implementation and avoid uncertainty by future development on which package to use.
- Include missing replace directives in example go.mod files
4 participants
In line with the aim stated in the #161, the instrumentation packages should be made more lightweight by removing the dependency on the main SDK package (go.opentelemetry.io/otel/sdk). In order achieve it, this changeset:
gocqlmetric tests, which previously depended on SDK - this has been replaced by internal mock for metricsAdditionally, dependabot configuration has been updated to reflect his, as well as
.gitignoreto ignore binaries built by example modules.Resolves #161