Conversation
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Geoffroy Couprie <geoffroy@apollographql.com>
...uter/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
Outdated
Show resolved
Hide resolved
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
BrynCooke
left a comment
There was a problem hiding this comment.
Approved with the following comments.
-
Not sure we should at this stage bother populating the response headers in the ApolloExporter. It's potentially inaccurate right now, and we'll need to think about how we configure this properly.
-
Still not keen on the enabled thing. Would prefer that the value "default" is used under the header_name. There's no hard and fast rules on when to use
enabled, but I guess we can tackle this when we move out of experimental.
garypen
left a comment
There was a problem hiding this comment.
Looks great. One question that I'd like to know the answer to.
o0Ignition0o
left a comment
There was a problem hiding this comment.
LGTM overall, a couple of nits that seem to be copy paste mishaps.
Love the experimental documentation section, this is great! ❤️
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
We agreed with the studio team to use this as a workaround right now, they have an issue on their side to add a field in proto file to handle that correctly. It's still correct except we don't insert all response headers inside. We discussed with Jesse about that and if we see a need or request from our users we will add them all later but for now that's ok.
Could you add this comment in the right discussion please ? --> #2147 |
close #2080 Configuration example: ```yaml telemetry: tracing: experimental_response_trace_id: enabled: true # default: false header_name: "my-trace-id" # default: "apollo-trace-id" propagation: request: header_name: "x-request-id" jaeger: true ``` Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Co-authored-by: Geoffroy Couprie <geoffroy@apollographql.com>
close #2080
Configuration example:
TODO: