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

Update dns-in-google-sheets.mdx to add caching in NSLookup #17298

Merged
merged 8 commits into from
Oct 24, 2024

Conversation

beltofte
Copy link
Contributor

@beltofte beltofte commented Oct 3, 2024

Adding internal caching of the results on the NSLookup function to limit the number of the DNS resolver requests and speed up the results - especially in larger Google Sheets.

Summary

The NSLookup function for Google Sheets provided in the original documentation page does not support internal caching of the results. This can lead to issues in Google Sheets with many calls to the function - this includes error 400 from the https://cloudflare-dns.com/dns-query if you use it too much.

It is possible to disable caching and control the cache TTL via arguments in the NSLookup function.

Screenshots (optional)

Documentation checklist

  • The documentation style guide has been adhered to.
  • If a larger change - such as adding a new page- an issue has been opened in relation to any incorrect or out of date information that this PR fixes.
  • Files which have changed name or location have been allocated redirects.

…kup function.

Adding internal caching of the results on the NSLookup function to limit the number of the DNS resolver requests and speed up the results - especially in larger Google Sheets.
@beltofte beltofte requested review from RebeccaTamachiro and a team as code owners October 3, 2024 11:48
@github-actions github-actions bot added the size/s label Oct 3, 2024
@github-actions github-actions bot added the product:1.1.1.1 Related to 1.1.1.1 product label Oct 3, 2024
@beltofte beltofte changed the title Update dns-in-google-sheets.mdx to include cache support in the NSLoo… Update dns-in-google-sheets.mdx to add caching in NSLookup Oct 3, 2024
type = type.toUpperCase();
domain = domain.toLowerCase();

if (usecache == true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be if (usecache) as usecache is validated to be boolean.

@@ -12,7 +12,7 @@ import { Details } from "~/components"
1.1.1.1 works directly inside Google Sheets. To get started, create a [Google Function](https://developers.google.com/apps-script/guides/sheets/functions) with the following code:

```js
function NSLookup(type, domain) {
function NSLookup(type, domain, usecache = true, cachettl = 1800) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value for usecache should be false, since correctness should be preferred over performance. Users who want to change that are free to do so.

- Change minCacheTTL from Math.min to Math.max 
- Change default value of cachettl from 1800 to 30.
@hunts
Copy link
Contributor

hunts commented Oct 23, 2024

Sorry for the confusing, I think our TTL calculation is still not right. This would be the changes I'd like to make before we can merge it.

Basically, use the minimal TTL from records, but clamp the lower bound at the minCacheTTL (which is 30 seconds). Also replaced var with const /let keywords by the way.

index df02454ac..480cd4be6 100644
--- a/src/content/docs/1.1.1.1/other-ways-to-use-1.1.1.1/dns-in-google-sheets.mdx
+++ b/src/content/docs/1.1.1.1/other-ways-to-use-1.1.1.1/dns-in-google-sheets.mdx
@@ -12,7 +12,7 @@ import { Details } from "~/components"
 1.1.1.1 works directly inside Google Sheets. To get started, create a [Google Function](https://developers.google.com/apps-script/guides/sheets/functions) with the following code:
 
 ```js
-function NSLookup(type, domain, usecache = false, cachettl = 30) {
+function NSLookup(type, domain, usecache = false, minCacheTTL = 30) {
 
   if (typeof type == 'undefined') {
     throw new Error('Missing parameter 1 dns type');
@@ -26,43 +26,44 @@ function NSLookup(type, domain, usecache = false, cachettl = 30) {
     throw new Error('Only boolean values allowed in 3 use cache');
   }
 
-  if (typeof cachettl != "number") {
-    throw new Error('Only numeric values allowed in 4 cache ttl');
+  if (typeof minCacheTTL != "number") {
+    throw new Error('Only numeric values allowed in 4 min cache ttl');
   }
 
   type = type.toUpperCase();
   domain = domain.toLowerCase();
 
+  let cache = null;
   if (usecache) {
     // Cache key and hash
     cacheKey = domain + "@" + type;
     cacheHash = Utilities.base64Encode(cacheKey);
     cacheBinKey = "nslookup-result-" + cacheHash;
 
-    var cache = CacheService.getScriptCache();
-    var cachedResult = cache.get(cacheBinKey);
+    cache = CacheService.getScriptCache();
+    const cachedResult = cache.get(cacheBinKey);
     if (cachedResult != null) {
       return cachedResult;
     }
   }
 
-  var url = 'https://cloudflare-dns.com/dns-query?name=' + encodeURIComponent(domain) + '&type=' + encodeURIComponent(type);
-  var options = {
+  const url = 'https://cloudflare-dns.com/dns-query?name=' + encodeURIComponent(domain) + '&type=' + encodeURIComponent(type);
+  const options = {
     muteHttpExceptions: true,
     headers: {
       accept: "application/dns-json"
     }
   };
 
-  var result = UrlFetchApp.fetch(url, options);
-  var rc = result.getResponseCode();
-  var resultText = result.getContentText();
+  const result = UrlFetchApp.fetch(url, options);
+  const rc = result.getResponseCode();
+  const resultText = result.getContentText();
 
   if (rc !== 200) {
     throw new Error(rc);
   }
 
-  var errors = [
+  const errors = [
     { name: "NoError", description: "No Error"}, // 0
     { name: "FormErr", description: "Format Error"}, // 1
     { name: "ServFail", description: "Server Failure"}, // 2
@@ -75,24 +76,25 @@ function NSLookup(type, domain, usecache = false, cachettl = 30) {
     { name: "NotAuth", description: "Not Authorized"} // 9
   ];
 
-  var response = JSON.parse(resultText);
+  const response = JSON.parse(resultText);
 
   if (response.Status !== 0) {
     return errors[response.Status].name;
   }
 
-  var outputData = [];
-  var minCacheTTL = cachettl;
+  const outputData = [];
+  let cacheTTL = 0;
 
-  for (var i in response.Answer) {
+  for (const i in response.Answer) {
     outputData.push(response.Answer[i].data);
-    minCacheTTL = Math.max(minCacheTTL, response.Answer[i].TTL);
+    const ttl = response.Answer[i].TTL;
+    cacheTTL = Math.min(cacheTTL || ttl, ttl);
   }
 
-  var outputString = outputData.join(',');
+  const outputString = outputData.join(',');
 
   if (usecache) {
-    cache.put(cacheBinKey, outputString, minCacheTTL);
+    cache.put(cacheBinKey, outputString, Math.max(cacheTTL, minCacheTTL));
   }
 
   return outputString;

Applying changes from cloudflare#17298 (comment) + rename usecache to useCache to make all variables/constants follow lowerCamelCase.
@beltofte
Copy link
Contributor Author

Thanks @hunts. I have committed the patch, and at the same time renamed usecache to useCache, so all variables / constants are following lowerCamelCase.

Copy link
Contributor

@hunts hunts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick updates. Looks good to me now.

@RebeccaTamachiro RebeccaTamachiro merged commit 99b9cf4 into cloudflare:production Oct 24, 2024
7 checks passed
@workers-devprod workers-devprod added the contribution [Holopin] Recognizes a docs contribution, big or small label Oct 24, 2024
Copy link

holopin-bot bot commented Oct 24, 2024

Congratulations @beltofte, the maintainer of this repository has issued you a holobyte! Here it is: https://holopin.io/holobyte/cm2n4dctz16090cldevclq0gd

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

harshil1712 pushed a commit that referenced this pull request Dec 3, 2024
* Update dns-in-google-sheets.mdx to include cache support in the NSLookup function.

Adding internal caching of the results on the NSLookup function to limit the number of the DNS resolver requests and speed up the results - especially in larger Google Sheets.

* Process suggested changes for Answer TTL and usecache disabled by default

* Change how minCacheTTL is set

- Change minCacheTTL from Math.min to Math.max 
- Change default value of cachettl from 1800 to 30.

* Minor code style - change tab to spaces.

* Apply patch + rename usecache to useCache

Applying changes from #17298 (comment) + rename usecache to useCache to make all variables/constants follow lowerCamelCase.

* Remove outdated cache value from descriptive paragraph

* Replace one remaining var by const

---------

Co-authored-by: Rebecca Tamachiro <rtamachiro@cloudflare.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution [Holopin] Recognizes a docs contribution, big or small product:1.1.1.1 Related to 1.1.1.1 product size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants