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

IOLocal to Local Access for Middlewares #195

Closed
ChristopherDavenport opened this issue May 3, 2023 · 8 comments · Fixed by #224
Closed

IOLocal to Local Access for Middlewares #195

ChristopherDavenport opened this issue May 3, 2023 · 8 comments · Fixed by #224

Comments

@ChristopherDavenport
Copy link
Member

End systems seem to require access to the Local[F, Vault] or IOLocal[Vault] to be able to leverage the TextMapPropagators. However currently the [A] =>> IOLocal[A] => F[_] =>> Local[F,A] is private which means any middleware that operates using the expected Local semantics has to request the end user application to do this transformation for them.

I know the desire is to upstream this to cats-effect, however it seems to me that anyone leveraging the java backend for propagators will need this transformation available or write the same boilerplate themselves into their end user application.

https://github.com/ChristopherDavenport/natchez-http4s-otel/blob/otel2/examples/src/main/scala/example/Http4sExample.scala#L41-L64

@armanbilge
Copy link
Member

@iRevive
Copy link
Contributor

iRevive commented May 3, 2023

There is an idea to simplify propagation in a way passing Local[F, Vault] or IOLocal[Vault] downstream is not mandatory.

#182 (comment)
#182 (comment)

@rossabaker
Copy link
Member

rossabaker commented May 3, 2023

I can't remember why I pulled LiftIO out of this, but that's basically the first method.

There's talk on #182 1 about how best to provide access to the local vault. It didn't really come up until people started exercising propagators.

We could provide some sort of temp package with whatever gets settled on? API is going to need to be rethought if that CE ticket fails. I still believe in that CE3 ticket: things get real weird using IOLocal statefully through arbitrary libraries.

Footnotes

  1. iRevive commented 7 minutes ago It's embarrassing when GitHub tells me how slow I type or how distracted I am...

@lacarvalho91
Copy link
Contributor

We could provide some sort of temp package with whatever gets settled on?

Is this something that we could do for now? Be good to not have to copy paste this Local instance around. Or is it fine to just use kleisli? I'm not really sure when to use one over the other.

@rossabaker
Copy link
Member

I see no reason not to provide the instance. The choice between that or Kleisli is really a matter of taste.

Kleisli tends to intimidate people, but shouldn't. Much like IO is a fancy way to build your program as a Function0, Kleisli is a fancy way to build your program as a Function1. There's no hidden fiber state. It's right there in the types. The problem is, it's right there in the types. That Vault is in your face unless you are rigorous about being final tagless. (I am, but I'm old and unfashionable.). And your main method probably involves some type tetris.

Tagless or not, I generally recommend IOLocal. It does weird shit if you use non-Local methods, but properly constrained, it works well with low cognitive overhead.

@armanbilge
Copy link
Member

armanbilge commented May 17, 2023

IOLocal is also a lot faster. There is virtually no performance penalty for unused IOLocals tagging along through untraced parts of an app/library. That's far from the case for Kleisli.

@rossabaker
Copy link
Member

That performance claim is true, easily demonstrable in contrived benchmarks, and doesn't tend to be meaningful as soon as the return type is RealIO[BloatedBusinessResponse].

@lacarvalho91
Copy link
Contributor

That Vault is in your face unless you are rigorous about being final tagless. (I am, but I'm old and unfashionable.)

@rossabaker I'm a big fan of tagless final, at $work we will be adopting this as we move away from Akka. Just thought it might be nice for you to hear that there are people still advocating for it!

I see no reason not to provide the instance

Cool - thanks, I'll try and raise a PR some point soon.

The choice between that or Kleisli is really a matter of taste

OK thats good to hear, personally I like having that context reflected in my effect type.

doesn't tend to be meaningful as soon as the return type is RealIO[BloatedBusinessResponse]

I was under this impression too, at $work we mostly write microservices that call other microservices/read DBs so I don't see that performance penalty having any meaningful impact on us

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.

5 participants