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

Include tracing information in the Kibana logs #112973

Merged
merged 40 commits into from
Nov 11, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
75bd5ee
change APM nodejs agent default
mshustov Sep 23, 2021
bdc9928
Merge branch 'master' into issue-102704-change-apm-default
mshustov Sep 28, 2021
2b2e5cd
Merge branch 'master' into issue-102704-change-apm-default
mshustov Sep 29, 2021
a030d68
emit trace IDs into the logs
mshustov Sep 30, 2021
cc1598c
Merge branch 'master' into issue-102704-change-apm-default
mshustov Sep 30, 2021
5d823c9
Merge branch 'master' into issue-102704-change-apm-default
mshustov Oct 1, 2021
99d2c40
use ELASTIC_APM_DISABLE_SEND to keep APM agent active but disable sen…
mshustov Oct 4, 2021
34e7914
send data whenver active is set to "true"
mshustov Oct 4, 2021
07a133b
Merge branch 'master' into issue-102704-change-apm-default
mshustov Oct 5, 2021
16610f1
update tests
mshustov Oct 5, 2021
8f288ca
keep APM agent active. control disableSend instead
mshustov Oct 5, 2021
4905c34
Merge branch 'master' into issue-102704-change-apm-default
mshustov Oct 11, 2021
861efdb
update snapshot tests
mshustov Oct 11, 2021
b7acb1a
add debug logging
mshustov Oct 11, 2021
8b17412
REMOVE me. log path to the agent
mshustov Oct 11, 2021
1972227
init APM agent explicitly in test plugin. it uses another package ins…
mshustov Oct 12, 2021
1887300
REMOVE me. create transaction explicitly
mshustov Oct 12, 2021
2e89205
increase timeout setting for the test
mshustov Oct 12, 2021
c3a167e
Merge branch 'master' into issue-102704-change-apm-default
mshustov Oct 12, 2021
464b3e2
refactor tests
mshustov Oct 12, 2021
2b4ff4b
remove debug logs
mshustov Oct 12, 2021
cdf2d30
remove explicit transaction creation
mshustov Oct 12, 2021
ac450a0
Revert "remove explicit transaction creation"
mshustov Oct 13, 2021
0a909b2
Merge branch 'master' into issue-102704-change-apm-default
mshustov Oct 13, 2021
b7a0127
Merge remote-tracking branch 'origin/issue-102704-change-apm-default'…
mshustov Oct 17, 2021
8317597
Merge branch 'master' into issue-102704-change-apm-default
mshustov Oct 17, 2021
16cfd22
Merge branch 'master' into issue-102704-change-apm-default
mshustov Oct 17, 2021
39ff4a2
Merge branch 'main' into issue-102704-change-apm-default
mshustov Nov 2, 2021
626cbe0
Merge branch 'main' into issue-102704-change-apm-default
mshustov Nov 3, 2021
95e4199
Merge branch 'main' into issue-102704-change-apm-default
mshustov Nov 5, 2021
73272b5
point to apm nodejs agent commit temporary until a new version is rel…
mshustov Nov 5, 2021
c25a31b
migrate from disableSend to contextPropagationOnly
mshustov Nov 5, 2021
62dda4f
TO DISCUSS. what if we enforce contextPropagationOnly to be configure…
mshustov Nov 5, 2021
bb6251b
Revert "TO DISCUSS. what if we enforce contextPropagationOnly to be c…
mshustov Nov 7, 2021
e2067e5
Merge branch 'main' into issue-102704-change-apm-default
mshustov Nov 8, 2021
cbd94bd
bump to version with fix
mshustov Nov 10, 2021
6417b5b
Merge branch 'main' into issue-102704-change-apm-default
mshustov Nov 10, 2021
bf121b0
commit using @elastic.co
Nov 10, 2021
c611f73
Merge branch 'main' into issue-102704-change-apm-default
kibanamachine Nov 10, 2021
4b025d2
Merge branch 'main' into issue-102704-change-apm-default
kibanamachine Nov 10, 2021
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
5 changes: 4 additions & 1 deletion .buildkite/scripts/common/env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ export ELASTIC_APM_TRANSACTION_SAMPLE_RATE=0.1
if is_pr; then
if [[ "${GITHUB_PR_LABELS:-}" == *"ci:collect-apm"* ]]; then
export ELASTIC_APM_ACTIVE=true
export ELASTIC_APM_DISABLE_SEND=false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

apm agent is always active, we use disableSend flag to enable sending data to the APM server

else
export ELASTIC_APM_ACTIVE=false
export ELASTIC_APM_ACTIVE=true
export ELASTIC_APM_DISABLE_SEND=true
fi

export CHECKS_REPORTER_ACTIVE=true
Expand All @@ -57,6 +59,7 @@ if is_pr; then
export PR_TARGET_BRANCH="$GITHUB_PR_TARGET_BRANCH"
else
export ELASTIC_APM_ACTIVE=true
export ELASTIC_APM_DISABLE_SEND=false
export CHECKS_REPORTER_ACTIVE=false
fi

Expand Down
116 changes: 110 additions & 6 deletions packages/kbn-apm-config-loader/src/config.test.ts
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,10 +86,11 @@ describe('ApmConfiguration', () => {
let config = new ApmConfiguration(mockedRootDir, {}, false);
expect(config.getConfig('serviceName')).toMatchInlineSnapshot(`
Object {
"active": false,
"active": true,
"breakdownMetrics": true,
"captureSpanStackTraces": false,
"centralConfig": false,
"disableSend": true,
"environment": "development",
"globalLabels": Object {},
"logUncaughtExceptions": true,
Expand All @@ -105,12 +106,13 @@ describe('ApmConfiguration', () => {
config = new ApmConfiguration(mockedRootDir, {}, true);
expect(config.getConfig('serviceName')).toMatchInlineSnapshot(`
Object {
"active": false,
"active": true,
"breakdownMetrics": false,
"captureBody": "off",
"captureHeaders": false,
"captureSpanStackTraces": false,
"centralConfig": false,
"disableSend": true,
"environment": "development",
"globalLabels": Object {
"git_rev": "sha",
Expand Down Expand Up @@ -162,13 +164,12 @@ describe('ApmConfiguration', () => {

it('does not load the configuration from the dev config in distributable', () => {
devConfigMock.raw = {
active: true,
serverUrl: 'https://dev-url.co',
active: false,
};
const config = new ApmConfiguration(mockedRootDir, {}, true);
expect(config.getConfig('serviceName')).toEqual(
expect.objectContaining({
active: false,
active: true,
})
);
});
Expand Down Expand Up @@ -224,4 +225,107 @@ describe('ApmConfiguration', () => {
})
);
});

describe('disableSend', () => {
it('sets "active: true" and "disableSend: true" by default', () => {
expect(new ApmConfiguration(mockedRootDir, {}, false).getConfig('serviceName')).toEqual(
expect.objectContaining({
active: true,
disableSend: true,
})
);

expect(new ApmConfiguration(mockedRootDir, {}, true).getConfig('serviceName')).toEqual(
expect.objectContaining({
active: true,
disableSend: true,
})
);
});

it('value from config overrides the default', () => {
const kibanaConfig = {
elastic: {
apm: {
active: false,
disableSend: false,
},
},
};

expect(
new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
).toEqual(
expect.objectContaining({
active: false,
disableSend: false,
})
);

expect(
new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
).toEqual(
expect.objectContaining({
active: false,
disableSend: false,
})
);
});

it('is "false" if "active:true" and "disableSend" is not specified', () => {
const kibanaConfig = {
elastic: {
apm: {
active: true,
},
},
};

expect(
new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
).toEqual(
expect.objectContaining({
active: true,
disableSend: false,
})
);

expect(
new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
).toEqual(
expect.objectContaining({
active: true,
disableSend: false,
})
);
});

it('is "true" if "active:false" and "disableSend" is not specified', () => {
const kibanaConfig = {
elastic: {
apm: {
active: false,
},
},
};

expect(
new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
).toEqual(
expect.objectContaining({
active: false,
disableSend: true,
})
);

expect(
new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
).toEqual(
expect.objectContaining({
active: false,
disableSend: true,
})
);
});
});
});
17 changes: 15 additions & 2 deletions packages/kbn-apm-config-loader/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import type { AgentConfigOptions } from 'elastic-apm-node';

// https://www.elastic.co/guide/en/apm/agent/nodejs/current/configuration.html
const DEFAULT_CONFIG: AgentConfigOptions = {
active: false,
active: true,
disableSend: true,
environment: 'development',
logUncaughtExceptions: true,
globalLabels: {},
Expand Down Expand Up @@ -112,6 +113,13 @@ export class ApmConfiguration {

if (process.env.ELASTIC_APM_ACTIVE === 'true') {
config.active = true;
config.disableSend = false;
}

if (process.env.ELASTIC_APM_DISABLE_SEND === 'true') {
config.disableSend = true;
} else if (process.env.ELASTIC_APM_DISABLE_SEND === 'false') {
config.disableSend = false;
}

if (process.env.ELASTIC_APM_ENVIRONMENT || process.env.NODE_ENV) {
Expand Down Expand Up @@ -143,7 +151,12 @@ export class ApmConfiguration {
* default config.
*/
private getConfigFromKibanaConfig(): AgentConfigOptions {
return this.rawKibanaConfig?.elastic?.apm ?? {};
const config: AgentConfigOptions = this.rawKibanaConfig?.elastic?.apm ?? {};

return {
...config,
disableSend: config.disableSend ?? (config.active === true ? false : true),
};
}

/**
Expand Down
3 changes: 3 additions & 0 deletions packages/kbn-logging/src/log_record.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,7 @@ export interface LogRecord {
error?: Error;
meta?: { [name: string]: any };
pid: number;
spanId?: string;
traceId?: string;
transactionId?: string;
}

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

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

47 changes: 47 additions & 0 deletions src/core/server/logging/layouts/json_layout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ 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 @@ -310,3 +320,40 @@ 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: 'span_override',
trace: 'trace_override',
transaction: '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' },
});
});
3 changes: 3 additions & 0 deletions src/core/server/logging/layouts/json_layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export class JsonLayout implements Layout {
process: {
pid: record.pid,
},
span: record.spanId ? { id: record.spanId } : undefined,
rudolf marked this conversation as resolved.
Show resolved Hide resolved
trace: record.traceId ? { id: record.traceId } : undefined,
transaction: record.transactionId ? { id: record.transactionId } : undefined,
};
const output = record.meta ? merge({ ...record.meta }, log) : log;

Expand Down
12 changes: 11 additions & 1 deletion src/core/server/logging/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import apmAgent from 'elastic-apm-node';
import { Appender, LogLevel, LogRecord, LoggerFactory, LogMeta, Logger } from '@kbn/logging';

function isError(x: any): x is Error {
Expand Down Expand Up @@ -73,6 +73,7 @@ export class BaseLogger implements Logger {
meta,
timestamp: new Date(),
pid: process.pid,
...this.getTraceIds(),
};
}

Expand All @@ -83,6 +84,15 @@ export class BaseLogger implements Logger {
meta,
timestamp: new Date(),
pid: process.pid,
...this.getTraceIds(),
};
}

private getTraceIds() {
return {
spanId: apmAgent.currentTraceIds['span.id'],
traceId: apmAgent.currentTraceIds['trace.id'],
transactionId: apmAgent.currentTraceIds['transaction.id'],
};
}
}
5 changes: 3 additions & 2 deletions vars/kibanaPipeline.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def withFunctionalTestEnv(List additionalEnvs = [], Closure closure) {
def corsTestServerPort = "64${parallelId}3"
// needed for https://github.com/elastic/kibana/issues/107246
def proxyTestServerPort = "64${parallelId}4"
def apmActive = githubPr.isPr() ? "false" : "true"
def apmDisableSend = githubPr.isPr() ? "true" : "false"

withEnv([
"CI_GROUP=${parallelId}",
Expand All @@ -109,7 +109,8 @@ def withFunctionalTestEnv(List additionalEnvs = [], Closure closure) {
"KBN_NP_PLUGINS_BUILT=true",
"FLEET_PACKAGE_REGISTRY_PORT=${fleetPackageRegistryPort}",
"ALERTING_PROXY_PORT=${alertingProxyPort}",
"ELASTIC_APM_ACTIVE=${apmActive}",
"ELASTIC_APM_ACTIVE=true",
"ELASTIC_APM_DISABLE_SEND=${apmDisableSend}",
"ELASTIC_APM_TRANSACTION_SAMPLE_RATE=0.1",
] + additionalEnvs) {
closure()
Expand Down
Loading