Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check lookup path syntax at compile time #13033

Closed
Tracked by #7070
fuchsnj opened this issue Jun 8, 2022 · 1 comment
Closed
Tracked by #7070

Check lookup path syntax at compile time #13033

fuchsnj opened this issue Jun 8, 2022 · 1 comment
Assignees
Labels
type: feature A value-adding code addition that introduce new functionality.

Comments

@fuchsnj
Copy link
Member

fuchsnj commented Jun 8, 2022

A note for the community

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Proposal

Today lookup paths outside of a VRL context (ex: in a transform config) are parsed at runtime. If the path has an invalid syntax, it is not detected until runtime and will just not match anything. There is no error or warning generated. This is not a great user experience. The path syntax should be checked at compile time and an error shown to the user if the syntax is invalid. Ideally this would happen when the path is pre-parsed (for performance).

Version

master

@fuchsnj fuchsnj added the type: feature A value-adding code addition that introduce new functionality. label Jun 8, 2022
@pront pront self-assigned this Jul 7, 2023
github-merge-queue bot pushed a commit that referenced this issue Jul 14, 2023
This part of #13033.

Tried this with a config where `source_type_key` is:
* `""` --> no source entry
* not defined --> same as above
* `"foo"` --> `"foo":"demo_logs"`
* `"foo.bar"` --> `"foo":{"bar":"demo_logs"}`
* `"foo.."` --> `Configuration error. error=Invalid field path "foo.."`

The config:
```
data_dir = "/Users/pavlos.rontidis/my_tests/vector"

[log_schema]
source_type_key = "foo"

[sources.source0]
format = "json"
type = "demo_logs"

[sinks.sink0]
inputs = ["source0"]
target = "stdout"
type = "console"

[sinks.sink0.encoding]
codec = "json"
```
github-merge-queue bot pushed a commit that referenced this issue Jul 18, 2023
This part of #13033.

Summary of these changes:
* LogSchema::host_key is now an `OptionalValuePath`
* `host_key`s that appear in configs are now also `OptionalValuePath`s
* There should be no `unwrap()` calls outside of tests.

---------

Co-authored-by: Stephen Wakely <fungus.humungus@gmail.com>
github-merge-queue bot pushed a commit that referenced this issue Jul 21, 2023
## Motivation
This part of #13033.

## Summary
* `LogSchema::message_key` is now an `OptionalValuePath`.
* To avoid hacky `String` to `&'static str` conversions, I changed the
`Requirement::meaning` key type to `String`.
@pront pront closed this as completed Sep 22, 2023
@pront
Copy link
Contributor

pront commented Sep 22, 2023

There are very few places where we are still parsing the path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A value-adding code addition that introduce new functionality.
Projects
None yet
Development

No branches or pull requests

2 participants