Expected literal as function argment

[elwinar]

I tried something like this:

format = "combined"
if (.stream == "stderr") {
    format = "error"
}
msg = parse_nginx_log!(.message, format)

and got the following error

error[E610]: function compilation error: error[E400] unexpected expression type
  ┌─ :5:7
  │
5 │ msg = parse_nginx_log!(.message, format)
  │       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  │       │
  │       unexpected expression for argument "format"
  │       expected: literal
  │       received: variable call
  │
  = learn more about error code 400 at https://errors.vrl.dev/400
  = see language documentation at https://vrl.dev
  = try your code in the VRL REPL, learn more at https://vrl.dev/examples

I've looked at the reference for this function (https://vector.dev/docs/reference/vrl/functions/#parse_nginx_log) and the VRL specification, and didn't read anywhere that variables couldn't be used in place of literals? Maybe I'm missing something?

[pront]

Hi @elwinar, this looks like a bug to me.

The if block doesn't change the type of the variable `format:

if (.stream == "stderr") {
    format = "error"
}

You can use this workaround as an immediate solution.

[pront]

I'm willing to have a go at fixing it if you think it's accessible to a behinner in rust.

I didn't dive into it so I cannot really comment on this. This is the compile function if you want to take a look. If try your example without the if block, it should work and your if block doesn't change the type of the variable but it results in a compilation error so I am suspicious of that part.

Also, I take it that the format argument is correctly expected to be a literal, meaning it can't be taken from the event itself?

The format argument is used to select one of the predefined regexes and then that regex is used to capture the fields and create the structured log. We do not auto-detect the format based on event contents although that would be interesting. Does this answer your question?

[jszwedko]

I believe the issue is that the format argument is defined as a literal and, with the if statement, the value becomes dynamic rather than static. However, both values are static so the compiler could likely become smart enough to compile a version of the function for each possible value. This is likely to be a much more difficult change though.

Instead, I think the parse_nginx_format function could be expanded to allow for a dynamic format. Usually we require literals when allowing for variables introduces a lot of performance overhead (e.g. parsing a regex for each execution of VRL would be very slow so we require regex arguments to be literals), but here I don't think that is the case. This would introduce an additional failure mode (the format argument being invalid since this couldn't be validated at compile time).

[jszwedko]

I've looked at the reference for this function (https://vector.dev/docs/reference/vrl/functions/#parse_nginx_log) and the VRL specification, and didn't read anywhere that variables couldn't be used in place of literals? Maybe I'm missing something?

This is true, the docs don't make it clear when function arguments need to be static and cannot be variables.

[pront]

Thanks Jesse, these are very help clarifications. Improving the compiler to generate a function per variants is not trivial indeed but it would generically solve this problem without any hit to performance.

[elwinar]

Haven't had the time to look at it yet, but it isn't forgotten. I'll have a go at generatic per-variant functions at some point.