diff --git a/src/core/server/logging/appenders/rolling_file/strategies/fs.ts b/src/core/server/logging/appenders/rolling_file/strategies/fs.ts deleted file mode 100644 index 5b089cc108de9..0000000000000 --- a/src/core/server/logging/appenders/rolling_file/strategies/fs.ts +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import Fs from 'fs'; -import { promisify } from 'util'; - -export const readdir = promisify(Fs.readdir); -export const unlink = promisify(Fs.unlink); -export const rename = promisify(Fs.rename); -export const exists = promisify(Fs.exists); diff --git a/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.mocks.ts b/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.mocks.ts index 16ae8264edcf8..4355ec7ffb2ec 100644 --- a/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.mocks.ts +++ b/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.mocks.ts @@ -20,18 +20,18 @@ export const readdirMock = jest.fn(); export const unlinkMock = jest.fn(); export const renameMock = jest.fn(); -export const existsMock = jest.fn(); +export const accessMock = jest.fn(); -jest.doMock('../fs', () => ({ +jest.doMock('fs/promises', () => ({ readdir: readdirMock, unlink: unlinkMock, rename: renameMock, - exists: existsMock, + access: accessMock, })); export const clearAllMocks = () => { readdirMock.mockClear(); unlinkMock.mockClear(); renameMock.mockClear(); - existsMock.mockClear(); + accessMock.mockClear(); }; diff --git a/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.ts b/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.ts index e730e00d98757..469ea450485a1 100644 --- a/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.ts +++ b/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.test.ts @@ -19,7 +19,7 @@ import { join } from 'path'; import { - existsMock, + accessMock, readdirMock, renameMock, unlinkMock, @@ -42,16 +42,18 @@ describe('NumericRollingStrategy tasks', () => { it('calls `exists` with the correct parameters', async () => { await shouldSkipRollout({ logFilePath: 'some-file' }); - expect(existsMock).toHaveBeenCalledTimes(1); - expect(existsMock).toHaveBeenCalledWith('some-file'); + expect(accessMock).toHaveBeenCalledTimes(1); + expect(accessMock).toHaveBeenCalledWith('some-file'); }); it('returns `true` if the file is current log file does not exist', async () => { - existsMock.mockResolvedValue(false); + accessMock.mockImplementation(() => { + throw new Error('ENOENT'); + }); expect(await shouldSkipRollout({ logFilePath: 'some-file' })).toEqual(true); }); it('returns `false` if the file is current log file exists', async () => { - existsMock.mockResolvedValue(true); + accessMock.mockResolvedValue(undefined); expect(await shouldSkipRollout({ logFilePath: 'some-file' })).toEqual(false); }); @@ -128,7 +130,12 @@ describe('NumericRollingStrategy tasks', () => { describe('getOrderedRolledFiles', () => { it('returns the rolled files matching the pattern in order', async () => { - readdirMock.mockResolvedValue(['kibana-3.log', 'kibana-1.log', 'kibana-2.log']); + readdirMock.mockResolvedValue([ + 'kibana-10.log', + 'kibana-1.log', + 'kibana-12.log', + 'kibana-2.log', + ]); const files = await getOrderedRolledFiles({ logFileFolder: 'log-folder', @@ -136,7 +143,7 @@ describe('NumericRollingStrategy tasks', () => { pattern: '-%i', }); - expect(files).toEqual(['kibana-1.log', 'kibana-2.log', 'kibana-3.log']); + expect(files).toEqual(['kibana-1.log', 'kibana-2.log', 'kibana-10.log', 'kibana-12.log']); }); it('ignores files that do no match the pattern', async () => { diff --git a/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.ts b/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.ts index d14f24dd44fb3..6fe065c5c1561 100644 --- a/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.ts +++ b/src/core/server/logging/appenders/rolling_file/strategies/numeric/rolling_tasks.ts @@ -18,14 +18,19 @@ */ import { join } from 'path'; +import { readdir, rename, unlink, access } from 'fs/promises'; import { getFileNameMatcher, getRollingFileName } from './pattern_matcher'; -import { readdir, rename, unlink, exists } from '../fs'; export const shouldSkipRollout = async ({ logFilePath }: { logFilePath: string }) => { // in case of time-interval triggering policy, we can have an entire // interval without any log event. In that case, the log file is not even // present, and we should not perform the rollout - return !(await exists(logFilePath)); + try { + await access(logFilePath); + return false; + } catch (e) { + return true; + } }; /** @@ -59,9 +64,7 @@ export const deleteFiles = async ({ logFileFolder: string; filesToDelete: string[]; }) => { - for (const fileToDelete of filesToDelete) { - await unlink(join(logFileFolder, fileToDelete)); - } + await Promise.all(filesToDelete.map((fileToDelete) => unlink(join(logFileFolder, fileToDelete)))); }; export const rollPreviousFilesInOrder = async ({ diff --git a/src/core/server/logging/integration_tests/rolling_file_appender.test.ts b/src/core/server/logging/integration_tests/rolling_file_appender.test.ts index b83e1d2821520..4680740195b44 100644 --- a/src/core/server/logging/integration_tests/rolling_file_appender.test.ts +++ b/src/core/server/logging/integration_tests/rolling_file_appender.test.ts @@ -18,7 +18,7 @@ */ import { join } from 'path'; -import { rmdirSync, mkdtempSync, readFileSync, readdirSync } from 'fs'; +import { rmdir, mkdtemp, readFile, readdir } from 'fs/promises'; import moment from 'moment-timezone'; import * as kbnTestServer from '../../../test_helpers/kbn_server'; import { getNextRollingTime } from '../appenders/rolling_file/policies/time_interval/get_next_rolling_time'; @@ -50,17 +50,17 @@ describe('RollingFileAppender', () => { let testDir: string; let logFile: string; - const getFileContent = (basename: string) => - readFileSync(join(testDir, basename)).toString('utf-8'); + const getFileContent = async (basename: string) => + (await readFile(join(testDir, basename))).toString('utf-8'); beforeEach(async () => { - testDir = mkdtempSync('rolling-test'); + testDir = await mkdtemp('rolling-test'); logFile = join(testDir, 'kibana.log'); }); afterEach(async () => { try { - rmdirSync(testDir); + await rmdir(testDir); } catch (e) { /* trap */ } @@ -110,12 +110,12 @@ describe('RollingFileAppender', () => { await flush(); - const files = readdirSync(testDir).sort(); + const files = await readdir(testDir); - expect(files).toEqual(['kibana.1.log', 'kibana.2.log', 'kibana.log']); - expect(getFileContent('kibana.log')).toEqual(expectedFileContent([7])); - expect(getFileContent('kibana.1.log')).toEqual(expectedFileContent([4, 5, 6])); - expect(getFileContent('kibana.2.log')).toEqual(expectedFileContent([1, 2, 3])); + expect(files.sort()).toEqual(['kibana.1.log', 'kibana.2.log', 'kibana.log']); + expect(await getFileContent('kibana.log')).toEqual(expectedFileContent([7])); + expect(await getFileContent('kibana.1.log')).toEqual(expectedFileContent([4, 5, 6])); + expect(await getFileContent('kibana.2.log')).toEqual(expectedFileContent([1, 2, 3])); }); it('only keep the correct number of files', async () => { @@ -157,12 +157,12 @@ describe('RollingFileAppender', () => { await flush(); - const files = readdirSync(testDir).sort(); + const files = await readdir(testDir); - expect(files).toEqual(['kibana-1.log', 'kibana-2.log', 'kibana.log']); - expect(getFileContent('kibana.log')).toEqual(expectedFileContent([7, 8])); - expect(getFileContent('kibana-1.log')).toEqual(expectedFileContent([5, 6])); - expect(getFileContent('kibana-2.log')).toEqual(expectedFileContent([3, 4])); + expect(files.sort()).toEqual(['kibana-1.log', 'kibana-2.log', 'kibana.log']); + expect(await getFileContent('kibana.log')).toEqual(expectedFileContent([7, 8])); + expect(await getFileContent('kibana-1.log')).toEqual(expectedFileContent([5, 6])); + expect(await getFileContent('kibana-2.log')).toEqual(expectedFileContent([3, 4])); }); }); @@ -210,11 +210,11 @@ describe('RollingFileAppender', () => { await flush(); - const files = readdirSync(testDir).sort(); + const files = await readdir(testDir); - expect(files).toEqual(['kibana-1.log', 'kibana.log']); - expect(getFileContent('kibana.log')).toEqual(expectedFileContent([3, 4])); - expect(getFileContent('kibana-1.log')).toEqual(expectedFileContent([1, 2])); + expect(files.sort()).toEqual(['kibana-1.log', 'kibana.log']); + expect(await getFileContent('kibana.log')).toEqual(expectedFileContent([3, 4])); + expect(await getFileContent('kibana-1.log')).toEqual(expectedFileContent([1, 2])); }); }); });