Skip to content

Commit

Permalink
Don't enable RUM agent if APM is run with contextPropagationOnly (#11…
Browse files Browse the repository at this point in the history
…8685) (#118995)

* do not enable RUM agent when nodejs is run in contextPropagationOnly mode

* move shouldInstrumentClient to apm-config package

Co-authored-by: Mikhail Shustov <mikhail.shustov@elastic.co>
  • Loading branch information
kibanamachine and Mikhail Shustov authored Nov 18, 2021
1 parent 9b2d723 commit ca86b98
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 9 deletions.
1 change: 1 addition & 0 deletions packages/kbn-apm-config-loader/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@

export { getConfiguration } from './config_loader';
export { initApm } from './init_apm';
export { shouldInstrumentClient } from './rum_agent_configuration';
export type { ApmConfiguration } from './config';
27 changes: 27 additions & 0 deletions packages/kbn-apm-config-loader/src/rum_agent_configuration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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 { shouldInstrumentClient } from './rum_agent_configuration';
describe('shouldInstrumentClient', () => {
it('returns false if apm is disabled', () => {
expect(shouldInstrumentClient({ active: false })).toBe(false);
});

it('returns false if apm is enabled with contextPropagationOnly: true', () => {
expect(shouldInstrumentClient({ active: true, contextPropagationOnly: true })).toBe(false);
});

it('returns false if apm is enabled with disableSend: true', () => {
expect(shouldInstrumentClient({ active: true, disableSend: true })).toBe(false);
});

it('returns true if apm is enabled', () => {
expect(shouldInstrumentClient({ active: true })).toBe(true);
expect(shouldInstrumentClient({ active: true, contextPropagationOnly: false })).toBe(true);
expect(shouldInstrumentClient({ active: true, disableSend: false })).toBe(true);
});
});
14 changes: 14 additions & 0 deletions packages/kbn-apm-config-loader/src/rum_agent_configuration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* 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 type { AgentConfigOptions } from 'elastic-apm-node';

export function shouldInstrumentClient(config?: AgentConfigOptions): boolean {
return Boolean(
config?.active === true && config.contextPropagationOnly !== true && config.disableSend !== true
);
}
2 changes: 2 additions & 0 deletions src/core/server/http_resources/get_apm_config.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
*/

export const getConfigurationMock = jest.fn();
export const shouldInstrumentClientMock = jest.fn(() => true);
jest.doMock('@kbn/apm-config-loader', () => ({
getConfiguration: getConfigurationMock,
shouldInstrumentClient: shouldInstrumentClientMock,
}));

export const agentMock = {} as Record<string, any>;
Expand Down
14 changes: 7 additions & 7 deletions src/core/server/http_resources/get_apm_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
* Side Public License, v 1.
*/

import { getConfigurationMock, agentMock } from './get_apm_config.test.mocks';
import {
getConfigurationMock,
agentMock,
shouldInstrumentClientMock,
} from './get_apm_config.test.mocks';
import { getApmConfig } from './get_apm_config';

const defaultApmConfig = {
Expand All @@ -17,6 +21,7 @@ const defaultApmConfig = {
describe('getApmConfig', () => {
beforeEach(() => {
getConfigurationMock.mockReturnValue(defaultApmConfig);
shouldInstrumentClientMock.mockReturnValue(true);
});

afterEach(() => {
Expand All @@ -25,12 +30,7 @@ describe('getApmConfig', () => {
});

it('returns null if apm is disabled', () => {
getConfigurationMock.mockReturnValue({
active: false,
});
expect(getApmConfig('/path')).toBeNull();

getConfigurationMock.mockReturnValue(undefined);
shouldInstrumentClientMock.mockReturnValue(false);
expect(getApmConfig('/path')).toBeNull();
});

Expand Down
4 changes: 2 additions & 2 deletions src/core/server/http_resources/get_apm_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
*/

import agent from 'elastic-apm-node';
import { getConfiguration } from '@kbn/apm-config-loader';
import { getConfiguration, shouldInstrumentClient } from '@kbn/apm-config-loader';

export const getApmConfig = (requestPath: string) => {
const baseConfig = getConfiguration('kibana-frontend');
if (!baseConfig?.active) {
if (!shouldInstrumentClient(baseConfig)) {
return null;
}

Expand Down

0 comments on commit ca86b98

Please sign in to comment.