#738 #1990 Route Metadata as custom properties

[vantm]

Closes #738 #1990

• Configuration for use by extensions, custom pipelines, or other extensions #738
• Proposal to add ExtraProps to the configuration #1990

Related to

• Extending Route configuration #651

Proposed Changes

• Adding metadata to the global configuration
• Adding metadata to the route configuration

Route metadata allows Ocelot users to add custom functions that solve ad-hoc requirements or maybe write their plugins/extensions.

The new configuration will look like:

{
  "Routes": [
      {
          "UpstreamHttpMethod": [ "GET" ],
          "UpstreamPathTemplate": "/posts/{postId}",
          "DownstreamPathTemplate": "/api/posts/{postId}",
          "DownstreamHostAndPorts": [
              { "Host": "localhost", "Port": 80 }
          ],
          "Metadata": {
              "api-id": "FindPost",
              "my-extension/param1": "overwritten-value",
              "other-extension/param1": "value1",
              "other-extension/param2": "value2",
              "tags": "tag1, tag2, area1, area2, feat1"
          }
      }
  ],
  "GlobalConfiguration": {
      "Metadata": {
          "server_name": "data-center-1-machine-1",
          "my-extension/param1": "default-value"
      }
  }
}

Now, the metadata is available in the DownstreamRoute object

var downstreamRoute = context.Items.DownstreamRoute();

if(downstreamRoute?.Metadata is {} metadata)
{
    var param1 = metadata.GetValueOrDefault("my-extension/param1") ?? throw new MyExtensionException("Param 1 is null");
    var param2 = metadata.GetValueOrDefault("my-extension/param2", "custom-value");

    // working with metadata
}

[raman-m]

Hi Van!
Thanks for PR submission!
Welcome to Ocelot world! 🐯

Please, Be patient! I need some time to read and understand linked issues...

[raman-m]

Extending route config by custom properties is interesting idea.
Please keep in mind we have #1183 which is similar.

[ggnaegi]

Extending route config by custom properties is interesting idea. Please keep in mind we have #1183 which is similar.

Are they really similar? The route metadata is a nice addition.

[raman-m]

@ggnaegi commented on Dec 10

Right, the technical designs are different.. But these both issues are about the same custom properties for route. Similarity in this thing only.
🙆‍♂️ I've put the Accepted label onto the linked issue #738

@RaynaldM Thanks for the review!
I have no idea which milestone we will include this PR in... Probable candidate for Jan'24 release if the author will be proactive...

[vantm]

All feedback has been resolved. Thank everybody for taking the time to work on my PR. Thank @RaynaldM for your reviews.

[raman-m]

• 1st concern, Why Metadata name?
  We have other names from linked issues: Extensions, see comment
  Internal code of Ocelot middlewares uses the HttpContext class Items dictionary to keep extra data of the route/request, see HttpContext.Items Property
  So, we use HttpContext.Items dictionary across all middlewares...

• 2nd concern is IDictionary<string, string> type of the Metadata property. But developer will use another types like int, double, even object... So, string data type looks like a restriction in data types we can define in JSON...

[vantm]

Hi @raman-m

1. • Why Use the Metadata Name? The linked issue lacks a detailed design; the term "Extensions" is merely a suggestion. In my opinion, "Metadata" carries a much broader meaning.
   • We use HttpContext.Items dictionary across all middlewares... As per my understands of Ocelot, it's a yes, but only when you want to access or manipulate configurations/states are stored here. Referencing these extension methods of HttpContext.Items here, it seems there is no restriction or issue of using them.
2. • I believe it's not advisable to manage entities beyond our scope. That means there is no strong-typed binding, metadata validation, complex metadata structure handling, etc. It is simply attaching extra information to a route. That is why I ended up with the approach of using key/pair data type (this idea is similar to the k8s metadata annotations). Enabling support for a wide array of data types would necessitate heavy design and implementation overhead.
   • Using string is a restriction in data types While string indeed restricts the data type, it doesn't restrict the data format. A string can express many formats such as numbers, plain text, JSON, XML, HTML, etc.

[ggnaegi]

I concur that merging is a good step, but it's better to address the issues I've identified. If you're short on time, I can tackle the problems highlighted in my code review. However, my immediate focus is on the current monthly release. Once that's handled, I'll return to this pull request.

@raman-m what monthly release? It wasn't planed like that!

[raman-m]

@ggnaegi

what monthly release? It wasn't planed like that!

This monthly release → March-April'24
And what we do now → Milestones

[ggnaegi]

@raman-m ready for re-review

[raman-m]

Ready for delivery ✅

• Code review ✔️✔️✔️
• Team approvals ✔️✔️✔️
• Unit testing ✔️✔️ → DefaultMetadataCreatorTests (link1) and DownstreamRouteExtensionsTests (link2)
• Acceptance testing ✔️ → DownstreamMetadataTests (link)
• Updated feature docs ✔️ → metadata.rst → link

[raman-m]

@ggnaegi Hey, mentor!
What about pressing magic Squash button? 🪄

[ggnaegi]

@raman-m in release/24.0, really? We can still revert it, but I'm not sure about the logic now...

[raman-m]

ThreeMammals:release/24.0 will be rebased onto develop soon, after current milestone release.
@ggnaegi What's your concern? Why to revert?
Do you want to merge everything to develop, into the current release?
We can discuss the process, seems you like GitHub workflow.
We have 2 parallel releases following Git workflow. But our work is too slow. Make sense to switch to GitHub workflow.
What I propose now is

• Make Spring'24 release
• Rebase release/24.0 onto develop
• Work hard on Annual 2023 milestone to close all issues
• Make Annual 2023 release during this summer

Sounds like a plan?

[raman-m]

The issue lies within the use of Milestones...
I attempt to structure and schedule releases by employing milestones. If we transition to a GitHub workflow (consolidating all changes into the develop branch) and adopt a monthly release cycle, then planning with Milestones seems redundant, rendering them essentially artificial.
Currently, I organize each release within a distinct milestone collection, allowing each release to have its own milestone. This is a feature unique to the Git workflow, which facilitates the development of releases in parallel. I have discovered that Milestones are beneficial and wish to continue utilizing them.
Another argument, milestone has own version, which is good. GitHub workflow makes a zoo of versions. I don't like this process personally.

[raman-m]

The release label has been updated to Spring'24❗

@ggnaegi, rest assured, this feature is slated for release in version 23.3.
Although the branch is incorrect, cherry-picking the feature to the develop branch should not pose a technical issue.

[ggnaegi]

@raman-m ok, fine, it's great!

[raman-m]

Feature commit in develop → 573a9d9