[filebeat][httpjson]- Added min & max functions to the template engine

[ShourieG]

Type of change

• Enhancement

What does this PR do?

Adds min, max methods to the template engine.

Why is it important?

This is important because it will help solve issues where min max methods are required to extract a certain value to be passed in the requests.

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.

Author's Checklist

• [ ]

How to test this PR locally

Related issues

• Relates [Cloudflare Logpull][Meta] invalid time range: too early: logs older than 168h0m0s are not available integrations#4414
• Closes [filebeat][httpjson] - Add min/max template functions #36094

Use cases

Screenshots

Logs

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @ShourieG? 🙏.
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

[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: 2023-07-24T07:04:29.916+0000

• Duration: 73 min 9 sec

Test stats 🧪

Test	Results
Failed	0
Passed	3076
Skipped	178
Total	3254

💚 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!)

[elasticmachine]

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

[andrewkroh]

FWIW consul-template (MPL-2.0) has a pretty complete implementation of min and max functions. sprig also has implementations.

[ShourieG]

FWIW consul-template (MPL-2.0) has a pretty complete implementation of min and max functions. sprig also has implementations.

@andrewkroh made the min/max functions generic in a similar fashion.

[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 httpjson/add_min_max upstream/httpjson/add_min_max
git merge upstream/main
git push upstream httpjson/add_min_max

[ShourieG]

@efd6 I've reworked the min max functions to be more specific.

[ShourieG]

@efd6 made suggested changes.

[ShourieG]

@efd6 if all looks ok, are we good for merging atm ?

[andrewkroh]

We don't have a need to support lists of arguments. It adds complexity so let's simplify down to basic two-parameter functions.

The min and max functions should be shortcuts for something that can be expressed via the built-ins. The built-in comparator functions make sane comparisons between like types (see the lt implementation in text/template).

[[if lt .A .B ]][[ .A ]][[ else ]][[ .B ]] == [[min .A .B]]

[[if gt .A .B ]][[ .A ]][[ else ]][[ .B ]] == [[max .A .B]]

The existing numerical functions expect int64. We could continue that pattern, but as I pointed out in a previous comment this can be difficult to use correctly. It requires the users to know that only int64 values will be accepted. This is similar to creating type specific functions (e.g. minInt) because users need to know the types involved to select the correct function.

Dan raised a good concern about the some of the comparisons between types in consul-template implementation. I think we should still have a single min and max, but only allow comparisons between like types (as is handled in the builtin lt() func).

My proposal in 4cdc09c re-uses the lt function to create min and max. WDYT?

[ShourieG]

We don't have a need to support lists of arguments. It adds complexity so let's simplify down to basic two-parameter functions.

The min and max functions should be shortcuts for something that can be expressed via the built-ins. The built-in comparator functions make sane comparisons between like types (see the lt implementation in text/template).

[[if lt .A .B ]][[ .A ]][[ else ]][[ .B ]] == [[min .A .B]]

[[if gt .A .B ]][[ .A ]][[ else ]][[ .B ]] == [[max .A .B]]

The existing numerical functions expect int64. We could continue that pattern, but as I pointed out in a previous comment this can be difficult to use correctly. It requires the users to know that only int64 values will be accepted. This is similar to creating type specific functions (e.g. minInt) because users need to know the types involved to select the correct function.

Dan raised a good concern about the some of the comparisons between types in consul-template implementation. I think we should still have a single min and max, but only allow comparisons between like types (as is handled in the builtin lt() func).

My proposal in 4cdc09c re-uses the lt function to create min and max. WDYT?

@andrewkroh The simplification of down to using just 2 values instead of a list makes sense, but this approach again re-introduces the ability to compare values of different ranges and signs, I agree the user would need the knowledge of certain types to implement the current approach but shouldn't we expect the user to have that basic knowledge when using something like httpjson ? Also this is using reflection so the code becomes more complex, similar to the consul code(minor nit-pick). @efd6 What are your thoughts on this ?

[andrewkroh]

but this approach again re-introduces the ability to compare values of different ranges and signs

There is a difference. lt only compares like kinds ¹ (e.g. int to int, float to float). It makes an exception to allow comparing uint and int ², and it only casts from int -> uint64 when the int kind's value is non-negative (which should be safe AFAICT). lt will not compare an int with a float32, for example.

Footnotes

1. https://github.com/golang/go/blob/5fe3f0a265c90a9c0346403742c6cafeb154503b/src/text/template/funcs.go#L554-L561 ↩

2. https://github.com/golang/go/blob/5fe3f0a265c90a9c0346403742c6cafeb154503b/src/text/template/funcs.go#L543-L546 ↩

[ShourieG]

@andrewkroh alright then, we can go ahead with this approach if everyone's onboard, waiting on you @efd6.

[efd6]

I much prefer the simpler solution; the possibility of comparing different types is unfortunate, but it is at least consistent with the rest of the templating language in that it is merely an optimisation of an expression that can be made in-language, i.e. [[if lt .A .B ]][[ .A ]][[ else ]][[ .B ]].

[ShourieG]

@andrewkroh I've merged your changes to this PR. @efd Could you review and approve ?