x-pack/filebeat/input/httpjson: Fix parseDate location offset

[chemamartinez]

Proposed commit message

We encountered an issue from an SDH due to how the time.Parse function in Go handles timezone abbreviations. It interprets them based on the server's local time zone, which can lead to incorrect conversions if the server's local time zone doesn't match the time zone abbreviation in the string.

Since this is a limitation of the time.Parse function, we decided to add a new helper parseDateInTZ where users can apply the proper timezone when parsing dates. It accepts numeric offsets and IANA time zone names.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @chemamartinez? 🙏.
For such, you'll need to label your PR with:

• The upcoming major version of the Elastic Stack
• The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b fix-httpjson-parseDate-timezone upstream/fix-httpjson-parseDate-timezone
git merge upstream/main
git push upstream fix-httpjson-parseDate-timezone

[elasticmachine]

❕ Build Aborted

There is a new build on-going so the previous on-going builds have been aborted.

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2024-01-25T08:52:18.873+0000

• Duration: 60 min 13 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2819
Skipped	176
Total	2995

Steps errors

Expand to view the steps failures

Error signal

• Took 0 min 0 sec . View more details here
• Description: Error 'org.jenkinsci.plugins.workflow.steps.FlowInterruptedException'

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /package : Generate the packages and run the E2E tests.

• /beats-tester : Run the installation tests with beats-tester.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2024-02-20T18:23:51.082+0000

• Duration: 136 min 3 sec

Test stats 🧪

Test	Results
Failed	0
Passed	3320
Skipped	180
Total	3500

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /package : Generate the packages and run the E2E tests.

• /beats-tester : Run the installation tests with beats-tester.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[efd6]

I'm not convinced that this is the right solution. If we cannot parse the timezone because it is incorrectly using a non-IANA tzdb label or absolute fixed timezone, it should be set in the UI config.

[chemamartinez]

I'm not convinced that this is the right solution. If we cannot parse the timezone because it is incorrectly using a non-IANA tzdb label or absolute fixed timezone, it should be set in the UI config.

@efd6 thanks for taking a look!

Since this is fixing a value template, that parses dates that can come from anywhere at any timezone, how would we set the timezone in the UI config? Do you mean forcing users that use this value template to define what's the timezone of the incoming dates?

On the other hand, do you see any particular issue that makes this change undesirable? I agree this may not be the optimal solution, even the Go developers seem to have been debating the issue for quite some time:
golang/go#56528
golang/go#63345
For now, if no native solution is provided, the time.Parse docs suggests using ParseInLocation when facing this kind of issues, and it works at least for the most common cases.

Please let me know your thoughts about this. Thanks!

[efd6]

The commentary in go.dev/issue/63345 goes over the issues with this very nicely and explains exactly why this is undesirable. The caveat in the final paragraph of time.Parse is also informative about why this is a dangerous action to take (briefly, if the agent is running in a location that does not know the abbreviation, the tzoffset will be zeroed, or worse if it is in another location where the abbreviation means something else it will be definitively incorrect).

In practice any particular instance of HTTPJSON would likely only be collecting data from a single tz, so if this is not parseable, a single configuration location would be enough to specify that time be handled in that singular location. Maybe it would be helpful to have a parseTimeInLocation template helper.

If this change here is added, it needs documentation making the sharp edges clear and suggesting that users configure time-offset timezone specs if possible.

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b fix-httpjson-parseDate-timezone upstream/fix-httpjson-parseDate-timezone
git merge upstream/main
git push upstream fix-httpjson-parseDate-timezone

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b fix-httpjson-parseDate-timezone upstream/fix-httpjson-parseDate-timezone
git merge upstream/main
git push upstream fix-httpjson-parseDate-timezone

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: b767c90

cc @chemamartinez

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: b767c90

cc @chemamartinez

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: b767c90

cc @chemamartinez

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: b767c90

cc @chemamartinez

[chemamartinez]

In practice any particular instance of HTTPJSON would likely only be collecting data from a single tz, so if this is not parseable, a single configuration location would be enough to specify that time be handled in that singular location. Maybe it would be helpful to have a parseTimeInLocation template helper.

If this change here is added, it needs documentation making the sharp edges clear and suggesting that users configure time-offset timezone specs if possible.

@efd6 I added a new value template parseDateInTZ and updated the docs intending to clarify edge cases and this timezone issue. Regarding the implementation of parseDateInTZ, I had two different approaches depending on what we would prefer:

1. Using time.ParseInLocation gives preference to the implicit timezone in the provided date over the specified timezone.
2. Using time.Parse after a manual assignment of the location. That way we prioritize the given one.

I chose the last option, let me know if you have second thoughts on this.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: b767c90

cc @chemamartinez

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: b767c90

cc @chemamartinez

[efd6]

I chose the last option

I agree.