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

[pkg/ottl] Validate Keys are used #30162

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

TylerHelmuth
Copy link
Member

Description:
This PR updates OTTL's validation logic to validate the keys are considered during path parsing.

Unlike the path, keys are needed during hot path execution. For that reason, the contexts need all the keys during parsing, not an individual key that might be linked to the next key.

This PR updates the Path interface to have a list of keys instead of a link to the first Key. This keeps the keys interaction closer to the original key struct from the grammar, which means when the interfaces release we have less breaking changes.

Link to tracking Issue:
Closes #30051

Testing:
Added unit tests to validate that indexing fails on paths that dont use keys and pass on paths that allow keys.

Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just a few nitpicks.

.chloggen/ottl-validate-key.yaml Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions_test.go Outdated Show resolved Hide resolved
@TylerHelmuth TylerHelmuth merged commit a4853ed into open-telemetry:main Jan 8, 2024
85 checks passed
@TylerHelmuth TylerHelmuth deleted the ottl-validate-key branch January 8, 2024 01:26
@github-actions github-actions bot added this to the next release milestone Jan 8, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jan 10, 2024
**Description:** 
This PR updates OTTL's validation logic to validate the keys are
considered during path parsing.

Unlike the path, keys are needed during hot path execution. For that
reason, the contexts need all the keys during parsing, not an individual
key that might be linked to the next key.

This PR updates the Path interface to have a list of keys instead of a
link to the first Key. This keeps the keys interaction closer to the
original key struct from the grammar, which means when the interfaces
release we have less breaking changes.

**Link to tracking Issue:** <Issue number if applicable>
Closes
open-telemetry#30051

**Testing:** <Describe what testing was performed and which tests were
added.>
Added unit tests to validate that indexing fails on paths that dont use
keys and pass on paths that allow keys.

---------

Co-authored-by: Evan Bradley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pkg/ottl] Validate that all parts of a Key are used by a Context
3 participants