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

Deprecate legacy logging configuration in favour of new logging system #82431

Closed
mshustov opened this issue Nov 3, 2020 · 11 comments · Fixed by #94238
Closed

Deprecate legacy logging configuration in favour of new logging system #82431

mshustov opened this issue Nov 3, 2020 · 11 comments · Fixed by #94238
Assignees
Labels
Breaking Change Feature:Logging Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0

Comments

@mshustov
Copy link
Contributor

mshustov commented Nov 3, 2020

The new logging system uses a different configuration model https://github.com/elastic/kibana/blob/master/src/core/server/logging/README.mdx which is already available to the end-users but not documented nor marked as experimental. Several blockers need to be addressed #58261 before we consider the new logging system to have feature parity with the legacy version to start deprecating the legacy config.

This change affects different stakeholders inside and outside of Elastic:

  • Cloud team have to update their logging infrastructure to switch to the new config format
  • Support team have to update their recommendations on collecting logs
  • external consumers have to adopt configuration files to use the new logging config
  • the logging format is subject to minor changes (needs another investigation)
  • Filebeat's Kibana module needs to update their ingest pipeline to pull from the correct LogRecord fields: [filebeat] Upgrade Kibana module to prepare for 8.0 breaking changes beats#24136

We also need to be sure to remove the elasticsearch.logQueries configuration

@mshustov mshustov added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 Feature:Logging Breaking Change labels Nov 3, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@mshustov
Copy link
Contributor Author

mshustov commented Mar 3, 2021

There are a few cases left when LP logs a message. These messages aren't affected by the Kibana platform logging config, which might be confusing from the user's point of view. I'd suggest migrating these leftovers to the KP if possible.

in /packages folder

@TinaHeiligers
Copy link
Contributor

TinaHeiligers commented Mar 3, 2021

I'd suggest migrating these leftovers to the KP if possible.

@restrry ack, I'll circle back with you about these when I start working on this issue.
To be clear, we won't be backporting the changes to 7.13. Am I correct?

@mshustov
Copy link
Contributor Author

mshustov commented Mar 4, 2021

To be clear, we won't be backporting the changes to 7.13. Am I correct?

No, but maybe we can migration some of them in 7.13

@TinaHeiligers
Copy link
Contributor

TinaHeiligers commented Mar 5, 2021

I'd suggest migrating these leftovers to the KP if possible

@restrry I'm not sure what the migration involves. Should we migrate the functionality of what these legacy logs are logging to the KP or just use the KP logger to log the messages from the LP?

I'm working under the assumption that we want the logs to respond to the new logging system config (replace the legacy logger with a KP logger instance under a similar/same context).

Second question: It's also not clear to me how to handle the logs in kbn-legacy-logging. More specifically, how to inject the KP logger from core into the package?

@mshustov
Copy link
Contributor Author

mshustov commented Mar 9, 2021

I'm not sure what the migration involves. Should we migrate the functionality of what these legacy logs are logging to the KP or just use the KP logger to log the messages from the LP?

We should migrate the code calling the legacy logger to the Kibana Platform or delete it. If it's not possible (like in the case of kbn-legacy-logging), well... nothing to do here 🤷‍♀️

@TinaHeiligers
Copy link
Contributor

@restrry re:

this.server.log(['info', 'config'], 'New logging configuration:\n' + plain);
might be unnecessary if the KP logs a similar message

We do log a similar message from KP:

function reloadLoggingConfig() {
const cliLogger = root.logger.get('cli');
cliLogger.info('Reloading logging configuration due to SIGHUP.', { tags: ['config'] });
try {
rawConfigService.reloadConfig();
} catch (err) {
return shutdown(err);
}
cliLogger.info('Reloaded logging configuration due to SIGHUP.', { tags: ['config'] });
}

The only difference is that the legacy logger outputs the content of the new LP logging config but we're not doing that in core.

If we did want to log the new logging-specific config changes from KP, we'd probably need to move or duplicate that logging logic to the logging system and add the new config as log meta. That way we can keep the log message fairly short in pattern format and only show the details when logs are in json. Is this what you had in mind?

@mshustov
Copy link
Contributor Author

mshustov commented Mar 10, 2021

The only difference is that the legacy logger outputs the content of the new LP logging config

I don't think it's necessary. I would even say undesirable: it might create a situation when Kibana logs sensitive config values.

@TinaHeiligers
Copy link
Contributor

The kbn:watch yarn command in Kibana's package.json uses --logging.json=false. We'll need to update that script too or at least handle the flag.

@pgayvallet
Copy link
Contributor

We'll need to update that script too or at least handle the flag.

let's just remove the flag from the script imho. This is a dev only command, after all.

@TinaHeiligers
Copy link
Contributor

@spalger I'm nervous about editing Kibana's package.json file to remove the --logging.json=false flag from the kbn:watch script. Should I just go ahead and do that or do you think it's best if the operations team handles the change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Feature:Logging Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0
Projects
5 participants