Skip to content

Commit

Permalink
Minimize API surface, fix mocks typings
Browse files Browse the repository at this point in the history
  • Loading branch information
gsoldevila committed Sep 29, 2022
1 parent 1b8dafc commit 2ea1ab2
Show file tree
Hide file tree
Showing 12 changed files with 39 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export interface AgentFactoryProvider {
getAgentFactory(agentOptions?: HttpAgentOptions): AgentFactory;
}

export interface AgentStore {
getAgents(): Set<NetworkAgent>;
}

/**
* 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.
Expand All @@ -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<HttpAgent>;

constructor(private agentOptions: HttpAgentOptions = DEFAULT_CONFIG) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ export type {
DeeplyMockedApi,
ElasticsearchClientMock,
} from './src/mocks';
export { createAgentStoreMock } from './src/agent_manager.mocks';
Original file line number Diff line number Diff line change
@@ -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<NetworkAgent> = new Set()): AgentStore => ({
getAgents: jest.fn(() => agents),
});
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class ElasticsearchService
}
this.unauthorizedErrorHandler = handler;
},
agentManager: this.agentManager,
agentStore: this.agentManager,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -22,7 +22,7 @@ export type InternalElasticsearchServicePreboot = ElasticsearchServicePreboot;

/** @internal */
export interface InternalElasticsearchServiceSetup extends ElasticsearchServiceSetup {
agentManager: AgentManager;
agentStore: AgentStore;
clusterInfo$: Observable<ClusterInfo>;
esNodesCompatibility$: Observable<NodesVersionCompatibility>;
status$: Observable<ServiceStatus<ElasticsearchStatusMeta>>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
elasticsearchClientMock,
type ClusterClientMock,
type CustomClusterClientMock,
createAgentStoreMock,
} from '@kbn/core-elasticsearch-client-server-mocks';
import type {
ElasticsearchClientConfig,
Expand All @@ -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<ElasticsearchServicePreboot>;

Expand Down Expand Up @@ -95,7 +95,7 @@ const createInternalSetupContractMock = () => {
level: ServiceStatusLevels.available,
summary: 'Elasticsearch is available',
}),
agentManager: new AgentManager(),
agentStore: createAgentStoreMock(),
};
return internalSetupContract;
};
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<HttpAgent>([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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ElasticsearchClientsMetrics>
{
constructor(private readonly agentManager: AgentManager) {}
constructor(private readonly agentStore: AgentStore) {}

public async collect(): Promise<ElasticsearchClientsMetrics> {
return getAgentsSocketsStats(this.agentManager.getAgents());
return getAgentsSocketsStats(this.agentStore.getAgents());
}

public reset() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,10 @@ export class MetricsService
this.coreContext.configService.atPath<OpsConfigType>(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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -23,15 +23,11 @@ export class OpsMetricsCollector implements MetricsCollector<OpsMetrics> {
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<OpsMetrics> {
Expand Down

0 comments on commit 2ea1ab2

Please sign in to comment.