[alerting] add a toString() to context variables whose values are objects

[pmuellr]

For context variables provided by an alert, to be used in action parameters, usually the values of the variables are strings or other primitive values. However, they can be objects as well. The metrics alert makes use of this for it's grouping (I believe).

When a customer uses these object-valued variables directly as a mustache "variable", mustache will render these values as the following, which isn't very useful.

[Object object]

It's possible to add a toString() method to these object values, which would then get invoked when the object is used as a "variable". So one could be added that returned eg JSON.stringify(this). It's unlikely to be the form a customer would want to use directly, but it is better than what currently happens, and should provide the info to the customer to refine the variable references while fine-tuning alert actions.

Alert executors can do this themselves, and I've proposed that before, but I'm wondering if we should just do this automagically in alerting. We could also add one on the top-level context, so you could use {{context}} to see the full context - again, mainly for diagnostic purposes, seems like it could be useful.

I suppose we'd "walk" the context provided by the alert, probably deep clone it (since we'll be modifying it), and then for every "object" in the graph, add the relevant toString() method to the object, if there's not already a toString property.

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[pmuellr]

This may be a DUP of issue #75601, haven't looked closely

[ChristopherShih]

Hello, I am new to contributing but would like to help. Would this problem be reasonable to start with? Any information on where I should start looking (files this logic is located) would be appreciated. Thanks in advance!

[pmuellr]

👋 @ChristopherShih

it might be reasonable! I'll let you decide.

The relevant code, is here:

kibana/x-pack/plugins/alerts/server/task_runner/transform_action_params.ts

Lines 39 to 62 in 3651748

	const result = cloneDeepWith(actionParams, (value: unknown) => {
	if (!isString(value)) return;
	// when the list of variables we pass in here changes,
	// the UI will need to be updated as well; see:
	// x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts
	const variables = {
	alertId,
	alertName,
	spaceId,
	tags,
	alertInstanceId,
	context,
	state,
	params: alertParams,
	};
	return Mustache.render(value, variables);
	});
	// The return type signature for `cloneDeep()` ends up taking the return
	// type signature for the customizer, but rather than pollute the customizer
	// with casts, seemed better to just do it in one place, here.
	return (result as unknown) as AlertActionParams;
	}

I think the trick is going to be to walk the variables object, recursively, and for any property that isn't a primitive, add a function to it, I think a toString() function, which would return the JSON.stringify()d version of that object. At the top-level, that's context, state and params. It may get a bit complicated with arrays, as arrays of primitives get rendered good enough by mustache already, but arrays of non-primiitve objects will look like [Object object], [Object object], etc. So we don't want to add the function to arrays, but do want to add it to non-primitive array elements.

The way you'd test this is to create an alert, add an action like a server log, trying using a variable like context directly in the message action parameter as {{context}}. Right now, that should render when the message is logged by the action, as [Object object]. Once we "fix" this, you would see { ... }, which would list all of the properties of the context, recursively.

I'm not sure why we have that variables variable set within the cloneDeepWith() call - it looks like it can probably be moved outside of the function, before cloneDeepWith() is invoked. Be worth a shot trying, I think it would make the function a little more readable.

[pmuellr]

As noted in #82044 (comment), this is a DUP of #75601 . And I have an implementation of this in not-yet-merged-ATM PR #83919, since I was already mucking around in that code, and this was easy enough to add.

[chadnormanimpact]

has anyone been able to surface something like user.email in Alerts?