Add type safe Getter/Expr and Value#45363
Add type safe Getter/Expr and Value#45363bogdandrutu wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
31928fb to
36b7352
Compare
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
There was a problem hiding this comment.
This seems like a good first step (defining the runtime API), but without example "interpreter" or programs to run this through, it's unclear if it's not just creating the same issues in the current set of evaluators and/or whether it can expand its language feature set going forward.
I'd just a test that creates an expression representing an "OTTL statement" manually from these primitives to get a feel for how it works.
For bonus points (on architecture), I'd use syntax not yet in OTTL, e.g. the proposed for-yield pythonic-generator PR I had from a while back, something like:
set(log.attributes, [attr for attr in log.attributes if startswith(attr.key, "test")])
You'd need a new Expression type for the loop, something like:
func newLoopExpr[?](collection Expr[?], yield Expr[?], filter Expr[?], itemName string) Expr[?] {
return newExpr(func(ctx: context.context, tCtx ?) (V, error) {
// Note I'm ignoring casting for now. This "expression" should show
// why, and why the current `tCtx` pattern is detrimental to the future of OTTL.
list := collection.Eval(ctx, tCtx)
result := []
for i, v := range list {
// Here I need to construct a new context where `Expr` can still
// find values from the original context, but also *new* things are available.
// E.g. I want `atttr` to be the name you use to look up `v` in context.
listCtx := newListContext(tCtx, itemName, v, i)
if filter.Eval(ctx, listCtx) {
result = append(result, yield.Eval(ctx, listCtx))
}
}
return result, nil
}
}|
|
||
| // Expr is the base struct that represents a function call. | ||
| type Expr[K any] interface { | ||
| // Disallow implementations outside this package. |
There was a problem hiding this comment.
Is this just a "keep it private before it's public" thing, or do you want to avoid extension of this type?
| type genericExpr[K any, V Value] interface { | ||
| Expr[K] | ||
|
|
||
| Eval(ctx context.Context, tCtx K) (V, error) |
There was a problem hiding this comment.
Tying an expression to a typed context is one of the big issues with OTTL right now. It means you can't have different "scopes" which prevents things like for-loops, lambdas, etc. from being introduced to the language without huge awkwardness (or straight out shenanigans with casting) in your interpretation engine.
Is this needed for compatibility or can you somehow erase this?
I think you'd be MUCH better off having an "erased" or type-less expression / runtime and putting your type-safe wrappers on the outside of that (at least in the current state of Go generics).
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
No description provided.