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

Support separate consul discovery properties for grpc and http services #250

Closed
OverDrone opened this issue Oct 10, 2021 · 9 comments
Closed
Labels
enhancement Auto-generates notes
Milestone

Comments

@OverDrone
Copy link

OverDrone commented Oct 10, 2021

Hello!
Starting from some version now grpc automatically registers itself on consul. I have TWO registrations now: <service name> and grpc-<service name> both with same set of tags. There is no way I can separate those two by tags or keep only <service name>. The only way is to filter by service name prefix grpc-, which is a very bad practice (what if I want to name my service grpc-something?). Currently I have to use awkward spring.autoconfigure.exclude (which doesn't concat automatically if specified in several profiles, see https://stackoverflow.com/questions/41170316/combine-list-from-multiple-spring-boot-yaml-files).
Here are few suggestions I have:

  1. Add a separate setting for enabling grpc consul (and probably other servers) registration. See OnConsulRegistrationEnabledCondition which requires both spring.cloud.service-registry.enabled and spring.cloud.consul.service-registry.enabled flags. We can disable all auto registration or only consul auto registration this way. For grpc you might wanna check both spring.cloud.service-registry.enabled and spring.cloud.grpc.discovery.service-registry.enabled
  2. Add grpc service registration tags which override spring.cloud.consul.discovery.tags when applied to grpc service registration like spring.cloud.grpc.discovery.tags
  3. Extract grpc- hardcode into config to something like spring.cloud.grpc.discovery.service-prefix
@jvmlet
Copy link
Collaborator

jvmlet commented Oct 10, 2021

@OverDrone , where did proxy- prefix come from ? May be you meant grpc- prefix ?
Can you please confirm you have spring-boot-web starter dependency as well in your app ?

@OverDrone
Copy link
Author

OverDrone commented Oct 10, 2021

where did proxy- prefix come from

Yes, sorry, I meant grpc-. Edited original post

Can you please confirm you have spring-boot-web starter dependency as well in your app

Yes, I have spring-boot-starter-web

@jvmlet
Copy link
Collaborator

jvmlet commented Oct 10, 2021

And your intention is to discover web services (excluding grpc services) by tags only ? Don't you discovery by service id ?
Can you please post your discovery logic ?

@jvmlet
Copy link
Collaborator

jvmlet commented Oct 10, 2021

what if I want to name my service grpc-something

In this case grpc service registered name for app named grpc-something will be grpc-grpc-something

@OverDrone
Copy link
Author

OverDrone commented Oct 10, 2021

I use nginx consul template which reads info from consul and builds nginx config. Process rely on tags to determine whether to include this service in route, what url suffix to use etc. But generally that doesn't matter much. Generally I see no reason keeping 2 registrations on consul for the same app instance. Also this way consul would call health checks twice for the same instance. Ofcause I can't say that my usecase is valid for all users out there, I've seen your configuration where for each GRPC service you create separate registration on consul. But at least give an option to control this. spring.cloud.service-registry.enabled is too generic, you need to have more specific option

@OverDrone
Copy link
Author

OverDrone commented Oct 10, 2021

In this case grpc service registered name for app named grpc-something will be grpc-grpc-something

yes, and some logic that depends on service name prefix would see 2 services instead of 1: grpc-something and grpc-grpc-something. Actually those were my thoughts on a design, I don't have a problem with service naming. The only problem is that I need to somehow separate those 2 consul registrations for the same service (for example by tags) or disable one completely how it was for some earlier version of this lib.

@jvmlet
Copy link
Collaborator

jvmlet commented Oct 10, 2021

Generally I see no reason keeping 2 registrations on consul for the same app instance. Also this way consul would call health checks twice for the same instance.

The health check is performed on service instance, not on app instance. I can imagine the situation where single app instance provides several services instances - some of them can become unhealthy and the rest services remain healthy. And its up to you to decide how to check health (globally or per service). Moreover , if you serve both http and grpc services in the same app instance, the health check is still not the same , even if you health-check globally, but per protocol (http and grpc) .

But I do understand the need to distinguish services at least by tag and the requirements to provide different set of tags for http and grpc services separately. The fastest you can get is that I add NOOP registration strategy in the nearest snapshot.

Later, I'll also provide configuration for ConsulDiscoveryProperties source binding (defaults to spring.cloud.consul.discovery prefix)

@OverDrone
Copy link
Author

Yes, that should do it for me, thx. I don't require hotfix solution. I can live with spring.autoconfigure.exclude for some time. Take your time to build quality product :)

@jvmlet
Copy link
Collaborator

jvmlet commented Oct 10, 2021

@OverDrone , FYI, see #251 , 4.5.8-SNAPSHOT should be published in few minutes

@jvmlet jvmlet changed the title Disable grpc registration on consul Support separate set of tags for grpc services in case both grpc and http services registration Oct 31, 2021
@jvmlet jvmlet added the enhancement Auto-generates notes label Oct 31, 2021
@jvmlet jvmlet added this to the 4.5.9 milestone Oct 31, 2021
@jvmlet jvmlet closed this as completed in b13a9f8 Nov 1, 2021
@jvmlet jvmlet changed the title Support separate set of tags for grpc services in case both grpc and http services registration Support separate consul discovery properties for grpc and http services Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Auto-generates notes
Projects
None yet
Development

No branches or pull requests

2 participants