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

Context service #1408

Merged
merged 20 commits into from
Jan 20, 2022
Merged

Context service #1408

merged 20 commits into from
Jan 20, 2022

Conversation

fedefernandez
Copy link
Contributor

@fedefernandez fedefernandez commented Jan 12, 2022

This PR makes the Span[F] context abstract so that you can make your service dependant on any "context."

The idea comes from #1328

Pending tasks:

  • Add tests for a custom context
  • Update docs -> Will do in the following PR

Currently, we provide some methods in the companion object for getting a parameterized algebra over Span[F] with Kleisli

Given:

trait MyService[F]

The method MyService.tracingClient[F[_]] provides a:

Resource[F, MyService[Kleisli[F, Span[F], *]]]

And the method MyService.bindTracingService[F[_]] asks for an implicit instance implementation of the algebra parametized with Kleisli[F, Span[F], *] (MyService[Kleisli[F, Span[F], *]]).

In this PR, we're adding alternative methods to those "tracing" ones where you can specify any type as a context as soon you provide a couple of implicit implementations.

The new method MyService.contextClient[F[_], C] will require an implicit ClientContext[F, C]:

final case class ClientContextMetaData[C](context: C, metadata: Metadata)
trait ClientContext[F[_], C] {
def apply[Req, Res](
descriptor: MethodDescriptor[Req, Res],
channel: Channel,
options: CallOptions,
current: C
): Resource[F, ClientContextMetaData[C]]

While the new method MyService.bindContextService[F[_]] will require an implicit ServerContext[F, C]

trait ServerContext[F[_], C] {
def apply[Req, Res](descriptor: MethodDescriptor[Req, Res], metadata: Metadata): Resource[F, C]
}

I've chosen traits because the instances need to be parameterized over F and C but allow any Req and Res (this is passed in the apply). I'm open to suggestions for a more ergonomic or better way to provide these implementations.

AdrianRaFo
AdrianRaFo previously approved these changes Jan 17, 2022
@fedefernandez fedefernandez changed the title [WIP] Context service Context service Jan 17, 2022
@fedefernandez fedefernandez marked this pull request as ready for review January 17, 2022 10:35
@cb372
Copy link
Member

cb372 commented Jan 17, 2022

LGTM. It's probably worth updating the docs (e.g. https://higherkindness.io/mu-scala/guides/distributed-tracing) and the examples repo to reflect these changes. And maybe add a small how-to guide to the docs, showing how to use this feature to access the metadata?

@fedefernandez
Copy link
Contributor Author

@cb372 yes! Initially, I was thinking about adding that in this PR, but this is getting bigger, and I prefer to keep this for code/implementation discussions and then open an issue to address that. Good point with the guide

Copy link
Member

@juanpedromoreno juanpedromoreno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides my minor comment, shall we tag this PR as breaking-change?

@fedefernandez
Copy link
Contributor Author

@juanpedromoreno this should be backward compatible (besides the deprecated warning) so I think this should be fine without the breaking-change tag. I'll take a look at the val names.

@fedefernandez fedefernandez merged commit 73930c4 into main Jan 20, 2022
@fedefernandez fedefernandez deleted the context-service branch January 20, 2022 19:02
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 this pull request may close these issues.

4 participants