java-sdk-contrib icon indicating copy to clipboard operation
java-sdk-contrib copied to clipboard

feat: added custom grpc resolver

Open pradeepbbl opened this issue 1 year ago • 3 comments

This PR

Added custom gRPC resolver to support envoy proxy

  • support gRPC custom resolver using new config options tragetUri

Related Issues

Fixes #995

How to test

Unit test are already added for the custom resolver for integration test you need a working envoy proxy support or any of the supported core resolver mentioned here

FlagdOptions.builder()
      .targetUri("envoy://localhost:9211/b-features-api.service")
      .selector("test-project")
      .resolverType(Config.Resolver.IN_PROCESS)
      .build();
DEBUG [2024-10-08 00:00:37,661] io.grpc.netty.NettyClientHandler: [id: 0xef9bc3d3, L:/127.0.0.1:35368 - R:localhost/127.0.0.1:9211] OUTBOUND HEADERS: streamId=3 headers=GrpcHttp2OutboundHeaders[:authority: b-features-api.service, :path: /flagd.sync.v1.FlagSyncService/SyncFlags, :method: POST, :scheme: http, content-type: application/grpc, te: trailers, user-agent: grpc-java-netty/1.58.0, grpc-accept-encoding: gzip, grpc-timeout: 599837m] streamDependency=0 weight=16 exclusive=false padding=0 endStream=false
DEBUG [2024-10-08 00:00:37,664] io.grpc.netty.NettyClientHandler: [id: 0xef9bc3d3, L:/127.0.0.1:35368 - R:localhost/127.0.0.1:9211] OUTBOUND HEADERS: streamId=5 headers=GrpcHttp2OutboundHeaders[:authority: b-features-api.service, :path: /flagd.sync.v1.FlagSyncService/GetMetadata, :method: POST, :scheme: http, content-type: application/grpc, te: trailers, user-agent: grpc-java-netty/1.58.0, grpc-accept-encoding: gzip, grpc-timeout: 9969998u] streamDependency=0 weight=16 exclusive=false padding=0 endStream=false
DEBUG [2024-10-08 00:00:37,666] io.grpc.netty.NettyClientHandler: [id: 0xef9bc3d3, L:/127.0.0.1:35368 - R:localhost/127.0.0.1:9211] OUTBOUND DATA: streamId=5 padding=0 endStream=true length=5 bytes=0000000000
DEBUG [2024-10-08 00:00:37,667] io.grpc.netty.NettyClientHandler: [id: 0xef9bc3d3, L:/127.0.0.1:35368 - R:localhost/127.0.0.1:9211] OUTBOUND DATA: streamId=3 padding=0 endStream=true length=15 bytes=000000000a12086665617475726573
DEBUG [2024-10-08 00:00:37,719] io.grpc.netty.NettyClientHandler: [id: 0xef9bc3d3, L:/127.0.0.1:35368 - R:localhost/127.0.0.1:9211] INBOUND HEADERS: streamId=5 headers=GrpcHttp2ResponseHeaders[:status: 200, server: envoy, date: Tue, 08 Oct 2024 00:00:37 GMT, content-type: application/grpc, content-length: 0, grpc-status: 12, grpc-message: Method flagd.sync.v1.FlagSyncService/GetMetadata is unimplemented, x-envoy-upstream-service-time: 43] padding=0 endStream=true
DEBUG [2024-10-08 00:00:37,722] io.grpc.netty.NettyClientHandler: [id: 0xef9bc3d3, L:/127.0.0.1:35368 - R:localhost/127.0.0.1:9211] INBOUND HEADERS: streamId=3 headers=GrpcHttp2ResponseHeaders[:status: 200, server: envoy, date: Tue, 08 Oct 2024 00:00:37 GMT, content-type: application/grpc, grpc-encoding: gzip, grpc-accept-encoding: gzip, x-envoy-upstream-service-time: 44] padding=0 endStream=false

pradeepbbl avatar Oct 08 '24 00:10 pradeepbbl

Hey @pradeepbbl ! Thanks a lot for this. I was hoping to get to it this week, but I won't be able to. I will review next week. Sorry about that.

toddbaert avatar Oct 10 '24 23:10 toddbaert

Hey @pradeepbbl ! Thanks a lot for this. I was hoping to get to it this week, but I won't be able to. I will review next week. Sorry about that.

No worries take your time meanwhile I'm exploring way to have e2e test using local envoy container

pradeepbbl avatar Oct 11 '24 15:10 pradeepbbl

added a new e2e test coverage for envoy custom resolver

Test Name: RunFlagdInProcessEnvoyCucumberTest

pradeepbbl avatar Oct 12 '24 00:10 pradeepbbl

I only see one small issue: the way GLUE_PROPERTY_NAME name works, for example here, it resolves all subclasses under that package: so your RunFlagdInProcessEnvoyCucumberTest class is running BOTH the dev.openfeature.contrib.providers.flagd.e2e.process.envoy.FlagdInProcessSetup AND the dev.openfeature.contrib.providers.flagd.e2e.process.FlagdInProcessSetup. To fix this, just make these classes/packages siblings (and probably rename your setup class to something like FlagdInProcessEnvoySetup or something like that).

toddbaert avatar Oct 15 '24 17:10 toddbaert

@pradeepbbl I merged another PR that results in some (I believe minor) conflicts here, mostly with the setup of the providers for e2e tests. Just be sure to use:

.deadline(1000)
.streamDeadlineMs(0) // this makes reconnect tests more predictable

for the options in those.

I think that's the only thing to note, but if you have issues with the conflicts or e2e tests not working, please let me know and I will do what I can to help.

toddbaert avatar Oct 15 '24 18:10 toddbaert

I only see one small issue: the way GLUE_PROPERTY_NAME name works, for example here, it resolves all subclasses under that package: so your RunFlagdInProcessEnvoyCucumberTest class is running BOTH the dev.openfeature.contrib.providers.flagd.e2e.process.envoy.FlagdInProcessSetup AND the dev.openfeature.contrib.providers.flagd.e2e.process.FlagdInProcessSetup. To fix this, just make these classes/packages siblings (and probably rename your setup class to something like FlagdInProcessEnvoySetup or something like that).

@toddbaert thanks as suggested fixed the GLUE_PROPERTY_NAME and also the merge conflict, please have a look when you have free time.

pradeepbbl avatar Oct 15 '24 22:10 pradeepbbl

Excellent work, thanks so much. This is my only blocking change (which is small and I would do myself, but I can't edit your branch)

yeah sorry for that as per the company policy all our open-source contribution need to be done through company github org

pradeepbbl avatar Oct 15 '24 22:10 pradeepbbl

https://github.com/open-feature/java-sdk-contrib/pull/1022#issuecomment-2417567334

toddbaert avatar Oct 16 '24 18:10 toddbaert