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

Generics #1189

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Generics #1189

wants to merge 5 commits into from

Conversation

sagikazarmark
Copy link
Contributor

This is just a very rough, first draft of how generics would look like in Go kit if it was available in Go today.

Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
@peterbourgon
Copy link
Member

Neat! So I haven't fully explored this yet, but I think the entire endpoint layer might be able to go away with generics — the transport layer should be able to yield requests and responses directly to the service layer. WDYT?

@sagikazarmark
Copy link
Contributor Author

You might be right. The only value I see in the endpoint layer is being able to construct service and transport independent middleware. Don't know how that would look like without the dedicated endpoint layer, but it might be possible.

@mcosta74
Copy link

Neat! So I haven't fully explored this yet, but I think the entire endpoint layer might be able to go away with generics — the transport layer should be able to yield requests and responses directly to the service layer. WDYT?

to be honest I think also with generics the endpoint layer still makes sense: when I have to support multiple transports (for instance HTTP and gRPC) I use the endpoint layer to do data validation before to call the service

@illotum
Copy link

illotum commented Dec 2, 2021

Isn't it more natural to do data validation at the service layer? I think there is no objective reason for the endpoint layer to exist. It only steals work from either the transport or the service.

I also believe there is value in reducing the amount of generic code the consumer has to write. Generic signatures are novel and perceived as ugly by some.

@sagikazarmark
Copy link
Contributor Author

Authentication is one great example for an endpoint layer feature: once you have the token from the transport, you want to authenticate the user.

You want to do it for every transport and every/most service.

Observability is another good example.

@mcosta74
Copy link

mcosta74 commented Dec 2, 2021

Isn't it more natural to do data validation at the service layer? I think there is no objective reason for the endpoint layer to exist. It only steals work from either the transport or the service.

It is matter of taste; I personally prefer to call the service only after the data is validated.

@peterbourgon
Copy link
Member

peterbourgon commented Dec 2, 2021

Isn't it more natural to do data validation at the service layer?

"Data validation" is an umbrella term that covers both protocol or schema validation — is this a valid HTTP request? does it contain a valid JSON object? does the "name" key correspond to a single string value? etc. — which should occur at the transport layer; and business or semantic validation — is the phone number in the correct format? is the provided transfer amount greater than zero? etc. — which should occur at the service layer.

@mcosta74
Copy link

mcosta74 commented Dec 2, 2021

is the phone number in the correct format? is the provided transfer amount greater than zero? etc. — which should occur at the service layer.

At transport layer you can check that the request has the right syntax (well formed JSON, correct gRPC, ....), the you have the "semantic" checks (is the string a valid email address, if field a greater than field b, ....).

Coming from other languages/tools I'm used to call the service on data that are already validated (syntax and semantic). As I said is matter of separation of concerns and both choices are valid.

I'm not against to remove the endpoint layer but, if you ask me, it still has reasons to exist.

@peterbourgon
Copy link
Member

The transport layer should not check semantic validity, because semantic validity is a property of, or defined by, or 1-to-1 with, the service itself. Whereas schema and protocol validity is a property of the transport used, and a single service can have multiple transports.

It's a separation of concerns thing. Let the transport layer just do work related to the transport layer. Of course this rule, like any rule, can be broken.

@mcosta74
Copy link

mcosta74 commented Dec 3, 2021

The transport layer should not check semantic validity, because semantic validity is a property of, or defined by, or 1-to-1 with, the service itself. Whereas schema and protocol validity is a property of the transport used, and a single service can have multiple transports.

It's a separation of concerns thing. Let the transport layer just do work related to the transport layer. Of course this rule, like any rule, can be broken.

what I was saying is that I'm used to do "syntax" validation (right JSON, protobuf, ...) in transport, "semantic" validation in endpoint and use the "validated data" in the service.

@peterbourgon
Copy link
Member

what I was saying is that I'm used to do "syntax" validation (right JSON, protobuf, ...) in transport, "semantic" validation in endpoint and use the "validated data" in the service.

Ah!

In theory this makes sense, but in practice it's rather awkward. The endpoint layer works with abstract requests and responses, currently defined as interface{}. You can do a lot of stuff without knowing the specific types of the RPC input and output: rate limiting, some amount of observability, etc. But if you want to validate you have to do type assertions, and that kind of violates the core invariant of the endpoint layer, which is that requests and responses are opaque.

If it works for you then 👍 and no problem. But in the general case, and IMO, semantic validation is better expressed at the service layer, probably as a service middleware.

Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
@sagikazarmark
Copy link
Contributor Author

I did some more experiments. In the http3 package I eliminated the dependency on the endpoint package.....by copying over the Endpoint type to the package. Technically it eliminates the "layer", although we still need the abstraction it provides.

It's also a bit unclear how we want to support transport and service independent middleware without an endpoint layer. Do we agree to use the same endpoint signature everywhere, so that we don't have to implement middleware individually for each transport?

Another thing we should address while we are here is the endpoint vs service error thing. Namely: it feels rather unnatural to return errors packaged into a response struct and the translation/encoding of errors vs reporting is also rather weird today.

@msf
Copy link

msf commented Jan 26, 2022

@peterbourgon just fwiw, I've reviewed go-kit and decided to adopt it explicitly without using endpoint layers.
So, in production right now, there are people using go-kit without endpoint abstraction with Go 1.13 -> 1.17.
The rationale was: reduce levels of abstraction and avoid completely the loss of typing and interface{} mess.
The way we've structured here at Unbabel.com is:

  • we use transports as per designed
  • at the transport package we use encoders/decoders into service-specific datatypes
  • our transport methods become:
 1 for each argument decodeGRPCTypesIntoNativeType(arg)
 1. out, err := s.srv.RealService(nativeType1, nativeType2)
 1. if err ...
 1. return encodeIntoGRPCType(out)

within the application/service, we have "client" modules to implement client libraries to talk to our downstream dependencies, at that layer we also don't use endpoints because the developer expectation of having implementations for remote apis as "client libraries" is already widespread and natural.

@peterbourgon
Copy link
Member

we use transports as per designed . . . at the transport package we use encoders/decoders into service-specific datatypes

Sure! This just means your services are tightly coupled to your transports. If that's not an issue for you, then great!

I've reviewed go-kit and decided to adopt it explicitly without using endpoint layers.

Honestly the best way to "use" Go kit is to cop it's architectural model and not actually import any of its packages at all 😉

@jerome-laforge
Copy link
Contributor

Go 1.18 is out for few months.
According to you, is it planned that Endpoint uses generics ?

@sagikazarmark
Copy link
Contributor Author

The idea is to use generics, but it hasn't been properly validated yet. In fact, it seems that adding generics might make things more complicated than they should be with the only benefit to make the transport layer more typesafe.

@pobochiigo
Copy link

pobochiigo commented Sep 17, 2022

Hey,
So any progress so far on how go-kit evolves with generics stuff?

@sagikazarmark
Copy link
Contributor Author

May previous comment stands: generics might not be the right path for this library.

@RangelReale
Copy link

I took some ideas from this PR and created a standalone package to implement go-kit endpoints with generics. It basically present a generic interface and internally calls the standard go-kit library converting the parameters as needed.

github.com/RangelReale/go-kit-typed

To make it easier to see the advantages/disadvantages, I converted the examples repo to generics using this library, this link is for comparing with the current code (click the "files changed" tab).

Please tell me what you think.

@longit644
Copy link

Hi all, any updates on this?

@francipvb
Copy link

Hello all,

Is there any activity on this repository and what about this PR?

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.

10 participants