A lot of fixes and some new functionality

[RobSchoenaker]

• Upgrade to ServiceStack 4.5.8
• Fix RegisterService
• Fix HealthCheckDelegate
• Fix versioning

NOTE: Breaking changes

[wwwlicious]

@RobSchoenaker Thanks for the PR but I cannot easily see what has changed so cannot accept the PR as is.

Can you resubmit the changes without a) reformating the code c) More of an explanation of what has changed, for example is upgrading to 4.5.8 required to access a ServiceStack API change and the nature of each fix including what the breaking change was?

[RobSchoenaker]

It's quite a list, but I must admit it took me quite some time to get this al working. It seems you made this for 'your situation' and I tried to stay as close to the code as I could. Since we use .Net 4.6.1 and SS 4.5.8 I had to upgrade the packages.

Per file changes and description: ConsulClient.cs

• GetServices changed. Parameter serviceName --> tagName
• Possibility to get a service based on a version. Naming convention: XXXXX ABCD where XXXXX is the name of the registered service and ABCDis the name of the service. Instead of always using the first service returned, use the version to detemine the right service (custom logic may be applied)
• JsConfig.BeginScope is used to take care of correct JSON serialization to Consul (if changed in AppHost)
• Get the services from Consul to determine the healty ones

ConsulServiceResponse.cs

• Used a KeyValuePair to story the service (prevent extra lookups)

ConsulUris.cs

• The URI in the code was wrong

ConsulFeature.cs

• Allow for NOT passing WebHostUrl to allow services to use other service without having their own callback and health checking
• Register only when the WebHostUrl is known
• DefaultGatewayDelegate parameter changed from a baseUri to a ConsulService reference. This allows for using different ServiceClients based on service type/info

ConsulFeatureSetting.cs

• Added ServiceName property
• Added ServiceId property (allows for registering the service under the same ServiceID upon restart)

ConsulDiscovery.cs

• Changed the way ServiceName and ServiceId are composed. Now, services are registered with names instead of 'api'
• Allow passing minVersion for a service
• Use ToGetUrl() for HealthCheck to be in line with the ServiceStack URI scheme
• Use IP for Redis, or registration may fail

IServiceDiscovery.cs

• Added minVersion parameter (optional)
• Added ResolveService methods to get the service bases on type or instance

ConsulServiceGatewayFactory.cs

• Resolve based on service instead of baseUri

ServiceStack.Discovery.Consul.csproj

• Upgraded to ServiceStack 4.5.8 to use the latest and greatest (not required)
• Upgraded to .Net Framework 4.6.1, since 4.5 is no longer supported

ConsulRegisterCheckValidator.cs

• Fixed RuleFor x.HTTP with would give errors it only TCP was provided

That's it :)

Tip: Use the SPLIT view to see the changes side by side

[wwwlicious]

@RobSchoenaker that is quite a list, thank you for that. 👍

Regarding the registration of services, the services were registered under the same name in order to support DNS lookups by tag.

[tag.]<service>.service[.datacenter].<domain>

If you register the services by each service name, you lose this ability as you would have to know which service you were querying; so it would make more sense to make this configurable, with the the default supporting DNS (by registering services with the same name) and an explicit override to support registering by service name if you don't need/want DNS support.

For the .net framework version, we want to keep parity with ServiceStack so will remain on 4.5 until they update their libraries and the same goes for the min ServiceStack version 4.0.58 which is when the IServiceGateway was introduced. Otherwise this forces anyone using the package to upgrade which we should avoid unless there are API changes to ServiceStack that make it unavoidable.

[RobSchoenaker]

The SS version and .Net framework version do not need to change, so please skip those changes. Can you point to the DNS you mean? In what way did I break this? How would this be useable in the code in your repo? For what I noticed is that consul is queried by the tagName (the actual Request DTO) which is fine. The changes I made handle this property on our end. So I am a bit confused now...

It took me about a week in order to get it all running. Happy to assist in new features though! There is a catch on the request DTO you may want to add to the documentation. Since Gateway.Send uses the IVerb derived interfaces to select the right method, please specify this in the docs (for Consul) or you will have people tear their hairs out when they get a POST to a GET only RequestDTO... At least I lost some :)

[RobSchoenaker]

I am not sure about JsConfigScope (reuiqred to make this work!)

[wwwlicious]

The DNS interface docs can be found here

This isn't something thats required for any ServiceStack apphost's when using the plugin but we use it outwith ServiceStack to both locate and aggregate all ServiceStack endpoints as part of some related projects that are still in development and use ServiceStack.IntroSpec for validation, documentation and making ServiceStack's service reference features work. The ability to query DNS for dto.api.service.consul is something we need but is not strictly required for this repo. It doesn't make much difference if they are registered with the same name IMO outwith the consul UI and it saves us from needing a servicestack-specific tag added to all registrations when aggregating a global catalog and provides the most flexible use-cases.

As I said, it can be wired up as a default with the ability to override should you want to customise the names the services are registered under.

Storing the lookups is something we considered but ultimately rejected at the time, as it would require us to handle entropy rather than let Consul deal with it. We felt the lookup overhead of querying the local agent was minimal. That said, I'm not against the idea, but if services become unhealthy or drop-off, how does the Gateway handle re-querying the agent?

Again it could be an option to provide a safe-default here (no lookup caching) and allow an override.

Versioning is something that can cause issues when aggregating the services so I'll need to check these changes more closely and sorting/selecting becomes problematic if it isn't numeric (just google sorting SemVer for a taste of how 'not great' this is!

It is also somewhat problematic to introduce the burden on the client to know which version of a service they are consuming. The work I'm doing on the related projects has brought this into sharp focus and it may be better in the long term to be quite opinionated about how this is done but this should probably be a topic on it's own.

Regarding the Doc's, definitely lots of room for improvement there so will update regarding the IVerb interfaces requirement and link to the ServiceStack docs where it also states this.

Really do appreciate the effort and contributions though so always glad to have help! 👍