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

Remove Census dependency from grpc-core #5076

Closed
ejona86 opened this issue Nov 20, 2018 · 3 comments
Closed

Remove Census dependency from grpc-core #5076

ejona86 opened this issue Nov 20, 2018 · 3 comments
Milestone

Comments

@ejona86
Copy link
Member

ejona86 commented Nov 20, 2018

Census is an implicit dependency today and automatically enables itself if Census is included on the classpath. This was because that behavior was a "requirement."

I've since learned that other languages didn't implement that "requirement." If we kicked census out of grpc-core, it would be possible to add grpc-context back into grpc-core to resolve #2727 and #3522 and similar. (The general idea here is that internal could also be moved out of grpc-core, so Context-only users don't pull in as much code.)

Unfortunately that puts us in a bit of a bind for existing users, as they will probably expect that they can enable Census classloader-wide without code changes to each call site.

A possible solution to that is to provide a global interceptor API, where we allow exactly one call to a method that sets the global interceptors that are used for all Channels. The expectation is that this method would be called in main().

Another option is to add a grpc-census artifact that grpc-core looks for and uses if available via reflection. That would certainly be easier for users to update.

@ejona86
Copy link
Member Author

ejona86 commented Sep 17, 2019

Since this was filed, we split grpc-api out of grpc-core. So this no longer has impact on context-only users. Although we still want it because the Census dependency does cause trouble and feels wrong to be in core. For example removing the dependency could be a solution to #5510.

@bogdandrutu
Copy link
Contributor

@ejona86 this can be closed now I think.

@ejona86
Copy link
Member Author

ejona86 commented Apr 20, 2020

This was fixed by #6577

@ejona86 ejona86 closed this as completed Apr 20, 2020
@ejona86 ejona86 modified the milestones: Next, 1.27 Apr 20, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants