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

Rename log.logger to event.dataset as recommended in ECS #2534

Closed
wants to merge 1 commit into from

Conversation

pebrc
Copy link
Collaborator

@pebrc pebrc commented Feb 7, 2020

Instead of

Screenshot 2020-02-07 at 11 16 57

we get

Screenshot 2020-02-07 at 11 15 20

in Kibanas log UI by following the ECS recommendation for event.dataset

@pebrc pebrc added >enhancement Enhancement of existing functionality v1.1.0 labels Feb 7, 2020
thbkrkr
thbkrkr previously approved these changes Feb 7, 2020
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

Good ratio number of lines of code modified versus added value :)

@thbkrkr thbkrkr dismissed their stale review February 7, 2020 10:40

I went a little fast.

@thbkrkr
Copy link
Contributor

thbkrkr commented Feb 7, 2020

I'm a little confused by the amount of fields that can be used to describe the source of events and when one should be used more than another (elastic/ecs#281 (comment)).

But this change doesn't look dangerous. I admit that I trust you on this one with slightly closed eyes.

How is it ok to not set log.logger?

@pebrc
Copy link
Collaborator Author

pebrc commented Feb 7, 2020

I have second thoughts now that I see that ECS seems to revise its position on where to put the logger in the discussion you linked to @thbkrkr

@barkbay
Copy link
Contributor

barkbay commented Feb 7, 2020

I didn't set this field for this reason and created an issue to discuss improvements re. ECS : #2467 (comment)

@pebrc
Copy link
Collaborator Author

pebrc commented Feb 7, 2020

Let's not do this for now.

@pebrc pebrc closed this Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v1.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants