Skip to content

Commit

Permalink
Revert "Include tracing information in the Kibana logs (elastic#112973)…
Browse files Browse the repository at this point in the history 
… (elastic#118604)"

This reverts commit 114c690.
  • Loading branch information
restrry committed Nov 24, 2021
1 parent 53315b2 commit f276208
Show file tree
Hide file tree
Showing 19 changed files with 24 additions and 360 deletions.
5 changes: 1 addition & 4 deletions .buildkite/scripts/common/env.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
139 changes: 6 additions & 133 deletions packages/kbn-apm-config-loader/src/config.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand Down Expand Up @@ -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,
})
);
});
Expand Down Expand Up @@ -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,
})
);
});
});
});
39 changes: 4 additions & 35 deletions packages/kbn-apm-config-loader/src/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
Expand Down Expand Up @@ -75,8 +74,6 @@ export class ApmConfiguration {

private getBaseConfig() {
if (!this.baseConfig) {
const configFromSources = this.getConfigFromAllSources();

this.baseConfig = merge(
{
serviceVersion: this.kibanaVersion,
Expand All @@ -85,7 +82,9 @@ export class ApmConfiguration {
this.getUuidConfig(),
this.getGitConfig(),
this.getCiConfig(),
configFromSources
this.getConfigFromKibanaConfig(),
this.getDevConfig(),
this.getConfigFromEnv()
);

/**
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
3 changes: 0 additions & 3 deletions packages/kbn-logging/src/log_record.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,4 @@ export interface LogRecord {
error?: Error;
meta?: LogMeta;
pid: number;
spanId?: string;
traceId?: string;
transactionId?: string;
}
6 changes: 0 additions & 6 deletions src/core/server/logging/__snapshots__/logging_system.test.ts.snap
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 0 additions & 23 deletions src/core/server/logging/layouts/__snapshots__/json_layout.test.ts.snap
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 0 additions & 47 deletions src/core/server/logging/layouts/json_layout.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,6 @@ const records: LogRecord[] = [
timestamp,
pid: 5355,
},
{
context: 'context-7',
level: LogLevel.Trace,
message: 'message-6',
timestamp,
pid: 5355,
spanId: 'spanId-1',
traceId: 'traceId-1',
transactionId: 'transactionId-1',
},
];

test('`createConfigSchema()` creates correct schema.', () => {
Expand Down Expand Up @@ -327,40 +317,3 @@ test('format() meta can not override timestamp', () => {
},
});
});

test('format() meta can not override tracing properties', () => {
const layout = new JsonLayout();
expect(
JSON.parse(
layout.format({
message: 'foo',
timestamp,
level: LogLevel.Debug,
context: 'bar',
pid: 3,
meta: {
span: { id: 'span_override' },
trace: { id: 'trace_override' },
transaction: { id: 'transaction_override' },
},
spanId: 'spanId-1',
traceId: 'traceId-1',
transactionId: 'transactionId-1',
})
)
).toStrictEqual({
ecs: { version: expect.any(String) },
'@timestamp': '2012-02-01T09:30:22.011-05:00',
message: 'foo',
log: {
level: 'DEBUG',
logger: 'bar',
},
process: {
pid: 3,
},
span: { id: 'spanId-1' },
trace: { id: 'traceId-1' },
transaction: { id: 'transactionId-1' },
});
});
Loading

0 comments on commit f276208

Please sign in to comment.