response_body is not valid under telemetry spans for custom attribute

[Samjin]

https://www.apollographql.com/docs/router/configuration/telemetry/instrumentation/selectors/#supergraph
It says supergraph has selector of response_body, but the router config schema does not contain it. Add a custom attribute under supergraph would fail.

Expected behavior
Expect response_body selector to be valid under either supergraph or router.

Output

telemetry:
  instrumentation:
    spans:
      supergraph:
        attributes:
┌        "x-custom2":
|           response_body: "arg2"
└-----> {"response_body":"arg2"} is not valid under any of the schemas listed in the 'anyOf' keyword

[Geal]

it looks like this is happening because response_body should be a JSON path. The error message is not clear enough, this is something we should fix

[Samjin]

it looks like this is happening because response_body should be a JSON path. The error message is not clear enough, this is something we should fix

Are you saying arg2 should be a JSON path? I think I tried that and didn't work. The schema config definition(aka (configuration_schema.json) doesn't have response_body as valid property either.

[smyrick]

Yep, adding here that under supergraph attributes in the YAML config, we only allow these response attributes (as seen from the generated configuration_schema.json from Router 1.43.1

• response_header
• response_context

Yet our docs indicate that we have support for response_body as well.

If we add support for response_body that would align with the docs and yes it should be a JSON selector but we might want to consider doing what we did for subgraph responses too and split that into

• supergraph_response_data
• supergraph_response_errors

[BrynCooke]

Hmm, yes response_body does seem to be missing.

[BrynCooke]

The main issue with implementing this is that the response when we get to the supergraph stage is a stream. To support this we will need to extend the Selector trait to add somethign along the lines of on_response_part. This would allow the selector handling to be injected into the stream of responses.

[smyrick]

Removing from the docs here to reduce confusion until it is actually implemented: #4905

[BrynCooke]

@bnjjj Is this fixed by your latest PR?

[bnjjj]

Yes it's part of this PR but it will be response_errors and response_data instead.

[abernix]

@bnjjj Should this have been marked as one of the "Fixes" in #5022? (In other words, can it be closed?)

[bnjjj]

Yes!