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] Improve errors for invalid cache access and add option to set context's cache value #37166

Merged
merged 6 commits into from
Jan 13, 2025

Conversation

edmocosta
Copy link
Contributor

@edmocosta edmocosta commented Jan 13, 2025

Description

  • Improved error message for higher context cache access. Now when users try to configure statements like set(log.attributes["log_key"], resource.cache["log_key"]), they will see an error message similar to:

    + access to cache must be be performed using the same statement's context, please replace "resource.cache[log_key]" with "log.cache[log_key]"`
    - invalid argument at position 1: segment "cache" from path "resource.cache[log_key]" is not a valid path nor a valid OTTL keyword for the Resource context - review https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/pkg/ottl/contexts/ottlresource to see all valid paths
  • Added the WithCache option to all OTTL transformation contexts. This change allows API consumer to set the initial cache value or share it among different contexts/executions.

Link to tracking issue

Split of #36888

Testing

Unit tests

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.

pkg/ottl/contexts/internal/errors.go Outdated Show resolved Hide resolved
@TylerHelmuth
Copy link
Member

@edmocosta needs make tidylist

@TylerHelmuth
Copy link
Member

@edmocosta sorry we reverted the tidylist as it was causing problems. Now we need a rebase

@edmocosta
Copy link
Contributor Author

@edmocosta sorry we reverted the tidylist as it was causing problems. Now we need a rebase

NP, CI is green now, thanks for handling it.

@TylerHelmuth TylerHelmuth merged commit 7e6b19a into open-telemetry:main Jan 13, 2025
163 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 13, 2025
@edmocosta edmocosta deleted the ottl-cache-changes branch January 14, 2025 09:42
chengchuanpeng pushed a commit to chengchuanpeng/opentelemetry-collector-contrib that referenced this pull request Jan 26, 2025
…set context's cache value (open-telemetry#37166)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
- Improved error message for higher context `cache` access. Now when
users try to configure statements like `set(log.attributes["log_key"],
resource.cache["log_key"])`, they will see an error message similar to:

  ```diff
+ access to cache must be be performed using the same statement's
context, please replace "resource.cache[log_key]" with
"log.cache[log_key]"`
- invalid argument at position 1: segment "cache" from path
"resource.cache[log_key]" is not a valid path nor a valid OTTL keyword
for the Resource context - review
https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/pkg/ottl/contexts/ottlresource
to see all valid paths
  ```
- Added the `WithCache` option to all OTTL transformation contexts. This
change allows API consumer to set the initial cache value or share it
among different contexts/executions.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Split of
open-telemetry#36888

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit tests

<!--Please delete paragraphs that you did not use before submitting.-->
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.

4 participants