[pkg/ottl] add Setter for value expression

[kaisecheng]

Component(s)

pkg/ottl

Is your feature request related to a problem? Please describe.

In signaltometricsconnector, the config accepts OTTL ValueExpression to extract a value from incoming data. For example, user can config resource.attribute["server.address"] to get hostname from log, metric and trace.

However, ValueExpression does not support Setter which means user cannot config resource.attribute["answer"] to save the result to the attribute.

Describe the solution you'd like

Add Setter support to ValueExpression and ParseValueExpression()

type ValueExpression[K any] struct {
	getter Getter[K]
	setter Setter[K]
}

Then, processor can support ValueExpression config for getting and setting an attribute dynamically. Example:

resolve:
  source: resource.attribute["server.hostname"]
  target: resource.attribute["server.ip"]

resolve:
  source: log.body["hostname"]
  target: log.body["ip"]

Describe alternatives you've considered

Without Setter, currently we can use ValueExpression to extract value and config target context and target attribute name to specify the location to store the result.

resolve:
    sources: resource.attributes["hostname"] # Value expressions + context inference (#40194)
    target_context: "resource" # context where the result attribute should be set
    target_attribute: "ip" # new attribute name

Additional context

No response

[github-actions]

Pinging code owners:

• pkg/ottl: @TylerHelmuth @evan-bradley @edmocosta

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[axw]

@kaisecheng have you considered this alternative? Parse the "target", and transform it to a statement like set(<target>, <placeholder>), where <placeholder> is a value expression that will resolve to the (presumably transformed) source value expression?

[kaisecheng]

@axw I can see two more alternatives

1. Processor has limit flexibility to config the target and expects a follow-up transform processor to set the value to desired position
2. Processor builds and parses a statement inside and evaluate result.

(1) is the alternative you mentioned.

I feel the pain when I'm forced to assign the value to a predefined location that doesn't suit my needs. This often requires me to add a follow-up OTTL statement to correct it per processor. If we could set the value in the right place from the beginning, the user experience would be much better.

(2) is a heavy alternative.

The processor needs to construct a string set(resource.attribute["answer"], "the answer"), parse it into ottl.statement and evaluate the result like running another Transform Processor internally.

However, I believe most processors don't need the full flexibility and complexity of ottl.statement with support for conditions and functions. Instead, processors prefer to take and set value using value expression with context inference based on user’s config.

[edmocosta]

I think @axw suggestion is a valid alternative and could work for this use-case, but it's true that it would require you to implement all the data consumers, similar to what the transform processor does.

I don't think it's possible to safely convert a value expression into an OTTL setter, as value expression are not limited to paths, for example, the expressions ToUpperCase(name), 1 + 1, value_double * count, [1, 2, 3] aren't paths, meaning they've no setters under the hood.

The only alternative I can think of would be exposing a new parser function that leverages the internal path's parser (ottl.PathExpressionParser), and takes a path string as argument, convert it, and return its corresponding getter/setter.
I can imagine we would have a few issues with this, but I'm curious to know what other code owners think about adding such feature.

[axw]

Processor has limit flexibility to config the target and expects a follow-up transform processor to set the value to desired position
...

(1) is the alternative you mentioned.

I feel the pain when I'm forced to assign the value to a predefined location that doesn't suit my needs. This often requires me to add a follow-up OTTL statement to correct it per processor. If we could set the value in the right place from the beginning, the user experience would be much better.

That doesn't sound like what I was proposing, maybe I was unclear. My proposal would be a bit more work on the processor implementation side, but the UX would be exactly what you described above, i.e.

resolve:
  source: resource.attribute["server.hostname"]
  target: resource.attribute["server.ip"]

In this case, your processor would internally take the target and turn it into the statement set(resource.attribute["server.ip"], ...). If I understood your second alternative above, it's the same as what I was suggesting. I agree that it's more work on the processor side though.

Coming back to the original proposal: say a Setter field were added to the ValueExpression struct type. At best, that Setter would return an error at runtime, and presumably you would want to know at startup whether the user has misconfigured things, e.g. by defining an expression like 1+1 as @edmocosta mentions above.

One way that might work is to add a Setter that is non-nil only for "settable" expressions, i.e. paths, and add a ValueExpression method to obtain the Setter and fail if the value is non-settable. This is similar to https://pkg.go.dev/reflect#Value.CanSet for example.

That could look like this:

var errNonSettableValueExpression = errors.New("value expression cannot be set") // maybe something more helpful

type ValueExpression[K any] struct {
	getter Getter[K]
	setter Setter[K] // nil for non-settable expressions
}

// Setter returns a Setter for settable value expressions, and returns an error otherwise.
func (e *ValueExpression[K]) Setter() (Setter[K], error) {
	if e.setter == nil {
		return nil, errNonSettableValueExpression
	}
	return e.setter, nil
}

[edmocosta]

I liked you suggestion @axw, If I'm not mistaken, when the ValueExpression getter field is built from a path, the underlying type is ottl.GetSetter, so with that approach, we probably wouldn't need to worry about creating the setter, just adapt the ValueExpression creation on the parser.

[edmocosta]

@TylerHelmuth @evan-bradley, when you get the chance, could you please share your thoughts on this? Thank you!