enhancement(remap): introduce `is_TYPE` helper functions #6775

prognant · 2021-03-16T11:17:59Z

Closes #6732

Introduce a set of helper functions to test the type of a VRL var:

is_string("foobar") returns true

is_string(1515) returns false

is_integer(11) returns true

and so on for the supported types.

Signed-off-by: prognant pierre.rognant@datadoghq.com

jszwedko · 2021-03-16T14:49:21Z

lib/vrl/stdlib/src/type_of.rs

+    }
+}
+
+/*#[cfg(test)]


I think the unit tests should be runnable now as-of #6492

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

JeanMertz

I haven't looked at the implementation details yet, but before I do that, I wanted to note that a previous issue was filed to discuss this feature:

#5261

In it, @binarylogic also proposed a type function similar to type_of.

I raised a concern that this could become problematic in the future if we want to introduce type-level scoping.

For example, I envision us being able to do:

if is_string(.foo) {
	// compiler knows `.foo` is a string, so this is OK
	.foo = upcase(.foo)
}

For this to work, we'd need to add a feature to the compiler that allows functions to bubble up type information invariants about one or more of their arguments. In this case, the is_string function would let the compiler know whether .foo is a string, which can then be used within the if block scope to allow functions to treat .foo as if it were a string.

By having a type_of function, this becomes more difficult and/or hacky to solve because it would no longer just be the function that can inform the compiler about the type, you'd also need to special-case the conditional that happens afterward (e.g. type_of(.foo) == "string").

So, my preference would be to add all missing is_* functions, then more forward with the type-level scoping proposal, and after that make a proposal on how we could add type_of.

prognant · 2021-03-22T16:28:06Z

@JeanMertz Thanks for the insights ! I read the discussion in #5261 and based on my somehow very low knowledge of VRL I concur with the conclusion that is_<type> functions are a better option as of today. And ultimately after having a type-level scoping, we could then implement a proper type_of function that could properly type-feed a conditional block.

I will rework the PR accordingly.

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

StephenWakely · 2021-03-30T10:40:25Z

lib/vrl/stdlib/src/is_array.rs

+        match v {
+            Value::Array(_) => Ok(value!(true)),
+            _ => Ok(value!(false)),
+        }


This code is fine, but could be simplified using matches! to something like:

Ok(value!(matches!(v, Value::Array(_))))

At that point, you might as well collapse it all together 😄

Value has is_xxx() already implemented, so you can do:

self.value.resolve(ctx).map(Value::is_array)

StephenWakely · 2021-03-30T10:47:39Z

I think it would be worth adding an is_null function as well. Currently we have is_nullish, but that does a bit more than a simple is_null would do. eg.. is_nullish("") == true but is_null("") == false.

prognant · 2021-03-30T11:04:19Z

I think it would be worth adding an is_null function as well. Currently we have is_nullish, but that does a bit more than a simple is_null would do. eg.. is_nullish("") == true but is_null("") == false.

Thanks for the review !
I was thinking that some_val == null could be used for null type assertion (but I'm not 100% sure it would work) anyway for consistency purpose I agreee that having an is_null function make sense. I will add it.

StephenWakely · 2021-03-30T11:08:54Z

I was thinking that some_val == null

Yes, of course, I hadn't thought of that.

I think it might still come in handy for future work as Jean suggested earlier where we may then be able to derive typing information inside the if blocks.

JeanMertz

LGTM! I added one comment, but I don't consider that a blocker.

I think it might still come in handy for future work as Jean suggested earlier where we may then be able to derive typing information inside the if blocks.

null is a bit of a special case, since it's both a type and a value. If/when we add typed scopes, we'll have to support foo == null regardless, otherwise it would be very weird for is_null(foo) to work, but foo == null not. I don't have a particular opinion on if we should support is_null, it seems a bit pointless, but it does round up all the is_* functions, which might be something we want to do.

docs/reference/remap/functions/is_timestamp.cue

JeanMertz · 2021-03-30T11:40:25Z

lib/vrl/stdlib/src/is_array.rs

+        match v {
+            Value::Array(_) => Ok(value!(true)),
+            _ => Ok(value!(false)),
+        }


At that point, you might as well collapse it all together 😄

Value has is_xxx() already implemented, so you can do:

self.value.resolve(ctx).map(Value::is_array)

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

jszwedko

This looks good to me! Could we add these to the benchmark suite too?

https://github.com/timberio/vector/blob/07a5427677934be4270fe39de31ec5b460344722/lib/vrl/stdlib/benches/benches.rs

prognant · 2021-03-30T15:01:58Z

Sure will do.

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

jszwedko

🎉

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

pablosichert

Just some pedantic suggestions for the docs 😄

pablosichert · 2021-03-30T10:26:45Z

docs/reference/remap/functions/is_array.cue

+	arguments: [
+		{
+			name:        "value"
+			description: #"The value to check"#


Suggested change

description: #"The value to check"#

description: "The value to check."

pablosichert · 2021-03-30T10:27:17Z

docs/reference/remap/functions/is_array.cue

+			#"Returns `true` if `value` is an array."#,
+			#"Returns `false` if `value` is anything else."#,


Suggested change

#"Returns `true` if `value` is an array."#,

#"Returns `false` if `value` is anything else."#,

"Returns `true` if `value` is an array.",

"Returns `false` if `value` is anything else.",

pablosichert · 2021-03-30T10:27:29Z

docs/reference/remap/functions/is_boolean.cue

+	arguments: [
+		{
+			name:        "value"
+			description: #"The value to check"#


Suggested change

description: #"The value to check"#

description: "The value to check."

pablosichert · 2021-03-30T10:27:42Z

docs/reference/remap/functions/is_boolean.cue

+			#"Returns `true` if `value` is a boolean."#,
+			#"Returns `false` if `value` is anything else."#,


Suggested change

#"Returns `true` if `value` is a boolean."#,

#"Returns `false` if `value` is anything else."#,

"Returns `true` if `value` is a boolean.",

"Returns `false` if `value` is anything else.",

pablosichert · 2021-03-30T10:28:00Z

docs/reference/remap/functions/is_float.cue

+	arguments: [
+		{
+			name:        "value"
+			description: #"The value to check"#


Suggested change

description: #"The value to check"#

description: "The value to check."

pablosichert · 2021-04-01T20:02:30Z

docs/reference/remap/functions/is_regex.cue

+			#"Returns `true` if `value` is a regex."#,
+			#"Returns `false` if `value` is anything else."#,


Suggested change

#"Returns `true` if `value` is a regex."#,

#"Returns `false` if `value` is anything else."#,

"Returns `true` if `value` is a regex.",

"Returns `false` if `value` is anything else.",

pablosichert · 2021-04-01T20:02:59Z

docs/reference/remap/functions/is_string.cue

+	arguments: [
+		{
+			name:        "value"
+			description: #"The value to check"#


Suggested change

description: #"The value to check"#

description: "The value to check."

pablosichert · 2021-04-01T20:03:19Z

docs/reference/remap/functions/is_string.cue

+			#"Returns `true` if `value` is a string."#,
+			#"Returns `false` if `value` is anything else."#,


Suggested change

#"Returns `true` if `value` is a string."#,

#"Returns `false` if `value` is anything else."#,

"Returns `true` if `value` is a string.",

"Returns `false` if `value` is anything else.",

pablosichert · 2021-04-01T20:03:41Z

docs/reference/remap/functions/is_timestamp.cue

+	arguments: [
+		{
+			name:        "value"
+			description: #"The value to check"#


Suggested change

description: #"The value to check"#

description: "The value to check."

pablosichert · 2021-04-01T20:04:02Z

docs/reference/remap/functions/is_timestamp.cue

+			#"Returns `true` if `value` is a timestamp."#,
+			#"Returns `false` if `value` is anything else."#,


Suggested change

#"Returns `true` if `value` is a timestamp."#,

#"Returns `false` if `value` is anything else."#,

"Returns `true` if `value` is a timestamp.",

"Returns `false` if `value` is anything else.",

prognant force-pushed the prognant/add_is_TYPE_functions_to_VRL branch from 59b0ad2 to fc42d19 Compare March 16, 2021 11:35

jszwedko reviewed Mar 16, 2021

View reviewed changes

prognant marked this pull request as ready for review March 19, 2021 14:45

prognant requested review from a team and pablosichert and removed request for a team March 19, 2021 14:45

prognant added 4 commits March 19, 2021 17:56

enhancement(remap): introduce type_of helper function

99669e4

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

add tests

1f71ce9

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

add doc

886d8d6

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

minor fixes

bf9f784

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

prognant force-pushed the prognant/add_is_TYPE_functions_to_VRL branch from f3debf4 to bf9f784 Compare March 19, 2021 16:56

JeanMertz reviewed Mar 22, 2021

View reviewed changes

prognant added 3 commits March 26, 2021 15:24

add is_TYPE helpers

6c95a3d

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

add doc

a320351

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

remove type_of helper + doc minor fix

59e5c23

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

prognant changed the title ~~enhancement(remap): introduce type_of helper function~~ enhancement(remap): introduce is_TYPE helper functions Mar 26, 2021

StephenWakely reviewed Mar 30, 2021

View reviewed changes

JeanMertz approved these changes Mar 30, 2021

View reviewed changes

Address review

2d6952d

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

prognant requested a review from StephenWakely March 30, 2021 14:34

jszwedko reviewed Mar 30, 2021

View reviewed changes

cue fmt

c69b216

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

prognant force-pushed the prognant/add_is_TYPE_functions_to_VRL branch from 07c64e5 to c69b216 Compare March 30, 2021 15:05

add benchmark

930593d

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

jszwedko approved these changes Mar 30, 2021

View reviewed changes

minor test fix

c12592d

Signed-off-by: prognant <pierre.rognant@datadoghq.com>

prognant merged commit b7d7c4a into master Mar 31, 2021

prognant deleted the prognant/add_is_TYPE_functions_to_VRL branch March 31, 2021 12:58

pablosichert reviewed Apr 1, 2021

View reviewed changes

jszwedko mentioned this pull request Aug 5, 2022

remap-lang: support querying the type of value #5261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enhancement(remap): introduce `is_TYPE` helper functions #6775

enhancement(remap): introduce `is_TYPE` helper functions #6775

prognant commented Mar 16, 2021 •

edited

jszwedko Mar 16, 2021

JeanMertz left a comment

prognant commented Mar 22, 2021 •

edited

StephenWakely Mar 30, 2021

JeanMertz Mar 30, 2021

StephenWakely commented Mar 30, 2021

prognant commented Mar 30, 2021

StephenWakely commented Mar 30, 2021

JeanMertz left a comment •

edited

JeanMertz Mar 30, 2021

jszwedko left a comment

prognant commented Mar 30, 2021

jszwedko left a comment

pablosichert left a comment

pablosichert Mar 30, 2021

pablosichert Mar 30, 2021

pablosichert Mar 30, 2021

pablosichert Mar 30, 2021

pablosichert Mar 30, 2021

pablosichert Apr 1, 2021

pablosichert Apr 1, 2021

pablosichert Apr 1, 2021

pablosichert Apr 1, 2021

pablosichert Apr 1, 2021

	description: #"The value to check"#
	description: "The value to check."

		#"Returns `true` if `value` is an array."#,
		#"Returns `false` if `value` is anything else."#,

		#"Returns `true` if `value` is a boolean."#,
		#"Returns `false` if `value` is anything else."#,

		#"Returns `true` if `value` is a regex."#,
		#"Returns `false` if `value` is anything else."#,

		#"Returns `true` if `value` is a string."#,
		#"Returns `false` if `value` is anything else."#,

		#"Returns `true` if `value` is a timestamp."#,
		#"Returns `false` if `value` is anything else."#,

enhancement(remap): introduce is_TYPE helper functions #6775

enhancement(remap): introduce is_TYPE helper functions #6775

Conversation

prognant commented Mar 16, 2021 • edited

Choose a reason for hiding this comment

JeanMertz left a comment

Choose a reason for hiding this comment

prognant commented Mar 22, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenWakely commented Mar 30, 2021

prognant commented Mar 30, 2021

StephenWakely commented Mar 30, 2021

JeanMertz left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jszwedko left a comment

Choose a reason for hiding this comment

prognant commented Mar 30, 2021

jszwedko left a comment

Choose a reason for hiding this comment

pablosichert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enhancement(remap): introduce `is_TYPE` helper functions #6775

enhancement(remap): introduce `is_TYPE` helper functions #6775

prognant commented Mar 16, 2021 •

edited

prognant commented Mar 22, 2021 •

edited

JeanMertz left a comment •

edited