Fix filebeat elasticsearch module ingest timezone

[pragkent]

This pull request fixes timezone parsing for elasticsearch module.

Just like #13308 fixes ingest timezone parsing for system module.

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[jsoriano]

ok to test

I think it doesn't fix the issue for plain text logs :(

[jsoriano]

@pragkent I was testing this branch with some of the test files we have and I have seen that for this log line:

[2018-05-17T08:19:35,939][INFO ][o.e.n.Node               ] [] initializing ...

The timestamp with timezone Europe/Berlin is parsed as:

"@timestamp": "2018-05-17T10:19:35.939+02:00"

But it should be:

"@timestamp": "2018-05-17T08:19:35.939+02:00"

I think that the same happens for any log without timestamp.

Something we could evaluate is to have different date processors for json and plain text files, as there are different pipelines for any of them.

[pragkent]

I'm confused about the behavior of the date processor. I checked the source code and found it will use the last pattern that matches the time string instead of the first one. This is the reason why the timezone is messed up. The json log works fine just because timezone is included in the log line.

If we remove the ISO8601 format from the second date processor, then the timezone will be handled correctly.

ES DateProcessor

[jsoriano]

If we remove the ISO8601 format from the second date processor, then the timezone will be handled correctly.

This can be an option, but then I guess that we should ignore errors in this processor because timestamps parsed in the previous date processor as ISO8601 will fail here.

In any case as the data format in JSON and plain text logs are different, what do you think about moving date parsing to the specific pipelines, pipeline-json.json and pipeline-plaintext.json?

[pragkent]

yes, I think you are right.
Moving date parsing to specific pipelines could make it better.
Let me try to move it around, and let's see the result then.

[pragkent]

I've moved these out to specific pipeline. When the log is in JSON format, timezone is already included, so I deleted event.timezone related date processor in json pipeline.

Please have a look.

[jsoriano]

Thanks!

[ycombinator]

Should this line be removed from this PR?

[jsoriano]

Oops, my fault, I did the merge, let me fix it.

[jsoriano]

Oh I see, I wanted to move the changelog entry 🙂

[ycombinator]

Now that this processor and the next one are both acting on the same field, perhaps we could add a "if": "ctx.event.timezone == null" property to this processor for a bit of extra clarity?

[pragkent]

I think it's better to not add "if": "ctx.event.timezone == null", because:

1. other modules don't have "if": "ctx.event.timezone == null" added, we'd better keep this convention
2. in most cases, event.timezone was not set. without "if": "ctx.event.timezone == null" would make the second processor looks like a optional path instead of a alternative path

[jsoriano]

I think @ycombinator is right, we could add this if, as only one of the processors is needed.

• other modules don't have "if": "ctx.event.timezone == null" added, we'd better keep this convention

In previous versions both processors were meant to be executed, one to parse the date, and the next one to apply the timezone. We (well, you @pragkent 🙂 ) found this approach is not correct in many cases, so now we are duplicating the date processor, first option only parses the date, and second option parses the date with a timezone if available.

• in most cases, event.timezone was not set. without "if": "ctx.event.timezone == null" would make the second processor looks like a optional path instead of a alternative path

Actually we are setting event.timezone by default since ~7.2 in all filebeat events, so in most cases both date processors are executed even if only the last one is needed.

Said that, as we had it quite tested by both @pragkent and me, and it solves an existing issue, I'd go on with merging this change as is, and have a follow up PR to review the conditions in the pipelines we have recently changed to fix this same issue. This way we keep a common convention for this.

[ycombinator]

LGTM.

[lucabelluccini]

Hello,
The Audit JSON content is not ISO8601, so needs the same extra treatment like the plaintext.
E.g.

{"@timestamp":"2019-09-05T14:02:37,921", "node.id":"UwRu4mReRtyJO1-FWAPvIQ", "event.type":"transport", "event.action":"authentication_success", "user.name":"_system", "origin.type":"local_node", "origin.address":"127.0.0.1:9300", "realm":"__fallback", "request.id":"474ZciqtQteOhjLO3OdZIw", "action":"indices:monitor/stats", "request.name":"IndicesStatsRequest"}