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

Log deprecations originating from Kibana on debug level #123660

Merged
merged 3 commits into from
Jan 25, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -540,14 +540,13 @@ describe('instrumentQueryAndDeprecationLogger', () => {
});
client.diagnostic.emit('response', new errors.ResponseError(response), response);

// One debug log entry from 'elasticsearch.query' context
expect(loggingSystemMock.collect(logger).debug.length).toEqual(1);
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch(
// Test debug[1] since theree is one log entry from 'elasticsearch.query' context
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch(
'Elasticsearch deprecation: 299 Elasticsearch-8.1.0 "GET /_path is deprecated"'
);
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch('Origin:kibana');
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch(/Stack trace:\n.*at/);
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch(
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch('Origin:kibana');
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch(/Stack trace:\n.*at/);
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch(
/Query:\n.*400\n.*GET \/_path\?hello\=dolly \[illegal_argument_exception\]: request \[\/_path\] contains unrecognized parameter: \[name\]/
);
});
Expand All @@ -573,7 +572,6 @@ describe('instrumentQueryAndDeprecationLogger', () => {
});
client.diagnostic.emit('response', null, response);

expect(loggingSystemMock.collect(logger).info).toEqual([]);
// Test debug[1] since theree is one log entry from 'elasticsearch.query' context
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch(
'Elasticsearch deprecation: 299 Elasticsearch-8.1.0 "GET /_path is deprecated"'
Expand Down Expand Up @@ -608,14 +606,13 @@ describe('instrumentQueryAndDeprecationLogger', () => {
});
client.diagnostic.emit('response', null, response);

// One debug log entry from 'elasticsearch.query' context
expect(loggingSystemMock.collect(logger).debug.length).toEqual(1);
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch(
// Test debug[1] since theree is one log entry from 'elasticsearch.query' context
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch(
'Elasticsearch deprecation: 299 Elasticsearch-8.1.0 "GET /_path is deprecated"'
);
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch('Origin:kibana');
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch(/Stack trace:\n.*at/);
expect(loggingSystemMock.collect(logger).info[0][0]).toMatch(
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch('Origin:kibana');
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch(/Stack trace:\n.*at/);
expect(loggingSystemMock.collect(logger).debug[1][0]).toMatch(
/Query:\n.*200\n.*GET \/_path\?hello\=dolly/
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,9 @@ export const instrumentEsQueryAndDeprecationLogger = ({
// Strip the first 5 stack trace lines as these are irrelavent to finding the call site
const stackTrace = new Error().stack?.split('\n').slice(5).join('\n');

const deprecationMsg = `Elasticsearch deprecation: ${event.warnings}\nOrigin:${requestOrigin}\nStack trace:\n${stackTrace}\nQuery:\n${queryMsg}`;
if (requestOrigin === 'kibana') {
deprecationLogger.info(deprecationMsg);
} else {
deprecationLogger.debug(deprecationMsg);
}
deprecationLogger.debug(
`Elasticsearch deprecation: ${event.warnings}\nOrigin:${requestOrigin}\nStack trace:\n${stackTrace}\nQuery:\n${queryMsg}`
);
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ describe('migration from 7.7.2-xpack with 100k objects', () => {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ function createRoot() {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ function createRoot() {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ function createRoot() {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ function createRoot(options: { maxBatchSizeBytes?: number }) {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ function createRoot(options: { maxBatchSizeBytes?: number }) {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ function createRoot() {
{
name: 'root',
appenders: ['file'],
level: 'info',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ function createRoot() {
{
name: 'root',
appenders: ['file'],
level: 'info',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ describe('migrating from 7.3.0-xpack which used v1 migrations', () => {
{
name: 'root',
appenders: ['file'],
level: 'info',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ describe('migrating from the same Kibana version that used v1 migrations', () =>
{
name: 'root',
appenders: ['file'],
level: 'info',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ function createRoot({ logFileName, hosts }: RootConfig) {
{
name: 'root',
appenders: ['file'],
level: 'info',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ async function createRoot({ logFileName }: CreateRootConfig) {
{
name: 'root',
appenders: ['file'],
level: 'info',
},
{
name: 'savedobjects-service',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ function createRoot() {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ function createRoot() {
loggers: [
{
name: 'root',
level: 'info',
appenders: ['file'],
},
],
Expand Down
19 changes: 19 additions & 0 deletions src/core/test_helpers/kbn_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,25 @@ export function createRootWithCorePlugins(settings = {}, cliArgs: Partial<CliArg
username: kibanaServerTestUser.username,
password: kibanaServerTestUser.password,
},
// Log ES deprecations to surface these in CI
logging: {
loggers: [
{
name: 'root',
level: 'error',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

before jest integration tests wouldn't log anything to the console, but having at least error logs is useful for failing/flaky tests

appenders: ['console'],
},
{
name: 'elasticsearch.deprecation',
level: 'all',
appenders: ['deprecation'],
},
],
appenders: {
deprecation: { type: 'console', layout: { type: 'json' } },
console: { type: 'console', layout: { type: 'pattern' } },
Comment on lines +125 to +126
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it going to be an issue to have both json and pattern messages output on stdout?

},
},
// createRootWithSettings sets default value to "true", so undefined should be threatened as "true".
...(cliArgs.oss === false
? {
Expand Down
1 change: 1 addition & 0 deletions test/common/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export default function () {
'--logging.appenders.deprecation.type=console',
'--logging.appenders.deprecation.layout.type=json',
'--logging.loggers[0].name=elasticsearch.deprecation',
'--logging.loggers[0].level=all',
'--logging.loggers[0].appenders[0]=deprecation',
],
},
Expand Down