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] Add new IsMap and IsString functions #22750

Merged
merged 14 commits into from
Jun 9, 2023

Conversation

TylerHelmuth
Copy link
Member

Description:
Adds new functions to facilitate type checking. Most useful when checking the type of a body to determine if it needs to be parsed or not.

Link to tracking Issue:
Closes #22161

Testing:
Added tests

Documentation:
Updated func readme

@TylerHelmuth TylerHelmuth requested a review from a team May 24, 2023 18:09
@github-actions github-actions bot requested a review from kentquirk May 24, 2023 18:09
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.

Overall I think this looks good, but as we discussed offline I think it would make sense to use the typed Getters to do the type checking in these functions so we have a single source of truth for type checks. We could have these functions take typed Getter parameters, then check whether the typed Getter returns an error. To distinguish type errors from other errors, typed Getters would return a TypeError or similar error type that indicates an error is from a type mismatch.

pkg/ottl/expression.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_is_string.go Outdated Show resolved Hide resolved
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 9, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

pkg/ottl/ottlfuncs/func_is_map_test.go Show resolved Hide resolved
pkg/ottl/expression_test.go Outdated Show resolved Hide resolved
@TylerHelmuth TylerHelmuth merged commit 8a2ac13 into open-telemetry:main Jun 9, 2023
@TylerHelmuth TylerHelmuth deleted the ottl-check-type branch June 9, 2023 19:16
@github-actions github-actions bot added this to the next release milestone Jun 9, 2023
Caleb-Hurshman pushed a commit to observIQ/opentelemetry-collector-contrib that referenced this pull request Jul 6, 2023
…2750)

* Add new functions

* Add docs

* changelog

* fix lint

* Switch to typed getters using typed errors

* Use errors.As

* Revert "Use errors.As"

This reverts commit 386d53540e5d1bb1cc8d0e30cf1db47309460d57.

* Ensure wrapped TypeErrors are returned

* Add type check to tests

* Added more tests

* Use TypeError in tests to ensure the error is wrapped

* Check that false is returned with error

* Fix lint

* Wrap errors in TypeLike getters
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] Allow checking path type
3 participants