feat(telemetry): selectively transmit/redact subgraph ftv1 error messages to studio #3011

bnjjj · 2023-04-27T15:11:09Z

Add ability to transmit un-redacted errors from federated traces to Apollo Studio

When using subgraphs which are enabled with Apollo Federated Tracing, the error messages within those traces will be redacted by default.

New configuration (tracing.apollo.errors.subgraph.all.redact, which defaults to true) enables and disables the redaction mechanism. Similar configuration (tracing.apollo.errors.subgraph.all.send, which also defaults to true) enables and disables the entire transmission of the error to Studio.

The error messages returned to the clients are not changed or redacted from their previous behavior.

To enable sending subgraph's federated trace error messages to Studio without redaction, you can set the following configuration:

telemetry:
  apollo:
    errors:
      subgraph:
        all:
          send: true # (true = Send to Studio, false = Do not send; default: true)
          redact: false # (true = Redact full error message, false = Do not redact; default: true)

It is also possible to configure this per-subgraph using a subgraphs map at the same level as all in the configuration, much like other sections of the configuration which have subgraph-specific capabilities:

telemetry:
  apollo:
    errors:
      subgraph:
        all:
          send: true
          redact: false # Disable redaction as a default.  The `accounts` service enables it below.
        subgraphs:
          accounts: # Applies to the `accounts` subgraph, overriding the `all` global setting.
            redact: true # Redact messages from the `accounts` service.

By @bnjjj in #3011

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

abernix

Looks pretty good to me! Mostly a suggested change to the changeset entry.

apollo-router/src/plugins/telemetry/apollo.rs

abernix · 2023-05-03T10:27:02Z

apollo-router/src/plugins/telemetry/apollo.rs

-    /// Enable field level instrumentation for subgraphs via ftv1. ftv1 tracing can cause performance issues as it is transmitted in band with subgraph responses.
-    /// 0.0 will result in no field level instrumentation. 1.0 will result in always instrumentation.
-    /// Value MUST be less than global sampling rate
+    /// Field level instrumentation for subgraphs via ftv1. ftv1 tracing can cause performance issues as it is transmitted in band with subgraph responses.


What was the motivation for this change in relation to this PR?

docs/source/configuration/apollo-telemetry.mdx

.changesets/feat_bnjjj_add_apollo_redact_errors.md

apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs

BrynCooke · 2023-05-03T11:14:57Z

apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs

+        if let Some(mut t) = decode_ftv1_trace(s.as_str()) {
+            if error_config.redact || !error_config.send {
+                if let Some(root) = &mut t.root {
+                    redact_node_errors(root, !error_config.send);


I feel this code is confusing. The function is called redact_node_errors but takes the send config.
It'd be easier to get if it had the following signature preprocess_errors(root, redact, send)

Agree with this feedback, but let's follow-up on this in another PR.

Done here #3030

Co-authored-by: Bryn Cooke <BrynCooke@gmail.com>

.changesets/feat_bnjjj_add_apollo_redact_errors.md

abernix

LGTM. Thanks! (There's one follow-up comment within from @BrynCooke for another PR [maybe?])

…llo studio (#3030) related to #3011 (comment) --------- Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

@bnjjj

…ages to studio (#3011) When using subgraphs which are enabled with [Apollo Federated Tracing](https://www.apollographql.com/docs/router/configuration/apollo-telemetry/#enabling-field-level-instrumentation), the error messages within those traces will be **redacted by default**. New configuration (`tracing.apollo.errors.subgraph.all.redact`, which defaults to `true`) enables and disables the redaction mechanism. Similar configuration (`tracing.apollo.errors.subgraph.all.send`, which also defaults to `true`) enables and disables the entire transmission of the error to Studio. The error messages returned to the clients are **not** changed or redacted from their previous behavior. To enable sending subgraph's federated trace error messages to Studio **without redaction**, you can set the following configuration: ```yaml title="router.yaml" telemetry: apollo: errors: subgraph: all: send: true # (true = Send to Studio, false = Do not send; default: true) redact: false # (true = Redact full error message, false = Do not redact; default: true) ``` It is also possible to configure this **per-subgraph** using a `subgraphs` map at the same level as `all` in the configuration, much like other sections of the configuration which have subgraph-specific capabilities: ```yaml title="router.yaml" telemetry: apollo: errors: subgraph: all: send: true redact: false # Disable redaction as a default. The `accounts` service enables it below. subgraphs: accounts: # Applies to the `accounts` subgraph, overriding the `all` global setting. redact: true # Redact messages from the `accounts` service. ``` By [@bnjjj](https://github.com/bnjjj) in #3011 --------- Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Co-authored-by: Jesse Rosenberger <git@jro.cc> Co-authored-by: Bryn Cooke <BrynCooke@gmail.com>

…llo studio (#3030) related to #3011 (comment) --------- Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

feat(telemetry): add a way to redact errors for studio

482b78b

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

This comment has been minimized.

Sign in to view

apollo-bot2 assigned bnjjj Apr 27, 2023

bnjjj added 4 commits April 28, 2023 14:03

feat(telemetry): add a way to redact errors for studio

f2bbf1e

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

feat(telemetry): add a way to redact errors for studio

9c83a19

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

feat(telemetry): add a way to redact errors for studio

b96cf07

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

docs

4ab2ba5

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

bnjjj marked this pull request as ready for review May 2, 2023 10:16

bnjjj requested a review from StephenBarlow as a code owner May 2, 2023 10:16

abernix changed the title ~~feat(telemetry): add a way to redact errors for studio~~ feat(telemetry): selectively transmit/redact subgraph ftv1 error messages to studio May 2, 2023

abernix requested a review from BrynCooke May 3, 2023 07:20

abernix reviewed May 3, 2023

View reviewed changes

BrynCooke reviewed May 3, 2023

View reviewed changes

Apply suggestions from code review

7fda23e

Co-authored-by: Bryn Cooke <BrynCooke@gmail.com>

abernix reviewed May 3, 2023

View reviewed changes

.changesets/feat_bnjjj_add_apollo_redact_errors.md Outdated Show resolved Hide resolved

Apply suggestions from code review

a0a7206

abernix enabled auto-merge (squash) May 3, 2023 11:41

abernix added 2 commits May 3, 2023 11:44

Update tests to use <redacted> rather than <masked>.

8ee33fb

Update snaps

c258655

abernix approved these changes May 3, 2023

View reviewed changes

abernix merged commit 10b7efa into dev May 3, 2023

abernix deleted the bnjjj/add_apollo_redact_errors branch May 3, 2023 12:36

bnjjj mentioned this pull request May 3, 2023

refactor(telemetry): more readable code to redact errors going to apollo studio #3030

Merged

abernix mentioned this pull request May 3, 2023

prep release: v1.16.0 #3032

Merged

bnjjj added a commit that referenced this pull request May 10, 2023

refactor(telemetry): more readable code to redact errors going to apo…

c83ef8b

…llo studio (#3030) related to #3011 (comment) --------- Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

garypen pushed a commit that referenced this pull request May 10, 2023

refactor(telemetry): more readable code to redact errors going to apo…

d02d73a

…llo studio (#3030) related to #3011 (comment) --------- Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

abernix mentioned this pull request May 11, 2023

prep release: v1.18.1 #3081

Merged

Geal mentioned this pull request May 26, 2023

prep release: v1.19.1 #3155

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(telemetry): selectively transmit/redact subgraph ftv1 error messages to studio #3011

feat(telemetry): selectively transmit/redact subgraph ftv1 error messages to studio #3011

bnjjj commented Apr 27, 2023 •

edited by abernix

Loading

This comment has been minimized.

abernix left a comment

abernix May 3, 2023

BrynCooke May 3, 2023

abernix May 3, 2023

bnjjj May 3, 2023

abernix left a comment

feat(telemetry): selectively transmit/redact subgraph ftv1 error messages to studio #3011

feat(telemetry): selectively transmit/redact subgraph ftv1 error messages to studio #3011

Conversation

bnjjj commented Apr 27, 2023 • edited by abernix Loading

Add ability to transmit un-redacted errors from federated traces to Apollo Studio

This comment has been minimized.

abernix left a comment

Choose a reason for hiding this comment

abernix May 3, 2023

Choose a reason for hiding this comment

BrynCooke May 3, 2023

Choose a reason for hiding this comment

abernix May 3, 2023

Choose a reason for hiding this comment

bnjjj May 3, 2023

Choose a reason for hiding this comment

abernix left a comment

Choose a reason for hiding this comment

bnjjj commented Apr 27, 2023 •

edited by abernix

Loading