Addons: make the API endpoint as cacheable as possible

[humitos]

Once we are fully migrated to the new Addons, we will be hitting the /_/addons/ endpoint a lot. This endpoint will be hit by every single request on a documentation page view. We would like to make this endpoint as fast as possible, but also try to cache it to avoid hitting our servers. It can be compared with the current Footer API.

The /_/addons/ endpoint needs to be cached by:

• url query string parameter
• X-RTD-Hosting-Integrations-Version HTTP header
• X-RTD-Hosting-Integrations-API-Version HTTP header (once API: return the api_version on the response #10276 gets implemented)

Note the endpoint is behind authentication, so that it's not an issue here.

To build the response, the endpoint depends on:

• Project instance
• Version instance
• Build instance

We need to use the "backend resolver" to build the base_url required by the DocDiff addons. That URL depends on the exact filename of the page, so we need cache by url parameter. We could talk about different ideas here to be able to not depend on it if we want to. Some ideas the mentioned:

• create a new endpoint to use the backend resolver, and call it for docdiff
• copy / re-implement the "backend resolver" logic in the frontend
• make the current endpoint "expandable" and only return docdiff data when users enable the docdiff in their docs
• add a new query string parameter docdiff=[true|false] that's only true on pageviews from *.readthedocs.build

The last item seems to be the easiest one and will give us most of the benefits, since we 95% of our traffic will call the endpoint with docdiff=false 😄

[humitos]

X-RTD-Hosting-Integrations-Version HTTP header
X-RTD-Hosting-Integrations-API-Version HTTP header

We also talked about moving these to query string paramenters because it's easier to cache them than HTTP headers: https://developers.cloudflare.com/cache/how-to/cache-keys/#create-custom-cache-keys

[humitos]

I took a look at the Algolia integration we have setup on https://rtd-with-docsearch.readthedocs.io/en/latest/ and it seems they send all the versions in the query string. Something to consider 😉

[humitos]

We also talked about moving these to query string paramenters because it's easier to cache them than HTTP headers

I implemented this part in #10753 and readthedocs/addons#138

[humitos]

Currently, we are using unresolver.unresolve_url(url) to get the Project and Version objects. I don't think we can easily get rid of the url= parameter since we need to, somehow, communicate to the endpoint the project and version the user is reading.

However, it would be interesting to figure it out how to do this because in that case, it will allow us to cache the request only "per project & version & build ¹" instead of "per project & version & build & exact URL" as we are currently doing now. There are some ideas here:

1. use the Referer HTTP header injected automatically by the browser
2. inject some meta HTML tags in the CF worker: <meta name="project_slug" content="test-builds"> and <meta name="version_slug" content="latest">. Then our JS client could send them in the request /_/addons/?project_slug=test-builds&version_slug="latest"

I think 1) is probably the easiest and simplest option since 2) seems pretty hacky 😅 . Following that pattern, I think this could be as follows:

• requests made from regular documentation pages:
  • get the Referer HTTP header and call unresolver.unresolve_url(referer)
  • cache these requests normally (using the project:version CDN tags we are already using)
  • do not return any data that depends on the exact URL
• requests made on documentation pages that depend on their exact URL (e.g. pull request previews)
  • send the url= attribute with the window.location.href value
  • use the url= attribute to call the unresolver
  • populate response data that depends on the exact URL (e.g. doc_diff.base_url)
  • cache these requests normally. They won't interfere with the previous cached requests because they have an extra url= attribute. They also won't interfere between themselves because they all have different url= values and CF will treat them differently 💪🏼

This sounds pretty simple to understand and follow in my head, but I've been thinking on this pattern for some time already. I'd like to get some feedback on this and decide how we want to move forward.

Note that this will have an impact in our web instances and also in our database. Besides, it will make our addons rendering a lot faster since most of the requests to the endpoint will be cached and returned directly from CF. Currently, the numbers for this endpoint in the last 7 days the response times are:

• AVG: 129 ms
• Median: 76.5 ms
• 95th percentile: 160 ms

Footnotes

1. the build is automatically fetched (no need to pass it to the endpoint) by querying the latest successful build for that particular version. ↩

[humitos]

get the Referer HTTP header and call unresolver.unresolve_url(referer)

Hrm, thinking a little more about this, isn't this the same as passing the url= attribute? I mean, instead of vary the cache on the url= we will need to vary it on Referer header, which will have exactly the same behavior, right?

Maybe option 2), even being hacky 🕵️, is the only option that works how we want 😄

[humitos]

Oh, wait! We are already returning X-RTD-project and X-RTD-Version headers to the browser 🚀 . This solves this problem in a much better way 💯

$ curl -sIL https://docs.readthedocs.io/en/stable/ | grep x-rtd-
x-rtd-domain: docs.readthedocs.io
x-rtd-path: /proxito/html/docs/stable/index.html
x-rtd-project: docs
x-rtd-project-method: public_domain
x-rtd-version: stable
x-rtd-version-method: path

With that data we can call our addons API as follows: /_/addons/?project__slug=<X-RTD-Project>&version__slug=<X-RTD-Version>

One thing that I'm not sure how to do is how to get those headers from a request that was done before our readthedocs-addons.js file was loaded. If there is no way to do that, we would need to do an extra HEAD request to get them:

fetch(window.location.href, {method: "HEAD"})
  .then((response) => { 
    const projectSlug = response.headers.get("x-rtd-project"))
    const versionSlug = response.headers.get("x-rtd-version"))
    fetch("/_/addons/?" + new URLSearchParams({ "project__slug": projectSlug, "version__slug": versionSlug})
      .then((response) => {
         // ...
      });
  }
); 

[humitos]

Even doing an extra HEAD request it will be much better. We can re-search if that possible and manage it as an improvement.

With this approach, we will have two different requests:

1. Including url= parameter: will be made if there is any addons enable on the page that depends on the URL itself. DocDiff addon is a good example of this. Then, on pages that have DocDiff enabled (pull request previews), our JS will hit /_/addons/?project__slug=...&version_slug=... + &url=...
2. Omitting the url= parameter: if there aren't any addons enabled on the current page that requires sending the exact URL to the API, the request won't include it: /_/addons/?project__slug=...&version__slug=...

With this, our CDN will handle the cache correctly since it will treat the URLs containing only the project__slug and version__slug as the same, no matter what page under the documentation the user is reading. Also, it will treat all the URLs containing url= different for all the different pages of the documentation.

[humitos]

Now, with this, we will need to know before performing the request what addons "could be enabled" on the current page to decide whether or not include the url=. We will need to ask each addon if they could be enabled, and in that case, if they require sending the url= parameter to the backend. Then, if none of the addons that could be enabled on the page needs to send the url= parameter, we don't send it 👍🏼

I imagine something like:

let addons = [ ... ];
let params = {"client-version": "...", "api-version": "..."};
let sendUrlParam = false;
for (const addon of addons) {
  if (addon.requiresURL() && addon.couldBeEnabledOnPage()) {
    sendUrlParam = true;
    break
  }
}

if (sendUrlParam) {
  params.url = window.location.href;
}

...

fetch(url, ....);

Then, an addons like DocDiff would implement:

export class DocDiffAddon extends AddonBase {

  static requiresUrl() {
    return true;
  }

  static couldBeEnabledOnPage() {
    return window.location.host.endsWith(".readthedocs.build");
  }

}

Links to addons code:

• https://github.com/readthedocs/readthedocs-client/blob/ef72cba9a6bb22d125bce73c8a17bbcb15fd7b3d/src/readthedocs-config.js#L7-L30
• https://github.com/readthedocs/readthedocs-client/blob/ef72cba9a6bb22d125bce73c8a17bbcb15fd7b3d/src/index.js#L11-L57

[ericholscher]

@humitos I don't love the idea of doing an extra HEAD request, because that will increase latency on the rendering of the addons (2 complete HTTP round trips after page load). If we're already injecting data into the page to add the addons JS, then injecting the value of the project & version slug doesn't seem like additional hackiness, and if it saves us that round trip, likely worth doing.

I don't believe there's a way to capture the headers from the request in our JS, which would be a really nice solution to the problem.

1. Including url= parameter: will be made if there is any addons enable on the page that depends on the URL itself. DocDiff addon is a good example of this. Then, on pages that have DocDiff enabled (pull request previews), our JS will hit /_/addons/?project__slug=...&version_slug=... + &url=...
2. Omitting the url= parameter: if there aren't any addons enabled on the current page that requires sending the exact URL to the API, the request won't include it: /_/addons/?project__slug=...&version__slug=...

Is this the plan we're going with, assuming that we get the project & version slug somehow..., either via a HEAD request, or injected into the page? This logic looks 💯 to me. I think the above logic is great, and then we can try injecting the project/version values in the CF worker, and if that doesn't work, always fall back to a HEAD request.

(Or some other idea we haven't come up with yet 🤔... I think perhaps we could also just parse the URL in JS and only send the hostname and matching version path to the API, but that's harder with the custom URLconf work we've been doing, which would be additional data coming from the server that we need to have in the client, which is the primary problem we're trying to solve here..)

[humitos]

Thanks @ericholscher for your input here.

Summarizing, this is the current plan that I will implement then:

• inject project/version slug on the HTML using <meta> tags via CF worker with the same script we are using to inject the addons js
• grab project/version slug via our addons js and send them to the API
• implement the Addon.couldBeEnabledOnPage js function on each addon to decide whether or not send url= attribute
• send url= attribute if there is at least one addon that has to be enabled on the page

Let me know if you agree with the current plan.

I think perhaps we could also just parse the URL in JS and only send the hostname and matching version path to the API, but that's harder with the custom URLconf work we've been doing

I thought about this already and I didn't see it clearly since we would have to re-implement the Python resolver logic into JavaScript and always keep it updated with all the changes/feature we introduce on it. I thought that would be fragile and we will end up with bugs on both making them behave differently and hard to debug in the end 🤷🏼

[ericholscher]

@humitos that plan sounds 💯.

[humitos]

inject project/version slug on the HTML using <meta> tags via CF worker with the same script we are using to inject the addons js

This is done and deployed already. See it in action at https://test-builds.readthedocs.io/en/latest/