Skip to content
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

Large number of dependencies when instrumenting a library #940

Closed
kyleconroy opened this issue Jul 15, 2020 · 6 comments
Closed

Large number of dependencies when instrumenting a library #940

kyleconroy opened this issue Jul 15, 2020 · 6 comments
Assignees
Labels
question Further information is requested

Comments

@kyleconroy
Copy link

First off, thanks for all the hard work on OpenTelemetry.

I recently ran into an issue with the go-redis package. Adding it to a new project pulled in a large set of dependencies, including google.golang.org/grpc and cloud.google.com/go. It turns out this is because the go-redis package imports go.opentelemetry.io/otel. This situation is addressed in the README

Libraries that produce telemetry data should only depend on api and defer the choice of the SDK to the application developer. Applications may depend on sdk or another package that implements the API.

However, a library can't only depend on the api package. When using Go modules, the entire go.opentelemetry.io/otel module is required. How feasible would it be to split the api and sdk packages into separate modules? This way, the api module could have a much smaller set of dependent packages, hopefully excluding google.golang.org/grpc.

@MrAlias MrAlias added question Further information is requested release:required-for-ga labels Jul 15, 2020
@MrAlias
Copy link
Contributor

MrAlias commented Jul 15, 2020

This seems like a reasonable ask to me. I'm going to add it to the SIG agenda to talk about tomorrow. I'll update with details of the discussion.

@kyleconroy
Copy link
Author

Thanks! Let me know what I can do to help. Happy to write up a larger explanation of my issue.

@MrAlias MrAlias self-assigned this Jul 23, 2020
@MrAlias
Copy link
Contributor

MrAlias commented Jul 23, 2020

Looking into this the status code for the Span in the trace API uses google.golang.org/grpc/codes bringing the import into the API. We need to look at alternatives to avoid this.

@MrAlias
Copy link
Contributor

MrAlias commented Aug 11, 2020

We've gone from 14 direct (external) dependencies:

opentelemetry-go/go.mod

Lines 5 to 20 in aff7a80

require (
github.com/DataDog/sketches-go v0.0.0-20190923095040-43f19ad77ff7
github.com/benbjohnson/clock v1.0.3
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/golang/protobuf v1.4.2
github.com/google/go-cmp v0.5.0
github.com/google/gofuzz v1.0.0 // indirect
github.com/kr/pretty v0.1.0 // indirect
github.com/opentracing/opentracing-go v1.2.0
github.com/stretchr/testify v1.6.1
golang.org/x/net v0.0.0-20190613194153-d28f0bde5980 // indirect
golang.org/x/sys v0.0.0-20200615200032-f1bc736245b1 // indirect
google.golang.org/genproto v0.0.0-20191009194640-548a555dbc03 // indirect
google.golang.org/grpc v1.30.0
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
)

To 2:

require (
github.com/google/go-cmp v0.5.1
github.com/stretchr/testify v1.6.1
)

With these last two being testing dependencies. I feel like we could in the future build out the testing to only rely on the testing package, but it might be diminishing returns to try here (likely just going to be reproducing imported functionality).

I'm going to close this at this point with all the reduction that we have achieved and if we want to tackle the last two a new issue can be opened to address them.

@MrAlias MrAlias closed this as completed Aug 11, 2020
@MrAlias
Copy link
Contributor

MrAlias commented Aug 11, 2020

Still need to do a release to get these savings published.

@kyleconroy
Copy link
Author

@MrAlias Wow! This is fantastic.

It took me about 10 minutes to open up the issue, but it obviously took far more work for the team to fix it. Thank you (and everyone else) for closing this out. I agree that testify and go-cmp are fine.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants