[pkg/ottl] Change OTTL contexts to handle paths with context

[edmocosta]

Description

This PR is part of #29017, and changes all existing OTTL contexts to properly handle ottl.Path with ottl.Path.Context. (I'm sorry for the big PR, but splitting it wouldn't make much difference considering all changes are related)

OTTL contexts changes:

• Added a new option EnablePathContextNames, so API consumers can enable the support to path with context names, previously added through [pkg/ottl] Change grammar to support expressing statements context via path names #34875.
• The parsePath functions were modified to properly handle paths with contexts.
  • For now, It's still backward compatible with paths without context so users do not need to rewrite their statements.
• Exposed a constant ContextName (it might be useful to configure the parser collection, for example, but I'm fine hiding it if folks think it's not necessary)

Considering we might deprecate the path without context support in the future, and aiming to avoid copy and paste all the existing tests, I've included some logic to dynamically copy the "without path context" tests, into a "with path context" tests, ensuring all paths are tested with both scenarios. I understand those generated tests make it hard to read/understand, but given the above considerations, I think it may be worth it (please let me know if you think otherwise, so I can change the approach).

Other important changes:

Currently, it's possible to have paths to contexts root objects, for example: set(attributes["body"], resource). Given resource has no dot separators on the path, the grammar extracts it into the path.Fields slice, letting the path.Context value empty. Why? This grammar behaviour is still necessary to keep backward compatibility with paths without context, otherwise it would start requiring contexts for all paths independently of the parser configuration.

Previous PRs didn't take this edge case into consideration, and a few places needed to be changed to address it:

• Context inferrer (including an extra fix of the default inferrer order)
• Parser prependContextToStatementPaths function
• Reusable OTTL contexts (contexts/internal), to also handle ottl.Path with only the Context set as an object root access (in addition to the exiting path == nil condition)

When/If we reach the point to deprecate paths without context, all those conditions can be removed, and the grammar changed to require and extract the context properly.

Link to tracking issue

#29017

Testing

Unit tests

Documentation

No changes

[edmocosta]

Hi @TylerHelmuth @evan-bradley!

I've marked this PR as a draft so I can double check my understanding on the final solution, and discuss/implement a topic before starting fully reviewing it.

Should we support fully qualified paths? let's take the transformprocessor, metric and datapoint contexts as an example:

convert_summary_count_val_to_sum("delta", true) where datapoint.count > 0

The above example would infer the datapoint context and run successfully, but what if I don't want to filter by any datapoint.*, and instead, I want to filter by a higher datapoint context:

convert_summary_count_val_to_sum("delta", true) where metric.name == "foo"

In this case, the inferred context would be the metric context, which does not support the convert_summary_count_val_to_sum function, for example.

No accepting fully qualified paths would make it impossible to users to enforce certain context without using it in the statement.

That said, I think we could accept both scenarios, for example, unqualified but correctly inferred from the statement:

convert_summary_count_val_to_sum("delta", true) where metric.name == "foo" and datapoint.count > 0

And fully qualified, aiming enforcing certain context:

convert_summary_count_val_to_sum("delta", true) where datapoint.metric.name == "foo"

WDYT?

[evan-bradley]

Thanks @edmocosta, really excited to see this one.

No accepting fully qualified paths would make it impossible to users to enforce certain context without using it in the statement.

I think that's a great observation. I'm personally in favor of adding this to allow users the ability to manually specify a context, I think we need to retain the ability for users to set contexts in ways not obvious from the paths alone. However, for this case:

convert_summary_count_val_to_sum("delta", true) where metric.name == "foo"

Could we (maybe in the future if fully-qualified paths will allow us to get around this) add an option to the context inferrer to use context-specific functions as hints for the context a statement should run in? I think this will offer the best UX and allow users to avoid needing to think about how to force a context.

[TylerHelmuth]

I agree that it seems like we need the function context to be a part of the equation. I'm glad you thought of this example now bc I don't think we discussed this before.

[edmocosta]

Could we (maybe in the future if fully-qualified paths will allow us to get around this) add an option to the context inferrer to use context-specific functions as hints for the context a statement should run in?

Yes, we can definitely make the context inferrer smarter adding the functions hint, it would require extracting the functions usage from the parsed statements, and verifying if the configured context parsers supports it, prioritising them accordingly.

Even though it would solve the functions issue, we would still need to deal with enums, which are prone to the exactly same problem. Differently from the functions, the ottl.Parser does not have a list of supported enums, so checking them would require either adapting the code or just trying to parse them using the configured EnumParser.

We can solve both issues changing the context inferrer - with the cost of increasing complexity a bit - but I'm wondering if we're missing any other edge-case here? Anyway, I don't think it would be a problem if we are - at least for now - considering users are still able to express their transformprocessor statements using the structured configuration style.

---

Summarising:

• Supporting fully qualified paths would be easy to implement, it only requires changing a bit the parsePath logic (introduced by this PR).
• If the context inferrer gets smarter and solves both issues, fully qualified path wouldn't be necessary, and it would offer the best UX possible.
• If any unforeseen case arrives, users are still able to configure their statements using the structured configuration style as an alternative.

That said, If you folks agree, I'll change the context inferrer as discussed, verifying the configured parsers functions, and trying to parse the enums.

[TylerHelmuth]

That said, If you folks agree, I'll change the context inferrer as discussed, verifying the configured parsers functions, and trying to parse the enums.

Ya I like this approach as it avoids users having to write datapoint.metric.name.

[edmocosta]

Hey @TylerHelmuth @evan-bradley, thanks again for the help, I've opened a new PR changing the context inferrer as discussed, and including the extra changes of this PR (to make it smaller). Once we get that PR merged, I'll update this one and mark as ready to review.