Skip to content

Keep unparsed user agent information in user_agent.original#8537

Merged
kvch merged 5 commits intoelastic:masterfrom
kvch:rename-user-agent-raw-to-original
Oct 5, 2018
Merged

Keep unparsed user agent information in user_agent.original#8537
kvch merged 5 commits intoelastic:masterfrom
kvch:rename-user-agent-raw-to-original

Conversation

@kvch
Copy link
Copy Markdown
Contributor

@kvch kvch commented Oct 2, 2018

user_agent.raw has been renamed to user_agent.original. As this field is not yet released I am renaming it to follow conventions.

@kvch kvch requested a review from jsoriano October 2, 2018 10:00
@kvch kvch added review Filebeat Filebeat labels Oct 2, 2018
@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Oct 2, 2018

Change LGTM. I think it will need a Changelog entry.

I have second thoughts if we should backport this to 6.x or not (not only raw to original, but the overall change).

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Oct 3, 2018

Why? I think it can provide useful information is case of exotic user agents. I assume when someone investigates weird events happening in his/her network, it's possible that the person who might be lurking around leaves behind "unconventional" user agents.

@kvch kvch removed the request for review from jsoriano October 3, 2018 09:47
@kvch kvch force-pushed the rename-user-agent-raw-to-original branch from bc1e9e9 to 3b450ed Compare October 3, 2018 09:49
@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Oct 3, 2018

Added changelog entry && rebased the branch

- Add tag "multiline" to "log.flags" if event consists of multiple lines. {pull}7997[7997]
- Add haproxy module. {pull}8014[8014]
- Release `docker` input as GA. {pull}8328[8328]
- Rename user_agent.raw to user_ageint.original to follow ECS conventions. {pull}8537[8537]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this hasn't been released yet, what about editing the previous entry instead of adding two for the same thing?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

++

@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Oct 3, 2018

@kvch Few thoughts around this that recently came up:

  • Should we actually index user_agent.original? Is it mean to be searchable or more for reprocessing or looking into it when an event is found for the user (it will still be part of _source). This could save quite a bit of space on indexing
  • The user_agent field will be a common field in 7.0 because of ECS. So the local fields will probably disappear. Should we add more fields now or wait for 7.0 until it's unified?

@kvch kvch changed the title Rename user_agent.raw to user_agent.original to follow ECS conventions Keep unparsed user agent information in user_agent.original Oct 3, 2018
@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Oct 3, 2018

  • +1 for not indexing it
  • I would wait for 7.0. I have already renamed a few fields, because ECS is still a work in progress. I would rather minimize this overhead. If unavoidable, I would do it in a bulk before 7.0 is released.

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Oct 4, 2018

@ruflin are you ok with merging this as is?

The name of the operating system.
- name: raw
- name: original
type: text
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As we set index: false it should not matter here what type is defined. In ECS it seems we put keyword. Only reason I mention this is we should check later what shows up in the docs for non indexed fields.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got it. Thanks for the info.

@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Oct 5, 2018

@kvch LGTM. Did an additional commit to resolve a CHANGELOG conflict.

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Oct 5, 2018

Failing tests are unrelated.

@kvch kvch merged commit f3e0801 into elastic:master Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants