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

armerify #119

Closed
codefromthecrypt opened this issue Mar 21, 2019 · 8 comments · Fixed by #120
Closed

armerify #119

codefromthecrypt opened this issue Mar 21, 2019 · 8 comments · Fixed by #120

Comments

@codefromthecrypt
Copy link
Member

I think we can do more and get out of dodging netty and/or boringssl conflicts by moving the stackdriver collector (not necessarily the sender) to armeria. This would imply figuring out if armeria has an oauth feature which is compatible with stackdriver

cc @openzipkin/armeria

@anuraaga
Copy link
Collaborator

What does it mean to migrate the collector to armeria? I thought the armeriafication of zipkin-server effectively makes all collectors armeria.

As for sending, here's an example of using armeria to issue gRPC to Google apis, including trace

https://github.com/curioswitch/curiostack/blob/master/common/google-cloud/core/src/main/java/org/curioswitch/curiostack/gcloud/core/grpc/GrpcApiClientBuilder.java

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Mar 21, 2019 via email

@codefromthecrypt
Copy link
Member Author

@anuraaga your approach looks good. Only concern is that it doesn't appear the auth package is extracted into a jar we could consume. I wonder if it could be a submodule in armeria or somewhere else?

@anuraaga
Copy link
Collaborator

Ah yeah - that package is sort of overkill, it is to get auth tokens using armeria, but in reality is only called every few minutes anyways. Zipkin should use the official Google auth library that just uses a Java URL.

I can whip something up over the weekend

@codefromthecrypt
Copy link
Member Author

PS I noticed we are still depending on grpc classes, which brings in a tainted dependency tree as they assume late version of guava (and old version of gson but not to api peril). Would be nice to implement grpc transport without this problem, but for now we can dodge. The issue is mostly the fight over guava which requires our server to artificially track here just to not conflict..

@anuraaga
Copy link
Collaborator

Hmm - it would be almost trivial to issue raw requests without the armeria-grpc at all in this case.

Unary requests are simple to frame. It would be difficult though to support advanced features like message compression, though, for which even armeria-grpc leaves the details to grpc-core. I guess message compression could be useful if posting from a different cloud, but don't know if the trace server even enables it.

@codefromthecrypt
Copy link
Member Author

@anuraaga yeah I suppose it would just be about the request auth etc.

for this specific integration though, only what is supported by stackdriver is needed.. but you are right to say certain compression things will make sense to support. There's sortof a push and pull here, but one guarantee is that grpc dep chain moves regardless of us and in incompatible ways. One of the benefits to armeria was to try and get out of that :P

@anuraaga
Copy link
Collaborator

The request auth would be using the google auth library, which is not updated as much as grpc (though I guess still depends on guava...). I think in practice, it really is only about compression. If we decided not to support it, we can rip out all the gRPC. I lean towards doing so since I guess most people will be using this in on GCP and don't care about compression.

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

Successfully merging a pull request may close this issue.

3 participants
@codefromthecrypt @anuraaga and others