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 OTTL context inferrer utility #35721

Merged

Conversation

edmocosta
Copy link
Contributor

Description

This PR is part of #29017, and adds the context inferrer utility into the ottl package. This utility will be used by components to infer the context it should use to parse some statements, based only on its ottl.path's context prefixes.

This PR delivers a priority-based ottl.ContextInferrer implementation, which chooses the context based on a priority list passed on the ottl.NewPriorityContextInferrer function. It also exposes the ottl.DefaultPriorityContextInferrer function, which creates the same context inferrer type, but using a pre-defined context priorities.

For example, given the context priority ["spanevent", "span", "resource"], and inferring the context from the statement
set(span.foo, resource.value) where spanevent.bar == true, would result in the spanevent context, as it has the highest priority.

For more details on how it will be used, please have a look at the complete draft implementation.

To discuss

Do we really need to support the NewPriorityContextInferrer ignoreUnknownContext argument?

I would say yes, considering the inferrer could be used for any purpose, having the capability of ignoring unknown/non-prioritised contexts would be helpful and wouldn't require users to check if the Infer(..) function returned an unexpected value.

Link to tracking issue

#29017

Testing

Unit tests

Documentation

No changes

pkg/ottl/context_inferrer.go Outdated Show resolved Hide resolved
pkg/ottl/context_inferrer.go Outdated Show resolved Hide resolved
pkg/ottl/context_inferrer.go Outdated Show resolved Hide resolved
pkg/ottl/context_inferrer.go Outdated Show resolved Hide resolved
pkg/ottl/context_inferrer.go Outdated Show resolved Hide resolved
@edmocosta
Copy link
Contributor Author

Hi @TylerHelmuth! I've addressed your suggestions to under expose the context inferrer impl (c0a8b67). In addition to that, I've also removed the ignoreUnknownContext option, as we won't need that anymore. All internal usages on the draft are setting that flag to false, and considering we won't be exposing the context inferrer anymore, it would be a dead code. Thanks!

@TylerHelmuth TylerHelmuth added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 22, 2024
@evan-bradley evan-bradley merged commit ecf9538 into open-telemetry:main Oct 22, 2024
167 of 168 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/ottl Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants