From 2ea1ab21f69540c7e0d1f8f50c25c81c77938eef Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Thu, 29 Sep 2022 10:26:21 +0200 Subject: [PATCH] Minimize API surface, fix mocks typings --- .../index.ts | 2 +- .../src/agent_manager.ts | 6 ++++- .../index.ts | 1 + .../src/agent_manager.mocks.ts | 13 ++++++++++ .../src/elasticsearch_service.ts | 2 +- .../src/types.ts | 4 ++-- .../src/elasticsearch_service.mock.ts | 4 ++-- .../src/agent_manager.test.mocks.ts | 24 ------------------- .../src/elasticsearch_client.test.ts | 9 ++++--- .../src/elasticsearch_client.ts | 6 ++--- .../src/metrics_service.ts | 12 ++++------ .../src/ops_metrics_collector.ts | 10 +++----- 12 files changed, 39 insertions(+), 54 deletions(-) create mode 100644 packages/core/elasticsearch/core-elasticsearch-client-server-mocks/src/agent_manager.mocks.ts delete mode 100644 packages/core/metrics/core-metrics-collectors-server-internal/src/agent_manager.test.mocks.ts diff --git a/packages/core/elasticsearch/core-elasticsearch-client-server-internal/index.ts b/packages/core/elasticsearch/core-elasticsearch-client-server-internal/index.ts index aa1364c179e18..6f1f276c7d089 100644 --- a/packages/core/elasticsearch/core-elasticsearch-client-server-internal/index.ts +++ b/packages/core/elasticsearch/core-elasticsearch-client-server-internal/index.ts @@ -9,7 +9,7 @@ export { ScopedClusterClient } from './src/scoped_cluster_client'; export { ClusterClient } from './src/cluster_client'; export { configureClient } from './src/configure_client'; -export { AgentManager } from './src/agent_manager'; +export { type AgentStore, AgentManager } from './src/agent_manager'; export { getRequestDebugMeta, getErrorMessage } from './src/log_query_and_deprecation'; export { PRODUCT_RESPONSE_HEADER, diff --git a/packages/core/elasticsearch/core-elasticsearch-client-server-internal/src/agent_manager.ts b/packages/core/elasticsearch/core-elasticsearch-client-server-internal/src/agent_manager.ts index 27888ab08e814..9a57cc44e04ad 100644 --- a/packages/core/elasticsearch/core-elasticsearch-client-server-internal/src/agent_manager.ts +++ b/packages/core/elasticsearch/core-elasticsearch-client-server-internal/src/agent_manager.ts @@ -26,6 +26,10 @@ export interface AgentFactoryProvider { getAgentFactory(agentOptions?: HttpAgentOptions): AgentFactory; } +export interface AgentStore { + getAgents(): Set; +} + /** * Allows obtaining Agent factories, which can then be fed into elasticsearch-js's Client class. * Ideally, we should obtain one Agent factory for each ES Client class. @@ -37,7 +41,7 @@ export interface AgentFactoryProvider { * exposes methods that can modify the underlying pools, effectively impacting the connections of other Clients. * @internal **/ -export class AgentManager implements AgentFactoryProvider { +export class AgentManager implements AgentFactoryProvider, AgentStore { private agents: Set; constructor(private agentOptions: HttpAgentOptions = DEFAULT_CONFIG) { diff --git a/packages/core/elasticsearch/core-elasticsearch-client-server-mocks/index.ts b/packages/core/elasticsearch/core-elasticsearch-client-server-mocks/index.ts index c46381d57a7b6..0b66d449df013 100644 --- a/packages/core/elasticsearch/core-elasticsearch-client-server-mocks/index.ts +++ b/packages/core/elasticsearch/core-elasticsearch-client-server-mocks/index.ts @@ -15,3 +15,4 @@ export type { DeeplyMockedApi, ElasticsearchClientMock, } from './src/mocks'; +export { createAgentStoreMock } from './src/agent_manager.mocks'; diff --git a/packages/core/elasticsearch/core-elasticsearch-client-server-mocks/src/agent_manager.mocks.ts b/packages/core/elasticsearch/core-elasticsearch-client-server-mocks/src/agent_manager.mocks.ts new file mode 100644 index 0000000000000..9f81431b43733 --- /dev/null +++ b/packages/core/elasticsearch/core-elasticsearch-client-server-mocks/src/agent_manager.mocks.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { AgentStore, NetworkAgent } from '@kbn/core-elasticsearch-client-server-internal'; + +export const createAgentStoreMock = (agents: Set = new Set()): AgentStore => ({ + getAgents: jest.fn(() => agents), +}); diff --git a/packages/core/elasticsearch/core-elasticsearch-server-internal/src/elasticsearch_service.ts b/packages/core/elasticsearch/core-elasticsearch-server-internal/src/elasticsearch_service.ts index 0f76454297560..fddff84293140 100644 --- a/packages/core/elasticsearch/core-elasticsearch-server-internal/src/elasticsearch_service.ts +++ b/packages/core/elasticsearch/core-elasticsearch-server-internal/src/elasticsearch_service.ts @@ -120,7 +120,7 @@ export class ElasticsearchService } this.unauthorizedErrorHandler = handler; }, - agentManager: this.agentManager, + agentStore: this.agentManager, }; } diff --git a/packages/core/elasticsearch/core-elasticsearch-server-internal/src/types.ts b/packages/core/elasticsearch/core-elasticsearch-server-internal/src/types.ts index 8cebdbb554eec..b03b86c7bdd1c 100644 --- a/packages/core/elasticsearch/core-elasticsearch-server-internal/src/types.ts +++ b/packages/core/elasticsearch/core-elasticsearch-server-internal/src/types.ts @@ -12,7 +12,7 @@ import type { ElasticsearchServiceStart, ElasticsearchServiceSetup, } from '@kbn/core-elasticsearch-server'; -import type { AgentManager } from '@kbn/core-elasticsearch-client-server-internal'; +import type { AgentStore } from '@kbn/core-elasticsearch-client-server-internal'; import type { ServiceStatus } from '@kbn/core-status-common'; import type { NodesVersionCompatibility, NodeInfo } from './version_check/ensure_es_version'; import type { ClusterInfo } from './get_cluster_info'; @@ -22,7 +22,7 @@ export type InternalElasticsearchServicePreboot = ElasticsearchServicePreboot; /** @internal */ export interface InternalElasticsearchServiceSetup extends ElasticsearchServiceSetup { - agentManager: AgentManager; + agentStore: AgentStore; clusterInfo$: Observable; esNodesCompatibility$: Observable; status$: Observable>; diff --git a/packages/core/elasticsearch/core-elasticsearch-server-mocks/src/elasticsearch_service.mock.ts b/packages/core/elasticsearch/core-elasticsearch-server-mocks/src/elasticsearch_service.mock.ts index 7b03586a2ed41..26d81da24318c 100644 --- a/packages/core/elasticsearch/core-elasticsearch-server-mocks/src/elasticsearch_service.mock.ts +++ b/packages/core/elasticsearch/core-elasticsearch-server-mocks/src/elasticsearch_service.mock.ts @@ -13,6 +13,7 @@ import { elasticsearchClientMock, type ClusterClientMock, type CustomClusterClientMock, + createAgentStoreMock, } from '@kbn/core-elasticsearch-client-server-mocks'; import type { ElasticsearchClientConfig, @@ -28,7 +29,6 @@ import type { ClusterInfo, } from '@kbn/core-elasticsearch-server-internal'; import { type ServiceStatus, ServiceStatusLevels } from '@kbn/core-status-common'; -import { AgentManager } from '@kbn/core-elasticsearch-client-server-internal'; type MockedElasticSearchServicePreboot = jest.Mocked; @@ -95,7 +95,7 @@ const createInternalSetupContractMock = () => { level: ServiceStatusLevels.available, summary: 'Elasticsearch is available', }), - agentManager: new AgentManager(), + agentStore: createAgentStoreMock(), }; return internalSetupContract; }; diff --git a/packages/core/metrics/core-metrics-collectors-server-internal/src/agent_manager.test.mocks.ts b/packages/core/metrics/core-metrics-collectors-server-internal/src/agent_manager.test.mocks.ts deleted file mode 100644 index c32e92e99f01b..0000000000000 --- a/packages/core/metrics/core-metrics-collectors-server-internal/src/agent_manager.test.mocks.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { AgentManager } from '@kbn/core-elasticsearch-client-server-internal'; - -export const AgentManagerMock: typeof AgentManager = jest.fn(() => ({ - getAgentFactory: jest.fn(), - getAgents: jest.fn(), - agentOptions: {}, - agents: [], -})); - -jest.doMock('@kbn/core-elasticsearch-client-server-internal', () => { - const actual = jest.requireActual('@kbn/core-elasticsearch-client-server-internal'); - return { - ...actual, - AgentManager: AgentManagerMock, - }; -}); diff --git a/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.test.ts b/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.test.ts index adfaaaf4bb876..363fca6430dbe 100644 --- a/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.test.ts +++ b/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.test.ts @@ -9,7 +9,7 @@ import { Agent as HttpAgent } from 'http'; import { Agent as HttpsAgent } from 'https'; import { sampleEsClientMetrics } from '@kbn/core-metrics-server-mocks'; -import { AgentManagerMock } from './agent_manager.test.mocks'; +import { createAgentStoreMock } from '@kbn/core-elasticsearch-client-server-mocks'; import { getAgentsSocketsStatsMock } from './get_agents_sockets_stats.test.mocks'; import { ElasticsearchClientsMetricsCollector } from './elasticsearch_client'; import { getAgentsSocketsStats } from './get_agents_sockets_stats'; @@ -19,14 +19,13 @@ jest.mock('@kbn/core-elasticsearch-client-server-internal'); describe('ElasticsearchClientsMetricsCollector', () => { test('#collect calls getAgentsSocketsStats with the Agents managed by the provided AgentManager', async () => { const agents = new Set([new HttpAgent(), new HttpsAgent()]); - const agentManager = new AgentManagerMock(); - agentManager.getAgents.mockReturnValueOnce(agents); + const agentStore = createAgentStoreMock(agents); getAgentsSocketsStatsMock.mockReturnValueOnce(sampleEsClientMetrics); - const esClientsMetricsCollector = new ElasticsearchClientsMetricsCollector(agentManager); + const esClientsMetricsCollector = new ElasticsearchClientsMetricsCollector(agentStore); const metrics = await esClientsMetricsCollector.collect(); - expect(agentManager.getAgents).toHaveBeenCalledTimes(1); + expect(agentStore.getAgents).toHaveBeenCalledTimes(1); expect(getAgentsSocketsStats).toHaveBeenCalledTimes(1); expect(getAgentsSocketsStats).toHaveBeenNthCalledWith(1, agents); expect(metrics).toEqual(sampleEsClientMetrics); diff --git a/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.ts b/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.ts index 55208aecf041d..08fedb8a50fa0 100644 --- a/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.ts +++ b/packages/core/metrics/core-metrics-collectors-server-internal/src/elasticsearch_client.ts @@ -7,16 +7,16 @@ */ import type { ElasticsearchClientsMetrics, MetricsCollector } from '@kbn/core-metrics-server'; -import type { AgentManager } from '@kbn/core-elasticsearch-client-server-internal'; +import type { AgentStore } from '@kbn/core-elasticsearch-client-server-internal'; import { getAgentsSocketsStats } from './get_agents_sockets_stats'; export class ElasticsearchClientsMetricsCollector implements MetricsCollector { - constructor(private readonly agentManager: AgentManager) {} + constructor(private readonly agentStore: AgentStore) {} public async collect(): Promise { - return getAgentsSocketsStats(this.agentManager.getAgents()); + return getAgentsSocketsStats(this.agentStore.getAgents()); } public reset() { diff --git a/packages/core/metrics/core-metrics-server-internal/src/metrics_service.ts b/packages/core/metrics/core-metrics-server-internal/src/metrics_service.ts index d4cc0019ce400..95a9dc09bba57 100644 --- a/packages/core/metrics/core-metrics-server-internal/src/metrics_service.ts +++ b/packages/core/metrics/core-metrics-server-internal/src/metrics_service.ts @@ -55,14 +55,10 @@ export class MetricsService this.coreContext.configService.atPath(OPS_CONFIG_PATH) ); - this.metricsCollector = new OpsMetricsCollector( - http.server, - elasticsearchService.agentManager, - { - logger: this.logger, - ...config.cGroupOverrides, - } - ); + this.metricsCollector = new OpsMetricsCollector(http.server, elasticsearchService.agentStore, { + logger: this.logger, + ...config.cGroupOverrides, + }); await this.refreshMetrics(); diff --git a/packages/core/metrics/core-metrics-server-internal/src/ops_metrics_collector.ts b/packages/core/metrics/core-metrics-server-internal/src/ops_metrics_collector.ts index 4d3b3697272ec..8a10f4071b11b 100644 --- a/packages/core/metrics/core-metrics-server-internal/src/ops_metrics_collector.ts +++ b/packages/core/metrics/core-metrics-server-internal/src/ops_metrics_collector.ts @@ -8,7 +8,7 @@ import { Server as HapiServer } from '@hapi/hapi'; import type { OpsMetrics, MetricsCollector } from '@kbn/core-metrics-server'; -import type { AgentManager } from '@kbn/core-elasticsearch-client-server-internal'; +import type { AgentStore } from '@kbn/core-elasticsearch-client-server-internal'; import { ProcessMetricsCollector, OsMetricsCollector, @@ -23,15 +23,11 @@ export class OpsMetricsCollector implements MetricsCollector { private readonly serverCollector: ServerMetricsCollector; private readonly esClientCollector: ElasticsearchClientsMetricsCollector; - constructor( - server: HapiServer, - agentManager: AgentManager, - opsOptions: OpsMetricsCollectorOptions - ) { + constructor(server: HapiServer, agentStore: AgentStore, opsOptions: OpsMetricsCollectorOptions) { this.processCollector = new ProcessMetricsCollector(); this.osCollector = new OsMetricsCollector(opsOptions); this.serverCollector = new ServerMetricsCollector(server); - this.esClientCollector = new ElasticsearchClientsMetricsCollector(agentManager); + this.esClientCollector = new ElasticsearchClientsMetricsCollector(agentStore); } public async collect(): Promise {