Fix search telemetry to only update SO periodically (#93130) (#94454)

* Fix search telemetry to only update SO periodically * Handle case when searches completed mid flight * Fix error in resetting counters * Update docs * update docs * Don't track restored searches * Update docs * Update docs Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Anton Dosov <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Anton Dosov <[email protected]>
elastic · Mar 11, 2021 · aa9e554 · aa9e554
1 parent bed89aa
commit aa9e554
Show file tree

Hide file tree

Showing 11 changed files with 130 additions and 63 deletions.
diff --git a/api_docs/case.mdx b/api_docs/case.mdx
diff --git a/api_docs/cases.json b/api_docs/cases.json
@@ -1,5 +1,5 @@
 {
-  "id": "case",
+  "id": "cases",
   "client": {
     "classes": [],
     "functions": [],
@@ -34,7 +34,7 @@
               {
                 "pluginId": "cases",
                 "scope": "server",
-                "docId": "kibCasePluginApi",
+                "docId": "kibCasesPluginApi",
                 "section": "def-server.CasesClient",
                 "text": "CasesClient"
               }
@@ -60,4 +60,4 @@
     "misc": [],
     "objects": []
   }
-}
+}
diff --git a/api_docs/cases.mdx b/api_docs/cases.mdx
@@ -0,0 +1,18 @@
+---
+id: kibCasesPluginApi
+slug: /kibana-dev-docs/casesPluginApi
+title: cases
+image: https://source.unsplash.com/400x175/?github
+summary: API docs for the cases plugin
+date: 2020-11-16
+tags: ['contributor', 'dev', 'apidocs', 'kibana', 'cases']
+warning: This document is auto-generated and is meant to be viewed inside our experimental, new docs system. Reach out in #docs-engineering for more info.
+---
+
+import casesObj from './cases.json';
+
+## Server
+
+### Interfaces
+<DocDefinitionList data={casesObj.server.interfaces}/>
+
diff --git a/api_docs/data_search.json b/api_docs/data_search.json
@@ -1435,7 +1435,15 @@
             "section": "def-server.SearchUsage",
             "text": "SearchUsage"
           },
-          " | undefined) => { next(response: ",
+          " | undefined, { isRestore }: ",
+          {
+            "pluginId": "data",
+            "scope": "common",
+            "docId": "kibDataSearchPluginApi",
+            "section": "def-common.ISearchOptions",
+            "text": "ISearchOptions"
+          },
+          ") => { next(response: ",
           {
             "pluginId": "data",
             "scope": "common",
@@ -1459,7 +1467,7 @@
             "description": [],
             "source": {
               "path": "src/plugins/data/server/search/collectors/usage.ts",
-              "lineNumber": 64
+              "lineNumber": 83
             }
           },
           {
@@ -1479,15 +1487,34 @@
             "description": [],
             "source": {
               "path": "src/plugins/data/server/search/collectors/usage.ts",
-              "lineNumber": 64
+              "lineNumber": 84
+            }
+          },
+          {
+            "type": "Object",
+            "label": "{ isRestore }",
+            "isRequired": true,
+            "signature": [
+              {
+                "pluginId": "data",
+                "scope": "common",
+                "docId": "kibDataSearchPluginApi",
+                "section": "def-common.ISearchOptions",
+                "text": "ISearchOptions"
+              }
+            ],
+            "description": [],
+            "source": {
+              "path": "src/plugins/data/server/search/collectors/usage.ts",
+              "lineNumber": 85
             }
           }
         ],
         "tags": [],
         "returnComment": [],
         "source": {
           "path": "src/plugins/data/server/search/collectors/usage.ts",
-          "lineNumber": 64
+          "lineNumber": 82
         },
         "initialIsOpen": false
       },

diff --git a/api_docs/lists.json b/api_docs/lists.json
@@ -4489,6 +4489,22 @@
         ],
         "initialIsOpen": false
       },
+      {
+        "tags": [],
+        "id": "def-common.osType",
+        "type": "Object",
+        "label": "osType",
+        "description": [],
+        "source": {
+          "path": "x-pack/plugins/lists/common/schemas/common/schemas.ts",
+          "lineNumber": 317
+        },
+        "signature": [
+          "KeyofC",
+          "<{ linux: null; macos: null; windows: null; }>"
+        ],
+        "initialIsOpen": false
+      },
       {
         "tags": [],
         "id": "def-common.osTypeArray",

diff --git a/api_docs/security_solution.json b/api_docs/security_solution.json
@@ -607,7 +607,7 @@
                 "description": [],
                 "source": {
                   "path": "x-pack/plugins/security_solution/server/plugin.ts",
-                  "lineNumber": 336
+                  "lineNumber": 337
                 }
               },
               {
@@ -626,15 +626,15 @@
                 "description": [],
                 "source": {
                   "path": "x-pack/plugins/security_solution/server/plugin.ts",
-                  "lineNumber": 336
+                  "lineNumber": 337
                 }
               }
             ],
             "tags": [],
             "returnComment": [],
             "source": {
               "path": "x-pack/plugins/security_solution/server/plugin.ts",
-              "lineNumber": 336
+              "lineNumber": 337
             }
           },
           {
@@ -650,7 +650,7 @@
             "returnComment": [],
             "source": {
               "path": "x-pack/plugins/security_solution/server/plugin.ts",
-              "lineNumber": 397
+              "lineNumber": 398
             }
           }
         ],

diff --git a/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md b/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md
@@ -35,7 +35,7 @@
 |  [getTime(indexPattern, timeRange, options)](./kibana-plugin-plugins-data-server.gettime.md) |  |
 |  [parseInterval(interval)](./kibana-plugin-plugins-data-server.parseinterval.md) |  |
 |  [plugin(initializerContext)](./kibana-plugin-plugins-data-server.plugin.md) | Static code to be shared externally |
-|  [searchUsageObserver(logger, usage)](./kibana-plugin-plugins-data-server.searchusageobserver.md) | Rxjs observer for easily doing <code>tap(searchUsageObserver(logger, usage))</code> in an rxjs chain. |
+|  [searchUsageObserver(logger, usage, { isRestore })](./kibana-plugin-plugins-data-server.searchusageobserver.md) | Rxjs observer for easily doing <code>tap(searchUsageObserver(logger, usage))</code> in an rxjs chain. |
 |  [shouldReadFieldFromDocValues(aggregatable, esType)](./kibana-plugin-plugins-data-server.shouldreadfieldfromdocvalues.md) |  |
 |  [usageProvider(core)](./kibana-plugin-plugins-data-server.usageprovider.md) |  |
 

diff --git a/...nt/plugins/data/server/kibana-plugin-plugins-data-server.searchusageobserver.md b/...nt/plugins/data/server/kibana-plugin-plugins-data-server.searchusageobserver.md
@@ -9,7 +9,7 @@ Rxjs observer for easily doing `tap(searchUsageObserver(logger, usage))` in an r
 <b>Signature:</b>
 
 ```typescript
-export declare function searchUsageObserver(logger: Logger, usage?: SearchUsage): {
+export declare function searchUsageObserver(logger: Logger, usage?: SearchUsage, { isRestore }?: ISearchOptions): {
     next(response: IEsSearchResponse): void;
     error(): void;
 };
@@ -21,6 +21,7 @@ export declare function searchUsageObserver(logger: Logger, usage?: SearchUsage)
 |  --- | --- | --- |
 |  logger | <code>Logger</code> |  |
 |  usage | <code>SearchUsage</code> |  |
+|  { isRestore } | <code>ISearchOptions</code> |  |
 
 <b>Returns:</b>
 

diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts
@@ -6,13 +6,13 @@
  * Side Public License, v 1.
  */
 
-import { once } from 'lodash';
+import { once, debounce } from 'lodash';
 import type { CoreSetup, Logger } from 'kibana/server';
-import { SavedObjectsErrorHelpers } from '../../../../../core/server';
-import type { IEsSearchResponse } from '../../../common';
+import type { IEsSearchResponse, ISearchOptions } from '../../../common';
+import { isCompleteResponse } from '../../../common';
+import { CollectedUsage } from './register';
 
 const SAVED_OBJECT_ID = 'search-telemetry';
-const MAX_RETRY_COUNT = 3;
 
 export interface SearchUsage {
   trackError(): Promise<void>;
@@ -25,34 +25,52 @@ export function usageProvider(core: CoreSetup): SearchUsage {
     return coreStart.savedObjects.createInternalRepository();
   });
 
-  const trackSuccess = async (duration: number, retryCount = 0) => {
-    const repository = await getRepository();
-    try {
-      await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [
-        { fieldName: 'successCount' },
-        {
-          fieldName: 'totalDuration',
-          incrementBy: duration,
-        },
-      ]);
-    } catch (e) {
-      if (SavedObjectsErrorHelpers.isConflictError(e) && retryCount < MAX_RETRY_COUNT) {
-        setTimeout(() => trackSuccess(duration, retryCount + 1), 1000);
-      }
-    }
+  const collectedUsage: CollectedUsage = {
+    successCount: 0,
+    errorCount: 0,
+    totalDuration: 0,
   };
 
-  const trackError = async (retryCount = 0) => {
-    const repository = await getRepository();
-    try {
-      await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [
-        { fieldName: 'errorCount' },
-      ]);
-    } catch (e) {
-      if (SavedObjectsErrorHelpers.isConflictError(e) && retryCount < MAX_RETRY_COUNT) {
-        setTimeout(() => trackError(retryCount + 1), 1000);
+  // Instead of updating the search count every time a search completes, we update some in-memory
+  // counts and only update the saved object every ~5 seconds
+  const updateSearchUsage = debounce(
+    async () => {
+      const repository = await getRepository();
+      const { successCount, errorCount, totalDuration } = collectedUsage;
+      const counterFields = Object.entries(collectedUsage)
+        .map(([fieldName, incrementBy]) => ({ fieldName, incrementBy }))
+        // Filter out any zero values because `incrementCounter` will still increment them
+        .filter(({ incrementBy }) => incrementBy > 0);
+
+      try {
+        await repository.incrementCounter<CollectedUsage>(
+          SAVED_OBJECT_ID,
+          SAVED_OBJECT_ID,
+          counterFields
+        );
+
+        // Since search requests may have completed while the saved object was being updated, we minus
+        // what was just updated in the saved object rather than resetting the values to 0
+        collectedUsage.successCount -= successCount;
+        collectedUsage.errorCount -= errorCount;
+        collectedUsage.totalDuration -= totalDuration;
+      } catch (e) {
+        // We didn't reset the counters so we'll retry when the next search request completes
       }
-    }
+    },
+    5000,
+    { maxWait: 5000 }
+  );
+
+  const trackSuccess = (duration: number) => {
+    collectedUsage.successCount++;
+    collectedUsage.totalDuration += duration;
+    return updateSearchUsage();
+  };
+
+  const trackError = () => {
+    collectedUsage.errorCount++;
+    return updateSearchUsage();
   };
 
   return { trackSuccess, trackError };
@@ -61,9 +79,14 @@ export function usageProvider(core: CoreSetup): SearchUsage {
 /**
  * Rxjs observer for easily doing `tap(searchUsageObserver(logger, usage))` in an rxjs chain.
  */
-export function searchUsageObserver(logger: Logger, usage?: SearchUsage) {
+export function searchUsageObserver(
+  logger: Logger,
+  usage?: SearchUsage,
+  { isRestore }: ISearchOptions = {}
+) {
   return {
     next(response: IEsSearchResponse) {
+      if (isRestore || !isCompleteResponse(response)) return;
       logger.debug(`trackSearchStatus:next  ${response.rawResponse.took}`);
       usage?.trackSuccess(response.rawResponse.took);
     },

diff --git a/src/plugins/data/server/search/es_search/es_search_strategy.ts b/src/plugins/data/server/search/es_search/es_search_strategy.ts
@@ -53,6 +53,6 @@ export const esSearchStrategyProvider = (
       }
     };
 
-    return from(search()).pipe(tap(searchUsageObserver(logger, usage)));
+    return from(search()).pipe(tap(searchUsageObserver(logger, usage, options)));
   },
 });
diff --git a/src/plugins/data/server/server.api.md b/src/plugins/data/server/server.api.md
@@ -1356,7 +1356,7 @@ export interface SearchUsage {
 // Warning: (ae-missing-release-tag) "searchUsageObserver" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public
-export function searchUsageObserver(logger: Logger_2, usage?: SearchUsage): {
+export function searchUsageObserver(logger: Logger_2, usage?: SearchUsage, { isRestore }?: ISearchOptions): {
     next(response: IEsSearchResponse): void;
     error(): void;
 };
-Original file line number
+Diff line change
@@ Expand Up / @@ -53,6 +53,6 @@ export const esSearchStrategyProvider = ( @@
           }
         };
-        return from(search()).pipe(tap(searchUsageObserver(logger, usage)));
+        return from(search()).pipe(tap(searchUsageObserver(logger, usage, options)));
       },
     });