From 839dd96c7dfe48a02c3dc5f1f42dcb3ca1aa6918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 31 Jan 2024 13:29:17 +0100 Subject: [PATCH] refactor(core): Move all code related to `onServerStarted` into `InternalHooks` (no-changelog) (#8500) --- packages/cli/src/Interfaces.ts | 32 ------ packages/cli/src/InternalHooks.ts | 87 ++++++++++---- packages/cli/src/Server.ts | 72 +----------- packages/cli/test/unit/InternalHooks.test.ts | 115 +++++++++++-------- 4 files changed, 131 insertions(+), 175 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index c067ba9baa..abcfa0bccb 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -31,7 +31,6 @@ import type { WorkflowExecute } from 'n8n-core'; import type PCancelable from 'p-cancelable'; -import type { DatabaseType } from '@db/types'; import type { AuthProviderType } from '@db/entities/AuthIdentity'; import type { SharedCredentials } from '@db/entities/SharedCredentials'; import type { TagEntity } from '@db/entities/TagEntity'; @@ -267,37 +266,6 @@ export interface IWebhookManager { executeWebhook(req: WebhookRequest, res: Response): Promise; } -export interface IDiagnosticInfo { - versionCli: string; - databaseType: DatabaseType; - notificationsEnabled: boolean; - disableProductionWebhooksOnMainProcess: boolean; - systemInfo: { - os: { - type?: string; - version?: string; - }; - memory?: number; - cpus: { - count?: number; - model?: string; - speed?: number; - }; - }; - executionVariables: { - [key: string]: string | number | boolean | undefined; - }; - deploymentType: string; - binaryDataMode: string; - smtp_set_up: boolean; - ldap_allowed: boolean; - saml_enabled: boolean; - binary_data_s3: boolean; - multi_main_setup_enabled: boolean; - licensePlanName?: string; - licenseTenantId?: number; -} - export interface ITelemetryUserDeletionData { user_id: string; target_user_old_status: 'active' | 'invited'; diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 1db38561b2..ca3e6daccd 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -1,5 +1,6 @@ import { Service } from 'typedi'; import { snakeCase } from 'change-case'; +import os from 'node:os'; import { get as pslGet } from 'psl'; import type { AuthenticationMethod, @@ -13,20 +14,22 @@ import type { import { TelemetryHelpers } from 'n8n-workflow'; import { InstanceSettings } from 'n8n-core'; +import config from '@/config'; import { N8N_VERSION } from '@/constants'; import type { AuthProviderType } from '@db/entities/AuthIdentity'; import type { GlobalRole, User } from '@db/entities/User'; import type { ExecutionMetadata } from '@db/entities/ExecutionMetadata'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { MessageEventBus, type EventPayloadWorkflow } from '@/eventbus'; import { determineFinalExecutionStatus } from '@/executionLifecycleHooks/shared/sharedHookFunctions'; import type { - IDiagnosticInfo, ITelemetryUserDeletionData, IWorkflowDb, IExecutionTrackProperties, IWorkflowExecutionDataProcess, } from '@/Interfaces'; +import { License } from '@/License'; import { EventsService } from '@/services/events.service'; import { NodeTypes } from '@/NodeTypes'; import { Telemetry } from '@/telemetry'; @@ -50,12 +53,14 @@ function userToPayload(user: User): { @Service() export class InternalHooks { constructor( - private telemetry: Telemetry, - private nodeTypes: NodeTypes, - private sharedWorkflowRepository: SharedWorkflowRepository, + private readonly telemetry: Telemetry, + private readonly nodeTypes: NodeTypes, + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + private readonly workflowRepository: WorkflowRepository, eventsService: EventsService, private readonly instanceSettings: InstanceSettings, private readonly eventBus: MessageEventBus, + private readonly license: License, ) { eventsService.on( 'telemetry.onFirstProductionWorkflowSuccess', @@ -71,31 +76,69 @@ export class InternalHooks { await this.telemetry.init(); } - async onServerStarted( - diagnosticInfo: IDiagnosticInfo, - earliestWorkflowCreatedAt?: Date, - ): Promise { + async onServerStarted(): Promise { + const cpus = os.cpus(); + const binaryDataConfig = config.getEnv('binaryDataManager'); + + const isS3Selected = config.getEnv('binaryDataManager.mode') === 's3'; + const isS3Available = config.getEnv('binaryDataManager.availableModes').includes('s3'); + const isS3Licensed = this.license.isBinaryDataS3Licensed(); + const authenticationMethod = config.getEnv('userManagement.authenticationMethod'); + const info = { - version_cli: diagnosticInfo.versionCli, - db_type: diagnosticInfo.databaseType, - n8n_version_notifications_enabled: diagnosticInfo.notificationsEnabled, - n8n_disable_production_main_process: diagnosticInfo.disableProductionWebhooksOnMainProcess, - system_info: diagnosticInfo.systemInfo, - execution_variables: diagnosticInfo.executionVariables, - n8n_deployment_type: diagnosticInfo.deploymentType, - n8n_binary_data_mode: diagnosticInfo.binaryDataMode, - smtp_set_up: diagnosticInfo.smtp_set_up, - ldap_allowed: diagnosticInfo.ldap_allowed, - saml_enabled: diagnosticInfo.saml_enabled, - license_plan_name: diagnosticInfo.licensePlanName, - license_tenant_id: diagnosticInfo.licenseTenantId, + version_cli: N8N_VERSION, + db_type: config.getEnv('database.type'), + n8n_version_notifications_enabled: config.getEnv('versionNotifications.enabled'), + n8n_disable_production_main_process: config.getEnv( + 'endpoints.disableProductionWebhooksOnMainProcess', + ), + system_info: { + os: { + type: os.type(), + version: os.version(), + }, + memory: os.totalmem() / 1024, + cpus: { + count: cpus.length, + model: cpus[0].model, + speed: cpus[0].speed, + }, + }, + execution_variables: { + executions_mode: config.getEnv('executions.mode'), + executions_timeout: config.getEnv('executions.timeout'), + executions_timeout_max: config.getEnv('executions.maxTimeout'), + executions_data_save_on_error: config.getEnv('executions.saveDataOnError'), + executions_data_save_on_success: config.getEnv('executions.saveDataOnSuccess'), + executions_data_save_on_progress: config.getEnv('executions.saveExecutionProgress'), + executions_data_save_manual_executions: config.getEnv( + 'executions.saveDataManualExecutions', + ), + executions_data_prune: config.getEnv('executions.pruneData'), + executions_data_max_age: config.getEnv('executions.pruneDataMaxAge'), + }, + n8n_deployment_type: config.getEnv('deployment.type'), + n8n_binary_data_mode: binaryDataConfig.mode, + smtp_set_up: config.getEnv('userManagement.emails.mode') === 'smtp', + ldap_allowed: authenticationMethod === 'ldap', + saml_enabled: authenticationMethod === 'saml', + license_plan_name: this.license.getPlanName(), + license_tenant_id: config.getEnv('license.tenantId'), + binary_data_s3: isS3Available && isS3Selected && isS3Licensed, + multi_main_setup_enabled: config.getEnv('multiMainSetup.enabled'), }; + const firstWorkflow = await this.workflowRepository.findOne({ + select: ['createdAt'], + order: { createdAt: 'ASC' }, + where: {}, + }); + return await Promise.all([ this.telemetry.identify(info), this.telemetry.track('Instance started', { ...info, - earliest_workflow_created: earliestWorkflowCreatedAt, + earliest_workflow_created: firstWorkflow?.createdAt, }), ]); } diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index d594d4bda5..17c5f9492f 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -7,7 +7,6 @@ import { Container, Service } from 'typedi'; import assert from 'assert'; import { exec as callbackExec } from 'child_process'; import { access as fsAccess } from 'fs/promises'; -import os from 'os'; import { join as pathJoin } from 'path'; import { promisify } from 'util'; import cookieParser from 'cookie-parser'; @@ -54,7 +53,7 @@ import { WorkflowStatisticsController } from '@/controllers/workflowStatistics.c import { ExternalSecretsController } from '@/ExternalSecrets/ExternalSecrets.controller.ee'; import { ExecutionsController } from '@/executions/executions.controller'; import { isApiEnabled, loadPublicApiVersions } from '@/PublicApi'; -import type { ICredentialsOverwrite, IDiagnosticInfo } from '@/Interfaces'; +import type { ICredentialsOverwrite } from '@/Interfaces'; import { CredentialsOverwrites } from '@/CredentialsOverwrites'; import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; import * as ResponseHelper from '@/ResponseHelper'; @@ -69,19 +68,12 @@ import { AbstractServer } from './AbstractServer'; import { PostHogClient } from './posthog'; import { MessageEventBus } from '@/eventbus'; import { InternalHooks } from './InternalHooks'; -import { License } from './License'; import { SamlController } from './sso/saml/routes/saml.controller.ee'; import { SamlService } from './sso/saml/saml.service.ee'; import { VariablesController } from './environments/variables/variables.controller.ee'; -import { - isLdapCurrentAuthenticationMethod, - isSamlCurrentAuthenticationMethod, -} from './sso/ssoHelpers'; import { SourceControlService } from '@/environments/sourceControl/sourceControl.service.ee'; import { SourceControlController } from '@/environments/sourceControl/sourceControl.controller.ee'; -import { WorkflowRepository } from '@db/repositories/workflow.repository'; - import { handleMfaDisable, isMfaFeatureEnabled } from './Mfa/helpers'; import type { FrontendService } from './services/frontend.service'; import { ActiveWorkflowsController } from './controllers/activeWorkflows.controller'; @@ -129,71 +121,11 @@ export class Server extends AbstractServer { await super.start(); this.logger.debug(`Server ID: ${this.uniqueInstanceId}`); - const cpus = os.cpus(); - const binaryDataConfig = config.getEnv('binaryDataManager'); - - const isS3Selected = config.getEnv('binaryDataManager.mode') === 's3'; - const isS3Available = config.getEnv('binaryDataManager.availableModes').includes('s3'); - const isS3Licensed = Container.get(License).isBinaryDataS3Licensed(); - - const diagnosticInfo: IDiagnosticInfo = { - databaseType: config.getEnv('database.type'), - disableProductionWebhooksOnMainProcess: config.getEnv( - 'endpoints.disableProductionWebhooksOnMainProcess', - ), - notificationsEnabled: config.getEnv('versionNotifications.enabled'), - versionCli: N8N_VERSION, - systemInfo: { - os: { - type: os.type(), - version: os.version(), - }, - memory: os.totalmem() / 1024, - cpus: { - count: cpus.length, - model: cpus[0].model, - speed: cpus[0].speed, - }, - }, - executionVariables: { - executions_mode: config.getEnv('executions.mode'), - executions_timeout: config.getEnv('executions.timeout'), - executions_timeout_max: config.getEnv('executions.maxTimeout'), - executions_data_save_on_error: config.getEnv('executions.saveDataOnError'), - executions_data_save_on_success: config.getEnv('executions.saveDataOnSuccess'), - executions_data_save_on_progress: config.getEnv('executions.saveExecutionProgress'), - executions_data_save_manual_executions: config.getEnv( - 'executions.saveDataManualExecutions', - ), - executions_data_prune: config.getEnv('executions.pruneData'), - executions_data_max_age: config.getEnv('executions.pruneDataMaxAge'), - }, - deploymentType: config.getEnv('deployment.type'), - binaryDataMode: binaryDataConfig.mode, - smtp_set_up: config.getEnv('userManagement.emails.mode') === 'smtp', - ldap_allowed: isLdapCurrentAuthenticationMethod(), - saml_enabled: isSamlCurrentAuthenticationMethod(), - binary_data_s3: isS3Available && isS3Selected && isS3Licensed, - multi_main_setup_enabled: config.getEnv('multiMainSetup.enabled'), - licensePlanName: Container.get(License).getPlanName(), - licenseTenantId: config.getEnv('license.tenantId'), - }; - if (inDevelopment && process.env.N8N_DEV_RELOAD === 'true') { void this.loadNodesAndCredentials.setupHotReload(); } - void Container.get(WorkflowRepository) - .findOne({ - select: ['createdAt'], - order: { createdAt: 'ASC' }, - where: {}, - }) - .then( - async (workflow) => - await Container.get(InternalHooks).onServerStarted(diagnosticInfo, workflow?.createdAt), - ); - + void Container.get(InternalHooks).onServerStarted(); Container.get(CollaborationService); } diff --git a/packages/cli/test/unit/InternalHooks.test.ts b/packages/cli/test/unit/InternalHooks.test.ts index 4bd10033ac..cd32c35023 100644 --- a/packages/cli/test/unit/InternalHooks.test.ts +++ b/packages/cli/test/unit/InternalHooks.test.ts @@ -1,67 +1,80 @@ -import { Telemetry } from '@/telemetry'; -import { InternalHooks } from '@/InternalHooks'; -import { mockInstance } from '../shared/mocking'; -import type { IDiagnosticInfo } from '@/Interfaces'; import { mock } from 'jest-mock-extended'; +import { N8N_VERSION } from '@/constants'; +import { InternalHooks } from '@/InternalHooks'; +import type { License } from '@/License'; +import type { Telemetry } from '@/telemetry'; -jest.mock('@/telemetry'); - -let internalHooks: InternalHooks; -let telemetry: Telemetry; +jest.mock('@/eventbus'); +jest.mock('node:os', () => ({ + tmpdir: () => '', + cpus: () => [{ model: 'MIPS R3000', speed: 40_000_000 }], + type: () => 'TempleOS', + version: () => '5.03', + totalmem: () => 1024 * 1024, +})); describe('InternalHooks', () => { - beforeAll(() => { - telemetry = mockInstance(Telemetry); - internalHooks = new InternalHooks(telemetry, mock(), mock(), mock(), mock(), mock()); - }); + const telemetry = mock(); + const license = mock(); + const internalHooks = new InternalHooks( + telemetry, + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + license, + ); + + beforeEach(() => jest.clearAllMocks()); it('Should be defined', () => { expect(internalHooks).toBeDefined(); }); it('Should forward license plan name and tenant id to identify when provided', async () => { - const licensePlanName = 'license-plan-name'; - const licenseTenantId = 1001; + license.getPlanName.mockReturnValue('Best Plan'); - const diagnosticInfo: IDiagnosticInfo = { - versionCli: '1.2.3', - databaseType: 'sqlite', - notificationsEnabled: true, - disableProductionWebhooksOnMainProcess: false, - systemInfo: { - os: {}, - cpus: {}, + await internalHooks.onServerStarted(); + + expect(telemetry.identify).toHaveBeenCalledWith({ + version_cli: N8N_VERSION, + db_type: 'sqlite', + n8n_version_notifications_enabled: true, + n8n_disable_production_main_process: false, + system_info: { + memory: 1024, + os: { + type: 'TempleOS', + version: '5.03', + }, + cpus: { + count: 1, + model: 'MIPS R3000', + speed: 40000000, + }, }, - executionVariables: {}, - deploymentType: 'testing', - binaryDataMode: 'default', - smtp_set_up: false, - ldap_allowed: true, - saml_enabled: true, - licensePlanName, - licenseTenantId, + execution_variables: { + executions_data_max_age: 336, + executions_data_prune: true, + executions_data_save_manual_executions: true, + executions_data_save_on_error: 'all', + executions_data_save_on_progress: false, + executions_data_save_on_success: 'all', + executions_mode: 'regular', + executions_timeout: -1, + executions_timeout_max: 3600, + }, + n8n_deployment_type: 'default', + n8n_binary_data_mode: 'default', + smtp_set_up: true, + ldap_allowed: false, + saml_enabled: false, + license_plan_name: 'Best Plan', + license_tenant_id: 1, binary_data_s3: false, multi_main_setup_enabled: false, - }; - - const parameters = { - version_cli: diagnosticInfo.versionCli, - db_type: diagnosticInfo.databaseType, - n8n_version_notifications_enabled: diagnosticInfo.notificationsEnabled, - n8n_disable_production_main_process: diagnosticInfo.disableProductionWebhooksOnMainProcess, - system_info: diagnosticInfo.systemInfo, - execution_variables: diagnosticInfo.executionVariables, - n8n_deployment_type: diagnosticInfo.deploymentType, - n8n_binary_data_mode: diagnosticInfo.binaryDataMode, - smtp_set_up: diagnosticInfo.smtp_set_up, - ldap_allowed: diagnosticInfo.ldap_allowed, - saml_enabled: diagnosticInfo.saml_enabled, - license_plan_name: diagnosticInfo.licensePlanName, - license_tenant_id: diagnosticInfo.licenseTenantId, - }; - - await internalHooks.onServerStarted(diagnosticInfo); - - expect(telemetry.identify).toHaveBeenCalledWith(parameters); + }); }); });