Skip to content

Commit

Permalink
lint fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Pradeep Mishra <pradeep.mishra@booking.com>
Signed-off-by: Pradeep <pradeepbbl@gmail.com>
  • Loading branch information
Pradeep Mishra authored and pradeepbbl committed Sep 24, 2024
1 parent ba292ec commit e8fe78d
Showing 1 changed file with 18 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# gRPC Custom Name Resolver Proposal (DRAFT)
# gRPC Custom Name Resolver Proposal (DRAFT)

## Details

Expand All @@ -20,7 +20,8 @@ breaking because the proxies need the destination domain i.e. service name in or
For such cases the gRPC core libs support few alternative resolver* also expose required interface to build custom one
you can find more details in the document referenced below

#### Ref:
### Reference

* [Custom Name Resolution](https://grpc.io/docs/guides/custom-name-resolution/)
* [Java Client](https://grpc.github.io/grpc-java/javadoc/io/grpc/ManagedChannelBuilder.html#forTarget(java.lang.String))
* [Golang](https://pkg.go.dev/google.golang.org/grpc#NewClient)
Expand Down Expand Up @@ -64,15 +65,15 @@ The idea is to
* and allow a new config option to pass the [target](https://grpc.io/docs/guides/custom-name-resolution/#life-of-a-target-string)
string

##### Target String Pattern*
### Target String Pattern*

There is no restriction on naming but the string most comply with below standard

```
```text
scheme://authority/endpoint_name
```

**Scheme Suggestion**
#### Scheme Suggestions

We can choose our own scheme considering it's not used in gRPC core

Expand All @@ -82,13 +83,12 @@ We can choose our own scheme considering it's not used in gRPC core
| **grpc-remote://** | | Approved / Reject |
| **tbd** | | Approved / Reject |


**Authority**
##### Authority

Authority needs to be a valid tcp endpoint of proxy/service discovery agent (passed by the user)
e.g. `localhost:9211`

**Endpoint Name**
##### Endpoint Name

The endpoint also specific to user deployment environment which define the `flagd` or `sync` backend
service name i.e. VirtualHost. This is used by the provided authority i.e. proxy service where to
Expand All @@ -101,12 +101,12 @@ flagd.sync.v1.FlagSyncService.GetMetadata
flagd.sync.v1.FlagSyncService.SyncFlags
```

**Samples**
##### Samples

* ``[ flagd || grpc-remote ]://[ 127.0.0.1:9211 ]/[ flagd-sync.service ]``
* ``[ flagd || grpc-remote ]://[ proxy.domain:443 ]/[ flagd-sync.service ]``

## Drawbacks
##### Drawbacks

* One of the big drawback was limited support of the language only `java` and `golang`
* Will introduce inconsistent user experience
Expand All @@ -117,27 +117,28 @@ providing sdks similar to [custom connector](https://github.com/open-feature/jav
## Alternatives

### Option-1

Allow users to override default ``authority`` header as shown above in `grpcurl`, the override option was
already supported by all major languages*

* [Golang](https://pkg.go.dev/google.golang.org/grpc#WithAuthority)
* [JAVA](https://grpc.github.io/grpc-java/javadoc/io/grpc/ForwardingChannelBuilder2.html#overrideAuthority(java.lang.String))
* [Python](https://grpc.github.io/grpc/python/glossary.html#term-channel_arguments)
* [Golang](https://pkg.go.dev/google.golang.org/grpc#WithAuthority)
* [JAVA](https://grpc.github.io/grpc-java/javadoc/io/grpc/ForwardingChannelBuilder2.html#overrideAuthority(java.lang.String))
* [Python](https://grpc.github.io/grpc/python/glossary.html#term-channel_arguments)

this option is simple and easy to implement, although it will not cover all the cases it will at least help with proxy
setup where `host_header` was used to route traffic.

**Ref**:

Java PR: https://github.com/open-feature/java-sdk-contrib/pull/949
Java PR: <https://github.com/open-feature/java-sdk-contrib/pull/949>

**Note**: JS, .NET, PHP still need to be explored if this options available

### Option-2

Only support the [xDS](https://grpc.io/docs/guides/custom-load-balancing/#service-mesh) protocol which already supported by gRPC core and doesn't require any custom
name resolver we can simply use any `target` string with `xds://` scheme. The big benefit of this approach was
it's going to be new stranded when it comes gRPC with service mesh and eliminate any custom implementation in `flagd`
Only support the [xDS](https://grpc.io/docs/guides/custom-load-balancing/#service-mesh) protocol which already supported by gRPC core and doesn't require any custom
name resolver we can simply use any `target` string with `xds://` scheme. The big benefit of this approach was
it's going to be new stranded when it comes gRPC with service mesh and eliminate any custom implementation in `flagd`
and the gRPC core team actively adding more features e.g. mTLS

For more details refer the below document
Expand All @@ -149,7 +150,6 @@ For more details refer the below document

TBD


## Unresolved questions

* What to do with un-supported languages
Expand Down

0 comments on commit e8fe78d

Please sign in to comment.