search

zio / zio-query

Add efficient pipelining, batching, and caching to any data source
https://zio.dev/zio-query
Apache License 2.0
150 stars 22 forks source link

Thoughts about compatibility with ZIO and also tracing #500

Open valenterry opened 1 month ago

valenterry commented 1 month ago

The ZIO ecosystem is awesome and very pragmatic. However, when it comes to ZIO and ZQuery, the lack of abstraction shows a bit.

ZQuery is missing quite a few convenience functions that are on ZIO (even though it should be no problem to add them on ZQuery). Also, there is no common interface, so when having code that needs to work with both sometimes, common abstractions are harder.

Furthermore, there is no OpenTelemetry support for ZQuery unless I missed something - that is really a bummer.

Thoughts?

kyri-petrou commented 1 month ago

Hey @valenterry, thanks for the feedback!

ZQuery is missing quite a few convenience functions that are on ZIO

I agree, although given how big the ZIO API is, it'll require a fair bit of work to support all of its methods. Are there any specific ones you feel like you're often missing? Also, feel free to open a PR to add any of them as you see fit!

Also, there is no common interface, so when having code that needs to work with both sometimes, common abstractions are harder.

I'm a little bit confused regarding this one. Can you provide an example of such usecase?

Furthermore, there is no OpenTelemetry support for ZQuery

I agree that it would be nice for Datasources to be instrumented for OTEL. However, I'm not sure to what degree this is useful since zio-query uses the ZIO runtime, which most OTEL libraries already support. In addition, this needs to be implemented at the OTEL libraries, not in zio-query. Which OTEL library do you use? Perhaps we could add an OTEL module for ZQuery in zio-opentracing.

valenterry commented 1 month ago

I'm a little bit confused regarding this one. Can you provide an example of such usecase?

For example, I use caliban. Some of my endpoints are defined as ZIO and some are defined as ZQuery. I have some functions factored out that first run common logic (e.g. auth and logging) and then run the effect. Their signature is something like def runInContext[...](f: Context => ZIO[...]) which means, I need to duplicate those functions for ZIO and ZQuery, even if I use the same functionality (like flatMap only).

As for the OpenTelemetry part, I'm actually not 100% sure how it work conceptually, given that ZQuery is precisely there to abstract over the querying-flow. As for what I use, that would be GCP's opentelemetry package.

kyri-petrou commented 1 month ago

Lifting a ZIO to ZQuery can be done via the ZQuery.fromZIO and ZQuery.unwrap methods, and ZIO effects can be "flatMapped" with ZQueries via the mapZIO method.

I think this is as best as we can do since ZQuery is a more powerful variant of ZIO, therefore we can only lift them one-way (from ZIO to ZQuery)

valenterry commented 1 month ago

Yeah, but if I have either a ZIO or ZQuery at hand, then I need to either convert them (which is annoying) or use runInContextZIO or runInContextZQuery depending on the value I have. I could overload runInContextZIO but overloading comes with its own disadvantages.

Since both can be me e.g. mapped, it would theoretically be possible to have a common trait Mappable (or Functor/Applicative ...) to use as an interface to avoid that.

However, I doubt that this will ever happen in the ZIO ecosystem - please just consider it a little rant. :)

As for the missing functionality, I will add some of these (I hope). Like ZQuery.filter(...) for example.

kyri-petrou commented 1 month ago

Since both can be me e.g. mapped, it would theoretically be possible to have a common trait Mappable (or Functor/Applicative ...)

I believe what you're asking for can be achieved by using the zio-prelude library. However, it only provides type classes for ZIO. Potentially we could also add type classes for ZQuery, but that would require a bit of effort and publishing a new package (zio-query-prelude?). I think it will be a good addition, but I got my hands a bit full at the moment so I'm not sure when I'd be able to get to it

valenterry commented 1 month ago

I believe what you're asking for can be achieved by using the zio-prelude library.

Oh yeah, I forgot that this exists. This is exactly what should be used IMHO. There is e.g. AssociativeFlatten which we could add a ZQuery instance for. But since ZQuery does not seem to depend on ZIO directly, yeah, it should be in a separate package I guess.

If you could setup the build/project structure then I'm happy to add the instances in there.

sideeffffect commented 44 minutes ago

Perhaps zio-query can start depending on zio-prelude. Then, zio-query can implement the relevant type classes for its own types in its own codebase.