Revert "Include tracing information in the Kibana logs (#112973) (#11…

…9567) * Revert "Include tracing information in the Kibana logs (#112973) (#118604)" This reverts commit 114c690. * Revert "Don't enable RUM agent if APM is run with contextPropagationOnly (#118685) (#118995)" This reverts commit ca86b98.
elastic · Nov 24, 2021 · 8dcf2b7 · 8dcf2b7
1 parent d8b02cc
commit 8dcf2b7
Show file tree

Hide file tree

Showing 25 changed files with 33 additions and 413 deletions.
diff --git a/.buildkite/scripts/common/env.sh b/.buildkite/scripts/common/env.sh
@@ -40,10 +40,8 @@ export ELASTIC_APM_SECRET_TOKEN=7YKhoXsO4MzjhXjx2c
 if is_pr; then
   if [[ "${GITHUB_PR_LABELS:-}" == *"ci:collect-apm"* ]]; then
     export ELASTIC_APM_ACTIVE=true
-    export ELASTIC_APM_CONTEXT_PROPAGATION_ONLY=false
   else
-    export ELASTIC_APM_ACTIVE=true
-    export ELASTIC_APM_CONTEXT_PROPAGATION_ONLY=true
+    export ELASTIC_APM_ACTIVE=false
   fi
 
   if [[ "${GITHUB_STEP_COMMIT_STATUS_ENABLED:-}" != "true" ]]; then
@@ -65,7 +63,6 @@ if is_pr; then
   export PR_TARGET_BRANCH="$GITHUB_PR_TARGET_BRANCH"
 else
   export ELASTIC_APM_ACTIVE=true
-  export ELASTIC_APM_CONTEXT_PROPAGATION_ONLY=false
   export CHECKS_REPORTER_ACTIVE=false
 fi
 

diff --git a/package.json b/package.json
@@ -217,7 +217,7 @@
     "deep-freeze-strict": "^1.1.1",
     "deepmerge": "^4.2.2",
     "del": "^5.1.0",
-    "elastic-apm-node": "3.24.0",
+    "elastic-apm-node": "^3.23.0",
     "execa": "^4.0.2",
     "exit-hook": "^2.2.0",
     "expiry-js": "0.1.7",

diff --git a/packages/kbn-apm-config-loader/src/config.test.ts b/packages/kbn-apm-config-loader/src/config.test.ts
@@ -21,7 +21,7 @@ describe('ApmConfiguration', () => {
   beforeEach(() => {
     // start with an empty env to avoid CI from spoiling snapshots, env is unique for each jest file
     process.env = {};
-    devConfigMock.raw = {};
+
     packageMock.raw = {
       version: '8.0.0',
       build: {
@@ -86,11 +86,10 @@ describe('ApmConfiguration', () => {
     let config = new ApmConfiguration(mockedRootDir, {}, false);
     expect(config.getConfig('serviceName')).toMatchInlineSnapshot(`
       Object {
-        "active": true,
+        "active": false,
         "breakdownMetrics": true,
         "captureSpanStackTraces": false,
         "centralConfig": false,
-        "contextPropagationOnly": true,
         "environment": "development",
         "globalLabels": Object {},
         "logUncaughtExceptions": true,
@@ -107,13 +106,12 @@ describe('ApmConfiguration', () => {
     config = new ApmConfiguration(mockedRootDir, {}, true);
     expect(config.getConfig('serviceName')).toMatchInlineSnapshot(`
       Object {
-        "active": true,
+        "active": false,
         "breakdownMetrics": false,
         "captureBody": "off",
         "captureHeaders": false,
         "captureSpanStackTraces": false,
         "centralConfig": false,
-        "contextPropagationOnly": true,
         "environment": "development",
         "globalLabels": Object {
           "git_rev": "sha",
@@ -166,12 +164,13 @@ describe('ApmConfiguration', () => {
 
   it('does not load the configuration from the dev config in distributable', () => {
     devConfigMock.raw = {
-      active: false,
+      active: true,
+      serverUrl: 'https://dev-url.co',
     };
     const config = new ApmConfiguration(mockedRootDir, {}, true);
     expect(config.getConfig('serviceName')).toEqual(
       expect.objectContaining({
-        active: true,
+        active: false,
       })
     );
   });
@@ -227,130 +226,4 @@ describe('ApmConfiguration', () => {
       })
     );
   });
-
-  describe('contextPropagationOnly', () => {
-    it('sets "active: true" and "contextPropagationOnly: true" by default', () => {
-      expect(new ApmConfiguration(mockedRootDir, {}, false).getConfig('serviceName')).toEqual(
-        expect.objectContaining({
-          active: true,
-          contextPropagationOnly: true,
-        })
-      );
-
-      expect(new ApmConfiguration(mockedRootDir, {}, true).getConfig('serviceName')).toEqual(
-        expect.objectContaining({
-          active: true,
-          contextPropagationOnly: true,
-        })
-      );
-    });
-
-    it('value from config overrides the default', () => {
-      const kibanaConfig = {
-        elastic: {
-          apm: {
-            active: false,
-            contextPropagationOnly: false,
-          },
-        },
-      };
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: false,
-          contextPropagationOnly: false,
-        })
-      );
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: false,
-          contextPropagationOnly: false,
-        })
-      );
-    });
-
-    it('is "false" if "active: true" configured and "contextPropagationOnly" is not specified', () => {
-      const kibanaConfig = {
-        elastic: {
-          apm: {
-            active: true,
-          },
-        },
-      };
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: true,
-          contextPropagationOnly: false,
-        })
-      );
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: true,
-          contextPropagationOnly: false,
-        })
-      );
-    });
-
-    it('throws if "active: false" set without configuring "contextPropagationOnly: false"', () => {
-      const kibanaConfig = {
-        elastic: {
-          apm: {
-            active: false,
-          },
-        },
-      };
-
-      expect(() =>
-        new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
-      ).toThrowErrorMatchingInlineSnapshot(
-        `"APM is disabled, but context propagation is enabled. Please disable context propagation with contextPropagationOnly:false"`
-      );
-
-      expect(() =>
-        new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
-      ).toThrowErrorMatchingInlineSnapshot(
-        `"APM is disabled, but context propagation is enabled. Please disable context propagation with contextPropagationOnly:false"`
-      );
-    });
-
-    it('does not throw if "active: false" and "contextPropagationOnly: false" configured', () => {
-      const kibanaConfig = {
-        elastic: {
-          apm: {
-            active: false,
-            contextPropagationOnly: false,
-          },
-        },
-      };
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: false,
-          contextPropagationOnly: false,
-        })
-      );
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: false,
-          contextPropagationOnly: false,
-        })
-      );
-    });
-  });
 });
diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts
@@ -17,8 +17,7 @@ import type { AgentConfigOptions as RUMAgentConfigOptions } from '@elastic/apm-r
 
 // https://www.elastic.co/guide/en/apm/agent/nodejs/current/configuration.html
 const DEFAULT_CONFIG: AgentConfigOptions = {
-  active: true,
-  contextPropagationOnly: true,
+  active: false,
   environment: 'development',
   logUncaughtExceptions: true,
   globalLabels: {},
@@ -75,8 +74,6 @@ export class ApmConfiguration {
 
   private getBaseConfig() {
     if (!this.baseConfig) {
-      const configFromSources = this.getConfigFromAllSources();
-
       this.baseConfig = merge(
         {
           serviceVersion: this.kibanaVersion,
@@ -85,7 +82,9 @@ export class ApmConfiguration {
         this.getUuidConfig(),
         this.getGitConfig(),
         this.getCiConfig(),
-        configFromSources
+        this.getConfigFromKibanaConfig(),
+        this.getDevConfig(),
+        this.getConfigFromEnv()
       );
 
       /**
@@ -118,12 +117,6 @@ export class ApmConfiguration {
       config.active = true;
     }
 
-    if (process.env.ELASTIC_APM_CONTEXT_PROPAGATION_ONLY === 'true') {
-      config.contextPropagationOnly = true;
-    } else if (process.env.ELASTIC_APM_CONTEXT_PROPAGATION_ONLY === 'false') {
-      config.contextPropagationOnly = false;
-    }
-
     if (process.env.ELASTIC_APM_ENVIRONMENT || process.env.NODE_ENV) {
       config.environment = process.env.ELASTIC_APM_ENVIRONMENT || process.env.NODE_ENV;
     }
@@ -260,28 +253,4 @@ export class ApmConfiguration {
       return {};
     }
   }
-
-  /**
-   * Reads APM configuration from different sources and merges them together.
-   */
-  private getConfigFromAllSources(): AgentConfigOptions {
-    const config = merge(
-      {},
-      this.getConfigFromKibanaConfig(),
-      this.getDevConfig(),
-      this.getConfigFromEnv()
-    );
-
-    if (config.active === false && config.contextPropagationOnly !== false) {
-      throw new Error(
-        'APM is disabled, but context propagation is enabled. Please disable context propagation with contextPropagationOnly:false'
-      );
-    }
-
-    if (config.active === true) {
-      config.contextPropagationOnly = config.contextPropagationOnly ?? false;
-    }
-
-    return config;
-  }
 }
diff --git a/packages/kbn-apm-config-loader/src/index.ts b/packages/kbn-apm-config-loader/src/index.ts
@@ -8,5 +8,4 @@
 
 export { getConfiguration } from './config_loader';
 export { initApm } from './init_apm';
-export { shouldInstrumentClient } from './rum_agent_configuration';
 export type { ApmConfiguration } from './config';
diff --git a/packages/kbn-apm-config-loader/src/rum_agent_configuration.test.ts b/packages/kbn-apm-config-loader/src/rum_agent_configuration.test.ts
diff --git a/packages/kbn-apm-config-loader/src/rum_agent_configuration.ts b/packages/kbn-apm-config-loader/src/rum_agent_configuration.ts
diff --git a/packages/kbn-logging/src/log_record.ts b/packages/kbn-logging/src/log_record.ts
@@ -21,7 +21,4 @@ export interface LogRecord {
   error?: Error;
   meta?: LogMeta;
   pid: number;
-  spanId?: string;
-  traceId?: string;
-  transactionId?: string;
 }
diff --git a/src/core/server/http_resources/get_apm_config.test.mocks.ts b/src/core/server/http_resources/get_apm_config.test.mocks.ts
@@ -7,10 +7,8 @@
  */
 
 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>;

diff --git a/src/core/server/http_resources/get_apm_config.test.ts b/src/core/server/http_resources/get_apm_config.test.ts
@@ -6,11 +6,7 @@
  * Side Public License, v 1.
  */
 
-import {
-  getConfigurationMock,
-  agentMock,
-  shouldInstrumentClientMock,
-} from './get_apm_config.test.mocks';
+import { getConfigurationMock, agentMock } from './get_apm_config.test.mocks';
 import { getApmConfig } from './get_apm_config';
 
 const defaultApmConfig = {
@@ -21,7 +17,6 @@ const defaultApmConfig = {
 describe('getApmConfig', () => {
   beforeEach(() => {
     getConfigurationMock.mockReturnValue(defaultApmConfig);
-    shouldInstrumentClientMock.mockReturnValue(true);
   });
 
   afterEach(() => {
@@ -30,7 +25,12 @@ describe('getApmConfig', () => {
   });
 
   it('returns null if apm is disabled', () => {
-    shouldInstrumentClientMock.mockReturnValue(false);
+    getConfigurationMock.mockReturnValue({
+      active: false,
+    });
+    expect(getApmConfig('/path')).toBeNull();
+
+    getConfigurationMock.mockReturnValue(undefined);
     expect(getApmConfig('/path')).toBeNull();
   });