[Rule Registry] Additional error logging

x-pack/plugins/rule_registry/server/rule_data_client/rule_data_client.test.ts

@@ -6,6 +6,8 @@
  */
 
 import { left, right } from 'fp-ts/lib/Either';
+import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
+import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
 import { RuleDataClient, RuleDataClientConstructorOptions, WaitResult } from './rule_data_client';
 import { IndexInfo } from '../rule_data_plugin_service/index_info';
 import { Dataset, RuleDataWriterInitializationError } from '..';
@@ -15,7 +17,7 @@ import { IndexPatternsFetcher } from '@kbn/data-plugin/server';
 import { createNoMatchingIndicesError } from '@kbn/data-views-plugin/server/fetcher/lib/errors';
 import { ElasticsearchClient } from '@kbn/core/server';
 
-const mockLogger = loggingSystemMock.create().get();
+const logger: ReturnType<typeof loggingSystemMock.createLogger> = loggingSystemMock.createLogger();
 const scopedClusterClient = elasticsearchServiceMock.createScopedClusterClient().asInternalUser;
 const mockResourceInstaller = resourceInstallerMock.create();
 
@@ -52,7 +54,7 @@ function getRuleDataClientOptions({
       waitUntilReadyForReading ?? Promise.resolve(right(scopedClusterClient) as WaitResult),
     waitUntilReadyForWriting:
       waitUntilReadyForWriting ?? Promise.resolve(right(scopedClusterClient) as WaitResult),
-    logger: mockLogger,
+    logger,
   };
 }
 
@@ -71,15 +73,12 @@ describe('RuleDataClient', () => {
   });
 
   describe('getReader()', () => {
-    beforeAll(() => {
+    beforeEach(() => {
+      jest.resetAllMocks();
       getFieldsForWildcardMock.mockResolvedValue(['foo']);
       IndexPatternsFetcher.prototype.getFieldsForWildcard = getFieldsForWildcardMock;
     });
 
-    beforeEach(() => {
-      getFieldsForWildcardMock.mockClear();
-    });
-
     afterAll(() => {
       getFieldsForWildcardMock.mockRestore();
     });
@@ -116,6 +115,10 @@ describe('RuleDataClient', () => {
           body: query,
         })
       ).rejects.toThrowErrorMatchingInlineSnapshot(`"something went wrong!"`);
+
+      expect(logger.error).toHaveBeenCalledWith(
+        `Error performing search in RuleDataClient - something went wrong!`
+      );
     });
 
     test('waits until cluster client is ready before getDynamicIndexPattern', async () => {
@@ -143,6 +146,10 @@ describe('RuleDataClient', () => {
       await expect(reader.getDynamicIndexPattern()).rejects.toThrowErrorMatchingInlineSnapshot(
         `"something went wrong!"`
       );
+
+      expect(logger.error).toHaveBeenCalledWith(
+        `Error fetching index patterns in RuleDataClient - something went wrong!`
+      );
     });
 
     test('correct handles no_matching_indices errors from getFieldsForWildcard', async () => {
@@ -155,6 +162,8 @@ describe('RuleDataClient', () => {
         timeFieldName: '@timestamp',
         title: '.alerts-observability.apm.alerts*',
       });
+
+      expect(logger.error).not.toHaveBeenCalled();
     });
 
     test('handles errors getting cluster client', async () => {
@@ -193,7 +202,7 @@ describe('RuleDataClient', () => {
       await expect(() => ruleDataClient.getWriter()).rejects.toThrowErrorMatchingInlineSnapshot(
         `"Rule registry writing is disabled. Make sure that \\"xpack.ruleRegistry.write.enabled\\" configuration is not set to false and \\"observability.apm\\" is not disabled in \\"xpack.ruleRegistry.write.disabledRegistrationContexts\\" within \\"kibana.yml\\"."`
       );
-      expect(mockLogger.debug).toHaveBeenCalledWith(
+      expect(logger.debug).toHaveBeenCalledWith(
         `Writing is disabled, bulk() will not write any data.`
       );
     });
@@ -210,15 +219,15 @@ describe('RuleDataClient', () => {
       await expect(() => ruleDataClient.getWriter()).rejects.toThrowErrorMatchingInlineSnapshot(
         `"There has been a catastrophic error trying to install index level resources for the following registration context: observability.apm. This may have been due to a non-additive change to the mappings, removal and type changes are not permitted. Full error: Error: could not get cluster client"`
       );
-      expect(mockLogger.error).toHaveBeenNthCalledWith(
+      expect(logger.error).toHaveBeenNthCalledWith(
         1,
         new RuleDataWriterInitializationError(
           'index',
           'observability.apm',
           new Error('could not get cluster client')
         )
       );
-      expect(mockLogger.error).toHaveBeenNthCalledWith(
+      expect(logger.error).toHaveBeenNthCalledWith(
         2,
         `The writer for the Rule Data Client for the observability.apm registration context was not initialized properly, bulk() cannot continue, and writing will be disabled.`
       );
@@ -228,7 +237,7 @@ describe('RuleDataClient', () => {
       await expect(() => ruleDataClient.getWriter()).rejects.toThrowErrorMatchingInlineSnapshot(
         `"Rule registry writing is disabled due to an error during Rule Data Client initialization."`
       );
-      expect(mockLogger.debug).toHaveBeenCalledWith(
+      expect(logger.debug).toHaveBeenCalledWith(
         `Writing is disabled, bulk() will not write any data.`
       );
     });
@@ -242,15 +251,15 @@ describe('RuleDataClient', () => {
       await expect(() => ruleDataClient.getWriter()).rejects.toThrowErrorMatchingInlineSnapshot(
         `"There has been a catastrophic error trying to install namespace level resources for the following registration context: observability.apm. This may have been due to a non-additive change to the mappings, removal and type changes are not permitted. Full error: Error: bad resource installation"`
       );
-      expect(mockLogger.error).toHaveBeenNthCalledWith(
+      expect(logger.error).toHaveBeenNthCalledWith(
         1,
         new RuleDataWriterInitializationError(
           'namespace',
           'observability.apm',
           new Error('bad resource installation')
         )
       );
-      expect(mockLogger.error).toHaveBeenNthCalledWith(
+      expect(logger.error).toHaveBeenNthCalledWith(
         2,
         `The writer for the Rule Data Client for the observability.apm registration context was not initialized properly, bulk() cannot continue, and writing will be disabled.`
       );
@@ -260,7 +269,7 @@ describe('RuleDataClient', () => {
       await expect(() => ruleDataClient.getWriter()).rejects.toThrowErrorMatchingInlineSnapshot(
         `"Rule registry writing is disabled due to an error during Rule Data Client initialization."`
       );
-      expect(mockLogger.debug).toHaveBeenCalledWith(
+      expect(logger.debug).toHaveBeenCalledWith(
         `Writing is disabled, bulk() will not write any data.`
       );
     });
@@ -297,7 +306,7 @@ describe('RuleDataClient', () => {
       await delay();
 
       expect(await writer.bulk({})).toEqual(undefined);
-      expect(mockLogger.debug).toHaveBeenCalledWith(
+      expect(logger.debug).toHaveBeenCalledWith(
         `Writing is disabled, bulk() will not write any data.`
       );
     });
@@ -317,11 +326,16 @@ describe('RuleDataClient', () => {
       await expect(() => writer.bulk({})).rejects.toThrowErrorMatchingInlineSnapshot(
         `"something went wrong!"`
       );
-      expect(mockLogger.error).toHaveBeenNthCalledWith(1, error);
+      expect(logger.error).toHaveBeenNthCalledWith(1, error);
       expect(ruleDataClient.isWriteEnabled()).toBe(true);
     });
 
     test('waits until cluster client is ready before calling bulk', async () => {
+      scopedClusterClient.bulk.mockResolvedValueOnce(
+        elasticsearchClientMock.createSuccessTransportRequestPromise(
+          {}
+        ) as unknown as estypes.BulkResponse
+      );
       const ruleDataClient = new RuleDataClient(
         getRuleDataClientOptions({
           waitUntilReadyForWriting: new Promise((resolve) =>

x-pack/plugins/rule_registry/server/rule_data_client/rule_data_client.ts

@@ -13,13 +13,16 @@ import { ElasticsearchClient } from '@kbn/core/server';
 import { Logger } from '@kbn/core/server';
 import { IndexPatternsFetcher } from '@kbn/data-plugin/server';
 
+import { ESSearchRequest, ESSearchResponse } from '@kbn/core/types/elasticsearch';
 import {
   RuleDataWriteDisabledError,
   RuleDataWriterInitializationError,
 } from '../rule_data_plugin_service/errors';
 import { IndexInfo } from '../rule_data_plugin_service/index_info';
 import { IResourceInstaller } from '../rule_data_plugin_service/resource_installer';
 import { IRuleDataClient, IRuleDataReader, IRuleDataWriter } from './types';
+import { ParsedTechnicalFields } from '../../common/parse_technical_fields';
+import { ParsedExperimentalFields } from '../../common/parse_experimental_fields';
 
 export interface RuleDataClientConstructorOptions {
   indexInfo: IndexInfo;
@@ -103,15 +106,24 @@ export class RuleDataClient implements IRuleDataClient {
     };
 
     return {
-      search: async (request) => {
-        const clusterClient = await waitUntilReady();
-
-        const body = await clusterClient.search({
-          ...request,
-          index: indexPattern,
-        });
-
-        return body as any;
+      search: async <TSearchRequest extends ESSearchRequest>(
+        request: TSearchRequest
+      ): Promise<
+        ESSearchResponse<Partial<ParsedTechnicalFields & ParsedExperimentalFields>, TSearchRequest>
+      > => {
+        try {
+          const clusterClient = await waitUntilReady();
+          return (await clusterClient.search({
+            ...request,
+            index: indexPattern,
+          })) as unknown as ESSearchResponse<
+            Partial<ParsedTechnicalFields & ParsedExperimentalFields>,
+            TSearchRequest
+          >;
+        } catch (err) {
+          this.options.logger.error(`Error performing search in RuleDataClient - ${err.message}`);
+          throw err;
+        }
       },
 
       getDynamicIndexPattern: async () => {
@@ -136,6 +148,9 @@ export class RuleDataClient implements IRuleDataClient {
               title: indexPattern,
             };
           }
+          this.options.logger.error(
+            `Error fetching index patterns in RuleDataClient - ${err.message}`
+          );
           throw err;
         }
       },

x-pack/plugins/rule_registry/server/rule_data_plugin_service/resource_installer.ts

@@ -95,25 +95,32 @@ export class ResourceInstaller {
    */
   public async installCommonResources(): Promise<void> {
     await this.installWithTimeout('common resources shared between all indices', async () => {
-      const { getResourceName } = this.options;
-
-      // We can install them in parallel
-      await Promise.all([
-        this.createOrUpdateLifecyclePolicy({
-          name: getResourceName(DEFAULT_ILM_POLICY_ID),
-          body: defaultLifecyclePolicy,
-        }),
-
-        this.createOrUpdateComponentTemplate({
-          name: getResourceName(TECHNICAL_COMPONENT_TEMPLATE_NAME),
-          body: technicalComponentTemplate,
-        }),
-
-        this.createOrUpdateComponentTemplate({
-          name: getResourceName(ECS_COMPONENT_TEMPLATE_NAME),
-          body: ecsComponentTemplate,
-        }),
-      ]);
+      const { getResourceName, logger } = this.options;
+
+      try {
+        // We can install them in parallel
+        await Promise.all([
+          this.createOrUpdateLifecyclePolicy({
+            name: getResourceName(DEFAULT_ILM_POLICY_ID),
+            body: defaultLifecyclePolicy,
+          }),
+
+          this.createOrUpdateComponentTemplate({
+            name: getResourceName(TECHNICAL_COMPONENT_TEMPLATE_NAME),
+            body: technicalComponentTemplate,
+          }),
+
+          this.createOrUpdateComponentTemplate({
+            name: getResourceName(ECS_COMPONENT_TEMPLATE_NAME),
+            body: ecsComponentTemplate,
+          }),
+        ]);
+      } catch (err) {
+        logger.error(
+          `Error installing common resources in RuleRegistry ResourceInstaller - ${err.message}`
+        );
+        throw err;
+      }
     });
   }
 
@@ -375,6 +382,9 @@ export class ResourceInstaller {
         },
       });
     } catch (err) {
+      logger.error(
+        `Error creating index ${initialIndexName} as the write index for alias ${primaryNamespacedAlias} in RuleRegistry ResourceInstaller: ${err.message}`
+      );
       // If the index already exists and it's the write index for the alias,
       // something else created it so suppress the error. If it's not the write
       // index, that's bad, throw an error.
@@ -460,6 +470,9 @@ export class ResourceInstaller {
         return createConcreteIndexInfo({});
       }
 
+      logger.error(
+        `Error fetching concrete indices for ${indexPatternForBackingIndices} in RuleRegistry ResourceInstaller - ${err.message}`
+      );
       // A non-404 error is a real problem so we re-throw.
       throw err;
     }