openzipkin / zipkin-gcp

Reporters and collectors for use in Google Cloud Platform
https://cloud.google.com/trace/docs/zipkin
Apache License 2.0
91 stars 54 forks source link

Extract reusable Stackdriver functionality into a common module #139

Closed elefeint closed 5 years ago

elefeint commented 5 years ago

StackdriverSender and StackdriverStorage contain similar but subtly different implementations of talking to and checking health of Stackdriver's BatchWriteSpans API.

Would it make sense to create a stackdriver-common module that both client and server-side functionality could depend on?

devinsba commented 5 years ago

In zipkin-aws we have storage-xray and sender-xray, the storage component uses the sender component for it's writes so there isn't duplicate code. Maybe this pattern is worth exploring

codefromthecrypt commented 5 years ago

I think we might end up with some dependency problems unless you are thinking of using a few different parameters or otherwise. If the base has no deps not already in both, it could make sense to reduce the amount of test duplication. OTOH, we would then have another module to maintain which is a bit more complex than not. the resulting types would be more complex also.

three redundants are a lot easier to reason with common extraction than 2.. 2 can go either way.

elefeint commented 5 years ago

Makes sense; closing for now.