[core.logging] Ensure LogMeta is ECS-compliant. by lukeelmers · Pull Request #96350 · elastic/kibana

lukeelmers · 2021-04-06T19:45:25Z

One goal of our new logging system is for the logs to be ECS-compliant (when using JSON layout). The one part of a log record that is outside of core's control is the LogMeta, where developers can put pretty much any object they want and have it added to the logs.

While this provides nice flexibility, it poses a challenge when it comes to adhering to ECS formats: since we merge any LogMeta into the actual log record itself, it means that folks would ideally place their meta into a known ECS field (if one exists for their data), or otherwise choose a custom field to use.

In looking more closely at #90406, we decided that we would create a full set of ECS typings in core to use for promoting ECS-friendly LogMeta.

This PR makes a few different changes to help with this effort:

Adds full set of ECS typings (v1.9) to @kbn/logging, and also exports them from core.
Updates LogMeta types to ensure they adhere to ECS types.
Adds generic type params to Logger so that folks can pass their own custom types which extend from thee base LogMeta/ECS types.
Updates downstream uses of LogMeta in core/plugins.
Makes a few changes to security's audit logging, which already had its own ECS implementation that we can now align with core's.

Plugin API Changes

Core's logging system has been updated to ensure that logs using a JSON layout are compliant with ECS. If you are using the optional LogMeta param in your plugin, you should check the ECS spec and ensure your meta conforms to ECS wherever possible.

To assist with this, we've updated the LogMeta TypeScript interface to require ECS-friendly data. Should you need to log fields that do not fit within the ECS spec, you can provide a generic type parameter which accepts an interface that extends from the base LogMeta:

// plugins/my-plugin/server/plugin.ts

import type { CoreSetup, LogMeta, Plugin, PluginInitializerContext } from 'src/core/server';

interface MyPluginLogMeta extends LogMeta {
  kibana: { myCustomField: string };
}

export class MyPlugin implements Plugin  {
  constructor(private readonly initContext: PluginInitializerContext) {
    this.logger = initContext.logger.get();
  }

  setup(core: CoreSetup) {
    this.logger.warn<MyPluginLogMeta>('my log with custom meta', {
      kibana: {
        myCustomField: 'heya',
      }
    });
  }
}

packages/kbn-logging/src/logger.ts

lukeelmers · 2021-04-12T04:55:42Z

src/core/server/index.ts

I ended up exporting these from core because audit logging will use some of them.

src/core/server/logging/layouts/json_layout.ts

lukeelmers · 2021-04-12T05:15:03Z

src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.ts

Spent awhile battling TS on this (and a similar situation in the actions plugin).

The issue is that dynamically calling a method from Logger results in an This expression is not callable error due to each member of the union type having signatures which are incompatible (similar to this issue).

Specifically, it seems that when you are mixing methods which could contain string | Error in the message (warn, error, fatal) with ones that only contain string (trace, debug, info), TS freaks out.

But interestingly, this only surfaced when I added the generic type params to the Logger interface... removing them resolves the issue.

Still not sure what's up with that, but would appreciate any insights folks may have. In the meantime, I resorted to casting in both places 😞

packages/kbn-logging/src/ecs/event.ts

lukeelmers · 2021-04-12T05:30:58Z

Pinging @mshustov @thomheymann for any initial thoughts on the approach here before I take it out of draft and ping a bunch of people 🙂

I added a few comments for discussion.

thomheymann

Nice one, looks great Luke!

One suggestion regarding quarantined fields below.

src/core/server/logging/layouts/json_layout.ts

packages/kbn-logging/src/logger.ts

packages/kbn-logging/src/ecs/agent.ts

src/core/server/logging/layouts/json_layout.ts

packages/kbn-logging/src/logger.ts

mshustov · 2021-04-13T16:52:02Z

packages/kbn-logging/src/logger.ts

-export interface LogMeta {
-  [key: string]: any;
-}
+export type LogMeta = Partial<Omit<Ecs, 'ecs'>>;


what about avoiding Omit and Pick in the public interfaces? Let's a public interface with ecs field internally

the current error message :

Let's a public interface with ecs field internally

The only issue I can see with this is a maintenance one -- if we update versions of ECS (or if someday the types are auto generated), then we will need to manually update the top-level keys in LogMeta as well. However, since top-level keys don't change all that much, it probably wouldn't hurt to duplicate them for now, and reconsider later if it turns out to be a problem in practice. Will update.

lukeelmers · 2021-04-14T21:42:01Z

x-pack/plugins/actions/server/lib/audit_events.ts

+      category: ['database'],
+      type: type ? [type] : undefined,


I tested the Kibana filebeat module and confirmed that converting this to an array, while correct from an ECS standpoint, will still break existing filters, etc:

I'm working on an update to the module to correct this.

Draft PR for the filebeat module: elastic/beats#25101

(@legrego @thomheymann would appreciate any expertise you could offer on that) ❤️

Thanks for testing how this gets displayed. I think we might need to check with the Observability team if they support arrays for event.type and event.category and how that should be rendered.

thomheymann

Looks great, really happy with the improvements!

Tested locally and audit logging is working as expected.

Got two questions. One below regarding ignore filters and the other one in the link regarding backwards compatibility: https://github.com/elastic/beats/pull/25101/files#r613936133

src/core/server/logging/layouts/json_layout.ts

x-pack/plugins/security/server/audit/audit_service.ts

src/core/server/logging/layouts/json_layout.ts

lukeelmers · 2021-04-15T20:24:59Z

@elasticmachine merge upstream

pmuellr

alerting changes LGTM

thomheymann

Audit logging changed LGTM! 👍

lukeelmers · 2021-04-20T13:29:19Z

@elasticmachine merge upstream

kibanamachine · 2021-04-20T15:27:40Z

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

API count

id	before	after	diff
`core`	2174	2178	+4
`security`	79	71	-8
total			-4

API count missing comments

id	before	after	diff
`core`	992	995	+3
`security`	38	31	-7
total			-4

API count with any type

id	before	after	diff
`core`	147	146	-1

History

💚 Build #120159 succeeded 28ef538
💔 Build #120131 failed ce675d2
💚 Build #119866 succeeded 7133d50
💔 Build #119782 failed 78e631a
💔 Build #119771 failed 63e8c10

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

cc @lukeelmers

kibanamachine · 2021-04-20T15:34:07Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

Co-authored-by: Luke Elmers <luke.elmers@elastic.co>

lukeelmers added WIP Work in progress v8.0.0 Feature:Logging v7.13.0 labels Apr 6, 2021

lukeelmers self-assigned this Apr 6, 2021

lukeelmers force-pushed the feat/ecs-logs branch 2 times, most recently from e75649e to 371d043 Compare April 12, 2021 04:50

lukeelmers commented Apr 12, 2021

View reviewed changes

lukeelmers force-pushed the feat/ecs-logs branch from 371d043 to 7cd8181 Compare April 12, 2021 05:28

lukeelmers force-pushed the feat/ecs-logs branch from 7cd8181 to 19f202b Compare April 12, 2021 15:02

lukeelmers added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. and removed WIP Work in progress labels Apr 12, 2021

lukeelmers added 7 commits April 12, 2021 15:57

Add initial types for ECS.

3aba9b2

Expose ECS types & enforce LogMeta to conform to ECS.

9f92dbe

Update core to conform to ECS LogMeta.

ff87325

Update security plugin to conform to ECS LogMeta.

9448244

Update actions plugin to conform to ECS LogMeta.

3d5ba7b

Update alerting plugin to conform to ECS LogMeta.

2f9e622

Update generated docs.

2d63e5b

lukeelmers force-pushed the feat/ecs-logs branch from 19f202b to 2d63e5b Compare April 12, 2021 22:01

thomheymann reviewed Apr 13, 2021

View reviewed changes

src/core/server/logging/layouts/json_layout.ts Outdated Show resolved Hide resolved

packages/kbn-logging/src/logger.ts Outdated Show resolved Hide resolved

mshustov reviewed Apr 13, 2021

View reviewed changes

lukeelmers added 2 commits April 13, 2021 23:15

Address initial feedback.

d2810ea

Update generated docs.

2702ee6

lukeelmers added the Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// label Apr 14, 2021

lukeelmers marked this pull request as ready for review April 14, 2021 05:21

lukeelmers requested review from a team as code owners April 14, 2021 05:21

lukeelmers added 3 commits April 14, 2021 13:54

Fix audit logging type errors.

b878341

Merge branch 'master' into feat/ecs-logs

63e8c10

Fix usage counters type error.

78e631a

lukeelmers commented Apr 14, 2021

View reviewed changes

lukeelmers mentioned this pull request Apr 15, 2021

[Filebeat] Ensure Kibana audit event.category and event.type are still processed as strings. elastic/beats#25101

Merged

3 tasks

Fix jest tests.

7133d50

thomheymann reviewed Apr 15, 2021

View reviewed changes

src/core/server/logging/layouts/json_layout.ts Outdated Show resolved Hide resolved

x-pack/plugins/security/server/audit/audit_service.ts Show resolved Hide resolved

mshustov approved these changes Apr 15, 2021

View reviewed changes

src/core/server/logging/layouts/json_layout.ts Outdated Show resolved Hide resolved

lukeelmers added 2 commits April 15, 2021 12:34

Core's logger values always overwrite LogMeta.

fb3e7eb

Clarify behavior of filterEvent and add tests.

ce675d2

lukeelmers requested a review from thomheymann April 15, 2021 19:09

Merge branch 'master' into feat/ecs-logs

28ef538

pmuellr approved these changes Apr 15, 2021

View reviewed changes

thomheymann approved these changes Apr 20, 2021

View reviewed changes

Merge branch 'master' into feat/ecs-logs

a0a2bad

lukeelmers added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 20, 2021

lukeelmers merged commit 12b245c into elastic:master Apr 20, 2021

lukeelmers deleted the feat/ecs-logs branch April 20, 2021 15:31

kibanamachine mentioned this pull request Apr 20, 2021

[7.x] [core.logging] Ensure LogMeta is ECS-compliant. (#96350) #97660

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 20, 2021

[core.logging] Ensure LogMeta is ECS-compliant. (elastic#96350)

3b649de

kibanamachine added a commit that referenced this pull request Apr 20, 2021

[core.logging] Ensure LogMeta is ECS-compliant. (#96350) (#97660)

4270e6d

Co-authored-by: Luke Elmers <luke.elmers@elastic.co>

pgayvallet mentioned this pull request Apr 27, 2021

Kibana crashed fatally with Unable to complete saved object migrations for the [.kibana_task_manager] index. TypeError: Cannot read property 'logger' of undefined #98406

Closed

pmuellr mentioned this pull request Feb 7, 2022

[Reporting] Remove EventLog Dependency #124762

Merged

This was referenced Sep 29, 2022

Kibana ingest pipeline overwrites root fields set in Filebeat config elastic/beats#32665

Closed

[filebeat] Fix ingest pipeline overwriting module field values elastic/beats#33236

Merged

Conversation

lukeelmers commented Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Plugin API Changes

Uh oh!

Uh oh!

lukeelmers Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lukeelmers Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lukeelmers commented Apr 12, 2021

Uh oh!

thomheymann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mshustov Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

lukeelmers Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

lukeelmers Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

lukeelmers Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

thomheymann Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

thomheymann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukeelmers commented Apr 15, 2021

Uh oh!

pmuellr left a comment

Choose a reason for hiding this comment

Uh oh!

thomheymann left a comment

Choose a reason for hiding this comment

Uh oh!

lukeelmers commented Apr 20, 2021

Uh oh!

kibanamachine commented Apr 20, 2021

💚 Build Succeeded

Metrics [docs]

API count

API count missing comments

API count with any type

History

Uh oh!

kibanamachine commented Apr 20, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lukeelmers commented Apr 6, 2021 •

edited

Loading