From d2ca429c5a3634c173c2cd76bdef59a688a93066 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Fri, 30 Sep 2022 14:54:14 +0200 Subject: [PATCH] Cover edge cases for getAgentsSocketsStats() --- .../get_agents_sockets_stats.test.mocks.ts | 15 ++ .../src/get_agents_sockets_stats.test.ts | 156 ++++++++++++------ .../src/get_agents_sockets_stats.ts | 57 ++++--- 3 files changed, 149 insertions(+), 79 deletions(-) diff --git a/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.mocks.ts b/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.mocks.ts index 8f894865d9ac5..0a398166cbaaf 100644 --- a/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.mocks.ts +++ b/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.mocks.ts @@ -6,8 +6,23 @@ * Side Public License, v 1. */ +import { Agent as HttpAgent } from 'http'; +import { Agent as HttpsAgent } from 'https'; + import { getAgentsSocketsStats } from './get_agents_sockets_stats'; +export const getHttpAgentMock = (defaults: Partial) => { + jest.doMock('http'); + const agent = new HttpAgent(); + return Object.assign(agent, defaults); +}; + +export const getHttpsAgentMock = (defaults: Partial) => { + jest.doMock('https'); + const agent = new HttpsAgent(); + return Object.assign(agent, defaults); +}; + export const getAgentsSocketsStatsMock: jest.MockedFunction = jest.fn(); diff --git a/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.ts b/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.ts index 1bc2f40ed05f1..513bf2caa8545 100644 --- a/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.ts +++ b/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.test.ts @@ -8,68 +8,44 @@ import { Socket } from 'net'; import { Agent, IncomingMessage } from 'http'; -import { Agent as HttpsAgent } from 'https'; import { getAgentsSocketsStats } from './get_agents_sockets_stats'; +import { getHttpAgentMock, getHttpsAgentMock } from './get_agents_sockets_stats.test.mocks'; jest.mock('net'); -jest.mock('http'); -jest.mock('https'); -const configProperties = { - maxSockets: 10, - maxFreeSockets: 10, - maxTotalSockets: 100, - destroy: jest.fn(), -}; - -const HttpAgentMock = Agent as jest.MockedClass; +const mockSocket = new Socket(); +const mockIncomingMessage = new IncomingMessage(mockSocket); describe('getAgentsSocketsStats()', () => { it('extracts aggregated stats from the specified agents', () => { - const mockSocket = new Socket(); - const mockIncomingMessage = new IncomingMessage(mockSocket); - - HttpAgentMock.mockImplementationOnce(() => { - return { - ...configProperties, - sockets: { - node1: [mockSocket, mockSocket, mockSocket], - node2: [mockSocket], - }, - freeSockets: { - node1: [mockSocket], - node3: [mockSocket, mockSocket, mockSocket, mockSocket], - }, - requests: { - node1: [mockIncomingMessage, mockIncomingMessage], - }, - }; + const agent1 = getHttpAgentMock({ + sockets: { + node1: [mockSocket, mockSocket, mockSocket], + node2: [mockSocket], + }, + freeSockets: { + node1: [mockSocket], + node3: [mockSocket, mockSocket, mockSocket, mockSocket], + }, + requests: { + node1: [mockIncomingMessage, mockIncomingMessage], + }, }); - HttpAgentMock.mockImplementationOnce(() => { - return { - ...configProperties, - sockets: { - node1: [mockSocket, mockSocket, mockSocket], - node4: [mockSocket], - }, - freeSockets: { - node3: [mockSocket, mockSocket, mockSocket, mockSocket], - }, - requests: { - node4: [ - mockIncomingMessage, - mockIncomingMessage, - mockIncomingMessage, - mockIncomingMessage, - ], - }, - }; + const agent2 = getHttpAgentMock({ + sockets: { + node1: [mockSocket, mockSocket, mockSocket], + node4: [mockSocket], + }, + freeSockets: { + node3: [mockSocket, mockSocket, mockSocket, mockSocket], + }, + requests: { + node4: [mockIncomingMessage, mockIncomingMessage, mockIncomingMessage, mockIncomingMessage], + }, }); - const agents = new Set([new Agent(), new Agent()]); - - const stats = getAgentsSocketsStats(agents); + const stats = getAgentsSocketsStats(new Set([agent1, agent2])); expect(stats).toEqual({ averageActiveSocketsPerNode: 2.6666666666666665, averageIdleSocketsPerNode: 4.5, @@ -86,14 +62,86 @@ describe('getAgentsSocketsStats()', () => { }); it('takes into account Agent types to determine the `protocol`', () => { + const httpAgent = getHttpAgentMock({ + sockets: { node1: [mockSocket] }, + freeSockets: {}, + requests: {}, + }); + + const httpsAgent = getHttpsAgentMock({ + sockets: { node1: [mockSocket] }, + freeSockets: {}, + requests: {}, + }); + const noAgents = new Set(); - const httpAgents = new Set([new Agent(), new Agent()]); - const httpsAgents = new Set([new HttpsAgent(), new HttpsAgent()]); - const mixedAgents = new Set([new Agent(), new HttpsAgent()]); + const httpAgents = new Set([httpAgent, httpAgent]); + const httpsAgents = new Set([httpsAgent, httpsAgent]); + const mixedAgents = new Set([httpAgent, httpsAgent]); expect(getAgentsSocketsStats(noAgents).protocol).toEqual('none'); expect(getAgentsSocketsStats(httpAgents).protocol).toEqual('http'); expect(getAgentsSocketsStats(httpsAgents).protocol).toEqual('https'); expect(getAgentsSocketsStats(mixedAgents).protocol).toEqual('mixed'); }); + + it('does not take into account those Agents that have not had any connection to any node', () => { + const pristineAgentProps = { + sockets: {}, + freeSockets: {}, + requests: {}, + }; + const agent1 = getHttpAgentMock(pristineAgentProps); + const agent2 = getHttpAgentMock(pristineAgentProps); + const agent3 = getHttpAgentMock(pristineAgentProps); + + const stats = getAgentsSocketsStats(new Set([agent1, agent2, agent3])); + + expect(stats).toEqual({ + averageActiveSocketsPerNode: 0, + averageIdleSocketsPerNode: 0, + connectedNodes: 0, + mostActiveNodeSockets: 0, + mostIdleNodeSockets: 0, + nodesWithActiveSockets: 0, + nodesWithIdleSockets: 0, + protocol: 'none', + totalActiveSockets: 0, + totalIdleSockets: 0, + totalQueuedRequests: 0, + }); + }); + + it('takes into account those Agents that have hold mappings to one or more nodes, but that do not currently have any pending requests, active connections or idle connections', () => { + const emptyAgentProps = { + sockets: { + node1: [], + }, + freeSockets: { + node2: [], + }, + requests: { + node3: [], + }, + }; + + const agent1 = getHttpAgentMock(emptyAgentProps); + const agent2 = getHttpAgentMock(emptyAgentProps); + + const stats = getAgentsSocketsStats(new Set([agent1, agent2])); + + expect(stats).toEqual({ + averageActiveSocketsPerNode: 0, + averageIdleSocketsPerNode: 0, + connectedNodes: 3, + mostActiveNodeSockets: 0, + mostIdleNodeSockets: 0, + nodesWithActiveSockets: 0, + nodesWithIdleSockets: 0, + protocol: 'http', + totalActiveSockets: 0, + totalIdleSockets: 0, + totalQueuedRequests: 0, + }); + }); }); diff --git a/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.ts b/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.ts index a3dbff02bfe62..635e056490a50 100644 --- a/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.ts +++ b/packages/core/metrics/core-metrics-collectors-server-internal/src/get_agents_sockets_stats.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import type { Agent } from 'http'; +import { NetworkAgent } from '@kbn/core-elasticsearch-client-server-internal'; import { Agent as HttpsAgent } from 'https'; import { mean } from 'lodash'; import type { @@ -14,7 +14,7 @@ import type { ElasticsearchClientsMetrics, } from '@kbn/core-metrics-server'; -export const getAgentsSocketsStats = (agents: Set): ElasticsearchClientsMetrics => { +export const getAgentsSocketsStats = (agents: Set): ElasticsearchClientsMetrics => { const nodes = new Set(); let totalActiveSockets = 0; let totalIdleSockets = 0; @@ -26,26 +26,33 @@ export const getAgentsSocketsStats = (agents: Set): ElasticsearchClientsM const nodesWithIdleSockets: Record = {}; agents.forEach((agent) => { - if (agent instanceof HttpsAgent) https = true; - else http = true; + const agentRequests = Object.entries(agent.requests) ?? []; + const agentSockets = Object.entries(agent.sockets) ?? []; + const agentFreeSockets = Object.entries(agent.freeSockets) ?? []; - Object.values(agent.requests ?? []).forEach( - (queue) => (totalQueuedRequests += queue?.length ?? 0) - ); + if (agentRequests.length || agentSockets.length || agentFreeSockets.length) { + if (agent instanceof HttpsAgent) https = true; + else http = true; - Object.entries(agent.sockets ?? []).forEach(([node, sockets]) => { - nodes.add(node); - const activeSockets = sockets?.length ?? 0; - totalActiveSockets += activeSockets; - nodesWithActiveSockets[node] = (nodesWithActiveSockets[node] ?? 0) + activeSockets; - }); + agentRequests.forEach(([node, queue]) => { + nodes.add(node); + totalQueuedRequests += queue?.length ?? 0; + }); - Object.entries(agent.freeSockets ?? []).forEach(([node, freeSockets]) => { - nodes.add(node); - const idleSockets = freeSockets?.length ?? 0; - totalIdleSockets += idleSockets; - nodesWithIdleSockets[node] = (nodesWithIdleSockets[node] ?? 0) + idleSockets; - }); + agentSockets.forEach(([node, sockets]) => { + nodes.add(node); + const activeSockets = sockets?.length ?? 0; + totalActiveSockets += activeSockets; + nodesWithActiveSockets[node] = (nodesWithActiveSockets[node] ?? 0) + activeSockets; + }); + + agentFreeSockets.forEach(([node, freeSockets]) => { + nodes.add(node); + const idleSockets = freeSockets?.length ?? 0; + totalIdleSockets += idleSockets; + nodesWithIdleSockets[node] = (nodesWithIdleSockets[node] ?? 0) + idleSockets; + }); + } }); const activeSocketCounters = Object.values(nodesWithActiveSockets); @@ -60,14 +67,14 @@ export const getAgentsSocketsStats = (agents: Set): ElasticsearchClientsM return { protocol, connectedNodes: nodes.size, - nodesWithActiveSockets: activeSocketCounters.length, - nodesWithIdleSockets: idleSocketCounters.length, + nodesWithActiveSockets: activeSocketCounters.filter(Boolean).length, + nodesWithIdleSockets: idleSocketCounters.filter(Boolean).length, totalActiveSockets, totalIdleSockets, totalQueuedRequests, - mostActiveNodeSockets: Math.max(...activeSocketCounters), - averageActiveSocketsPerNode: mean(activeSocketCounters), - mostIdleNodeSockets: Math.max(...idleSocketCounters), - averageIdleSocketsPerNode: mean(idleSocketCounters), + mostActiveNodeSockets: activeSocketCounters.length ? Math.max(...activeSocketCounters) : 0, + averageActiveSocketsPerNode: activeSocketCounters.length ? mean(activeSocketCounters) : 0, + mostIdleNodeSockets: idleSocketCounters.length ? Math.max(...idleSocketCounters) : 0, + averageIdleSocketsPerNode: idleSocketCounters.length ? mean(idleSocketCounters) : 0, }; };