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

Use log4j pattern syntax #57433

Merged
merged 13 commits into from
Feb 13, 2020
Merged

Use log4j pattern syntax #57433

merged 13 commits into from
Feb 13, 2020

Conversation

mshustov
Copy link
Contributor

@mshustov mshustov commented Feb 12, 2020

Summary

  • switch to log4j syntax {level} --> %level. Note: we support only long patterns atm (%date, not %d, since the latter is a bit cryptic)
  • rename timestamp --> date, context --> logger to follow log4j terms. But still keep meta, since map is referenced to MapMessage in log4j
  • remove file from pre-defined appenders. I don't want to introduce a default for the log file name, since it creates another maintainability problem.
  • document conversion patterns for Pattern layout
  • document how to reproduce Legacy platform setup with Logging config

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Dev docs

Logging output of the New platform plugins can use adjusted via new config.

@@ -66,21 +67,27 @@ function validateTimezone(timezone: string) {
}

function validate(rawString: string) {
for (const matched of rawString.matchAll(timestampRegExp)) {
Copy link
Contributor Author

@mshustov mshustov Feb 12, 2020

Choose a reason for hiding this comment

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

TIL matchAll is not polyfilled in runtime. that was a surprise, since it works in tests

Copy link
Contributor

Choose a reason for hiding this comment

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

What was the issue outside of the scope of tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I contacted @elastic/kibana-operations to clarify this. The test and runtime envs should be identical

Copy link
Contributor Author

@mshustov mshustov Feb 12, 2020

Choose a reason for hiding this comment

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

should be fixed in #57467

@@ -140,13 +140,6 @@ export class LoggingConfig {
layout: { kind: 'pattern', highlight: true },
} as AppenderConfigType,
],
[
'file',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed since it requires defining a file name. I don't think we should set a default for it.

@mshustov mshustov marked this pull request as ready for review February 12, 2020 14:25
@mshustov mshustov requested a review from a team as a code owner February 12, 2020 14:25
@mshustov mshustov added Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Feb 12, 2020
@elasticmachine
Copy link
Contributor

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

@mshustov mshustov added v7.7.0 release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels Feb 12, 2020
src/core/server/logging/README.md Outdated Show resolved Hide resolved
src/core/server/logging/README.md Outdated Show resolved Hide resolved
src/core/server/logging/README.md Outdated Show resolved Hide resolved
@@ -66,21 +67,27 @@ function validateTimezone(timezone: string) {
}

function validate(rawString: string) {
for (const matched of rawString.matchAll(timestampRegExp)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the issue outside of the scope of tests?

@mshustov
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@mshustov mshustov merged commit 9c1f5ae into elastic:master Feb 13, 2020
@mshustov mshustov deleted the use-log4j-syntax branch February 13, 2020 13:28
mshustov added a commit to mshustov/kibana that referenced this pull request Feb 13, 2020
* address comments

* use log4j-like syntax in layout pattern

* %timestamp --> %date to match log4j conversion pattern

* %context --> %logger to match log4j pattern

* remove file from pre-defined appenders.

file name is required. let users to setup everything

* matchAll is not polyfilled in runtime

* document available patterns and migration path

* document BWC requirements

* Revert "matchAll is not polyfilled in runtime"

This reverts commit 9f491d4.

* address comments

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
mbondyra added a commit to mbondyra/kibana that referenced this pull request Feb 13, 2020
* master: (22 commits)
  Use log4j pattern syntax (elastic#57433)
  [ML] Categorization field example endpoint tests (elastic#57471)
  [Lens] Filter out pinned filters from saved object of Lens (elastic#57197)
  Lens client side shim cleanup (elastic#56976)
  [Maps] do not show border color for icon in legend when border width is zero (elastic#57501)
  refactors 'data-providers' tests (elastic#57474)
  add `absolute` option to `getUrlForApp` (elastic#57193)
  [Telemetry] Migrate public to NP (elastic#56285)
  address flaky test where instances might have different start… (elastic#57506)
  fix(NA): support legacy plugins path in plugins (elastic#57472)
  build immutable bundles for new platform plugins (elastic#53976)
  [SIEM] [Detection Engine] Reject if duplicate rule_id in request payload (elastic#57057)
  Add autocomplete="off" for input type="password" to appease the scanners (elastic#56922)
  Use default spaces suffix for signals index if spaces disabled (elastic#57244)
  [Alerting] Create alert design cleanup (elastic#56929)
  Management Api - add to migration guide (elastic#56892)
  fixing maps (elastic#56706)
  [Maps] Autocomplete for custom color palettes and custom icon palettes (elastic#56446)
  [Alerting] make actionGroup name's i18n-able (elastic#57404)
  fixed flaky test (elastic#57490)
  ...

# Conflicts:
#	src/legacy/core_plugins/telemetry/public/components/__snapshots__/telemetry_form.test.js.snap
#	src/plugins/telemetry/public/components/telemetry_management_section.tsx
mshustov added a commit that referenced this pull request Feb 13, 2020
* address comments

* use log4j-like syntax in layout pattern

* %timestamp --> %date to match log4j conversion pattern

* %context --> %logger to match log4j pattern

* remove file from pre-defined appenders.

file name is required. let users to setup everything

* matchAll is not polyfilled in runtime

* document available patterns and migration path

* document BWC requirements

* Revert "matchAll is not polyfilled in runtime"

This reverts commit 9f491d4.

* address comments

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 13, 2020
* master: (22 commits)
  skip flaky suite (elastic#50018)
  skip settings tests (elastic#57608)
  skip failing suite (elastic#44631)
  [SIEM] [Case] Initial UI (elastic#57283)
  handle viewing sample dashboards on default dist (elastic#57510)
  Fix detection of "system requests" in plugins (elastic#57149)
  [ML] New Platform server shim: update job service schema (elastic#57614)
  skip flaky suite (elastic#44631)
  [APM] Update monospace font family variable (elastic#57555)
  skip flaky test (elastic#57377)
  Skip save query tests (elastic#57589)
  [Maps] allow simultaneous opening of multiple tooltips (elastic#57226)
  [Uptime] Fix/host connected components (elastic#56969)
  [logs][metrics][docs] Update screenshots for 7.6 (elastic#57254)
  [ML] New Platform server shim: update job service routes to use new platform router (elastic#57403)
  [Maps] Fix document source top hits split by scripted field (elastic#57481)
  Use log4j pattern syntax (elastic#57433)
  [ML] Categorization field example endpoint tests (elastic#57471)
  [Lens] Filter out pinned filters from saved object of Lens (elastic#57197)
  Lens client side shim cleanup (elastic#56976)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants