xds: NACKs missing error detail

[howardjohn]

What version of gRPC are you using?

1.33.1

What version of Go are you using (go version)?

1.14

What operating system (Linux, Windows, …) and version?

Linux

What did you do?

Sent an invalid XDS response

What did you expect to see?

NACK with ErrorDetails set. From the spec "If Envoy had instead rejected configuration update X, it would reply with error_detail populated"

What did you see instead?

ErrorDetails unset

[menghanl]

Thanks for reporting. I will have make a fix soon.

What problem did this cause? Do you use the error details for other purposes other than logging?

[howardjohn]

Our control plane (Istio) expects ErrorDetails set on NACKs

[howardjohn]

Well, it only expects it for logging and metrics - but these are critical

[menghanl]

The fix is in #3975. Please take a look.

but these are critical

I'm not sure what you by "critical"

I totally agree that they are important. But the main purpose is for debugging (for human's use).
If you are relying on this field for other purposes (like, to check if a request is a NACK), that would cause other problems.

[howardjohn]

If config is being NACKed it means it not applying. This absolutely must be exposed to the user, trigger alerts, etc for any serious user running in production. I think we are agreeing but in different words - it is just for humans