#1225 Update ServiceDiscovery documentation and samples to include Custom Providers

[leonluc-dev]

Closes #1225

• 1225

The new section in service discovery documentation on how to implement a custom service provider.

Proposed Changes

• Add "Custom Providers" section to Service Discovery documentation
• Add new OcelotServiceDiscovery sample with initial solution of custom service discovery provider

[raman-m]

Hi @leonluc-dev ! Leon or Leonluc? So great to see that people working on writing docs! 🤣 I will check tomorrow...

[raman-m]

Hey @leonluc-dev ! Did you check current open issues and maybe there is some issue which is related to custom ServiceDiscovery providers? It would be highly welcomed to attach an issue to this PR.

[raman-m]

@leonluc-dev I expect that you have some real working solution. Did you write some unit tests and/or acceptance tests for this "Custom service discovery providers" feature?

[leonluc-dev]

@raman-m Thank you for the response.

I made no modifications to the Ocelot code itself. What I've documented was an already present and tested Ocelot interface (IServiceDiscoveryProvider) I've been using in my own code for a while now using the public Ocelot NuGet packages.

Most if not all of the Ocelot code this documentation example utilizes is already unit tested through the ServiceDiscoveryProviderFactory and related tests: ServiceDiscoveryProviderFactoryTests

While there have been no Github issues directly related to this particular interface, there have been questions about custom service discovery in the past (example: #1225)

If this interface/code was not meant to be used in that sense then this PR can be closed without merge.

Otherwise I do have a working project where I utilize a custom service discovery provider. I could turn that into a proper sample and add it to samples if necessary? (I should have a bit of time for that later this or next week)

[raman-m]

@leonluc-dev Thanks for the reply!

Otherwise I do have a working project where I utilize a custom service discovery provider. I could turn that into a proper sample and add it to samples if necessary? (I should have a bit of time for that later this or next week)

It is not required, but it will be highly welcomed to add.

Regarding tests. Yes, current tests should cover custom provider. I am not sure the example in docs is canonic. But, real working solution in samples will be very useful. In the future we can link a Sample app from this doc to show how to build custom providers.

Once again, Thanks for sharing of your practical case! That's how to make Ocelot customizable, even in such kind of critical features of Ocelot core.

[leonluc-dev]

@ramam-m I have added the sample for custom service discovery providers to the PR.

[raman-m]

It was bad idea to include the new sample project right to this PR which was about docs updating. Now we have broken pipelines and I need to review once again. It was better to create separate PR. Well... Now it is too late. I forgot about asking you to prepare second PR...

Leon, you should expect that this PR will have long story...

[leonluc-dev]

@raman-m Apologies for the misunderstanding.

If it makes it easier, I can rebase the branch and remove the commit involving the sample from this PR. Afterwards I can add the sample to a new seperate PR.

[raman-m]

Leon, Do nothing with this PR. I've started code review...

And just FYI, your develop branch is not canonic now. And your source repo develop branch should have the same top commits in target/base branch. You will be blocked in creation of new feature branches, because both develop branches must be synchronized always. Hope you see...

[leonluc-dev]

@raman-m The requested changes should be addressed now.

[raman-m]

Hi Leon! Thanks for fixing code review issues!

I've added first commit as the result of code review: Add 2 options/ways of solution development via ConfigureServices On my opinion we need to give more flexibility for developer to choose between easy and hard ways of implementation. This commit contains my idea how it could be. If we will develop this idea, finally we need to update our new "Custom Providers" paragraph in Service Discovery docs.

Could you read and share your opinion plz?

Later I will upgrade the sample app as it was done for k8s sample... Finally, we have to ensure that the sample works without problems on 100%.

[raman-m]

@leonluc-dev Could you fix code review issues please?

[leonluc-dev]

@leonluc-dev Could you fix code review issues please?

Requested adjustments have been made.

[raman-m]

@leonluc-dev Leon, Thank you for your idea about custom providers for Service Discovery feature. We didn't develop the feature, but we've prepared important documentation update and finished a great sample how to build custom Service Discovery providers. Great pair programming! ❤️

@TomPallister Hi Tom! We have a next piece of art to merge to develop branch! 😺 The main achievement of this PR is a new sample web app for creation of custom Service Discovery providers. Also the docs were updated. I believe the issue #1225 will be closed by this PR. The rest is Dynamic Routes registration based on current custom service discovery provider. Should we work more on Dynamic Routes registration in case of #1225 as it was requested in #340 ? What do you think, Tom?

[raman-m]

@leonluc-dev The feature branch has been rebased onto ThreeMammals:develop! Welcome to code review!

[raman-m]

@schwede Hi Swede! Could you look at this PR and review the sample please? Do you like it? Did you get answers on your questions from original your issue?

[raman-m]

@leonluc-dev Congrats! 🎉