Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate warnings mixin to core #94273

Merged
merged 1 commit into from
Mar 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions src/core/server/environment/environment_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,24 @@ describe('UuidService', () => {
let logger: ReturnType<typeof loggingSystemMock.create>;
let configService: ReturnType<typeof configServiceMock.create>;
let coreContext: CoreContext;
let service: EnvironmentService;

beforeEach(() => {
jest.clearAllMocks();
beforeEach(async () => {
logger = loggingSystemMock.create();
configService = getConfigService();
coreContext = mockCoreContext.create({ logger, configService });

service = new EnvironmentService(coreContext);
});

afterEach(() => {
jest.clearAllMocks();
});

describe('#setup()', () => {
it('calls resolveInstanceUuid with correct parameters', async () => {
const service = new EnvironmentService(coreContext);
await service.setup();

expect(resolveInstanceUuid).toHaveBeenCalledTimes(1);
expect(resolveInstanceUuid).toHaveBeenCalledWith({
pathConfig,
Expand All @@ -83,8 +89,8 @@ describe('UuidService', () => {
});

it('calls createDataFolder with correct parameters', async () => {
const service = new EnvironmentService(coreContext);
await service.setup();

expect(createDataFolder).toHaveBeenCalledTimes(1);
expect(createDataFolder).toHaveBeenCalledWith({
pathConfig,
Expand All @@ -93,8 +99,8 @@ describe('UuidService', () => {
});

it('calls writePidFile with correct parameters', async () => {
const service = new EnvironmentService(coreContext);
await service.setup();

expect(writePidFile).toHaveBeenCalledTimes(1);
expect(writePidFile).toHaveBeenCalledWith({
pidConfig,
Expand All @@ -103,9 +109,31 @@ describe('UuidService', () => {
});

it('returns the uuid resolved from resolveInstanceUuid', async () => {
const service = new EnvironmentService(coreContext);
const setup = await service.setup();

expect(setup.instanceUuid).toEqual('SOME_UUID');
});

describe('process warnings', () => {
it('logs warnings coming from the process', async () => {
await service.setup();

const warning = new Error('something went wrong');
process.emit('warning', warning);

expect(logger.get('process').warn).toHaveBeenCalledTimes(1);
expect(logger.get('process').warn).toHaveBeenCalledWith(warning);
});

it('does not log deprecation warnings', async () => {
await service.setup();

const warning = new Error('something went wrong');
warning.name = 'DeprecationWarning';
process.emit('warning', warning);

expect(logger.get('process').warn).not.toHaveBeenCalled();
});
});
});
});
10 changes: 10 additions & 0 deletions src/core/server/environment/environment_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ export interface InternalEnvironmentServiceSetup {
/** @internal */
export class EnvironmentService {
private readonly log: Logger;
private readonly processLogger: Logger;
private readonly configService: IConfigService;
private uuid: string = '';

constructor(core: CoreContext) {
this.log = core.logger.get('environment');
this.processLogger = core.logger.get('process');
this.configService = core.configService;
}

Expand All @@ -50,6 +52,14 @@ export class EnvironmentService {
this.log.warn(`Detected an unhandled Promise rejection.\n${reason}`);
});

process.on('warning', (warning) => {
// deprecation warnings do no reflect a current problem for the user and should be filtered out.
if (warning.name === 'DeprecationWarning') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: I know we're simply migrating the legacy logic, but I'm wondering: should we log it when in dev mode so we're all aware of those DeprecationWarnings before it's too late?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're logging it in both dev and prod mode. You mean logging it only in dev mode?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are currently skipping DeprecationWarning logs. I meant logging DeprecationWarning entries in dev mode.

return;
}
this.processLogger.warn(warning);
});

await createDataFolder({ pathConfig, logger: this.log });
await writePidFile({ pidConfig, logger: this.log });

Expand Down
13 changes: 0 additions & 13 deletions src/legacy/server/config/complete.js

This file was deleted.

54 changes: 0 additions & 54 deletions src/legacy/server/config/complete.test.js

This file was deleted.

6 changes: 0 additions & 6 deletions src/legacy/server/kbn_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import { Config } from './config';
import httpMixin from './http';
import { coreMixin } from './core';
import { loggingMixin } from './logging';
import warningsMixin from './warnings';
import configCompleteMixin from './config/complete';
import { optimizeMixin } from '../../optimize';

/**
Expand Down Expand Up @@ -66,10 +64,6 @@ export default class KbnServer {
coreMixin,

loggingMixin,
warningsMixin,

// tell the config we are done loading plugins
configCompleteMixin,

// setup routes that serve the @kbn/optimizer output
optimizeMixin
Expand Down
19 changes: 0 additions & 19 deletions src/legacy/server/warnings/index.js

This file was deleted.