Add toggle for including error messages in reports#1615
Add toggle for including error messages in reports#1615martijnwalraven merged 3 commits intomasterfrom
Conversation
We always send traces that includes an error node if the trace has an error. In the case that sending errors is disabled, we replace the message and remove the location.
11bab34 to
8e30c45
Compare
|
This PR does not currently test the end to end integration of the error pipeline from apollo server to Engine ingestion to Engine frontend. At best right now, we can use a similar strategy to the full AS2 lifecycle test |
|
I actually just fixed that test today, because the trace report checking wasn't actually being called, among other things. But that should work now. |
|
Agree we need more comprehensive Engine reporting integration tests, but we can tackle that later. |
| }), | ||
| ); | ||
|
|
||
| // With noErrorTraces, the Engine proxy strips all error information |
There was a problem hiding this comment.
I wouldn't explain the behavior by comparing it to the noErrorTraces option in the Engine proxy, we should just describe what it does.
There was a problem hiding this comment.
Okay that makes sense. I'll include the comment in the commit message. I think it's important to understand the context around how AS2 differs from the proxy.
| 'sendReport()' on other signals if you'd like. Note that 'sendReport()' | ||
| does not run synchronously so it cannot work usefully in an 'exit' handler. | ||
|
|
||
| * `sendErrorTraces`: boolean |
There was a problem hiding this comment.
Since this setting isn't really about whether or not to send a trace, maybe it should be something like maskErrors or maskErrorDetails?
There was a problem hiding this comment.
Totally! The name is definitely an artifact of evolving from not sending them completely to masking
| ), | ||
| json: JSON.stringify(error), | ||
| } | ||
| : { message: 'Masked by Apollo Engine Reporting' }; |
There was a problem hiding this comment.
Maybe just <masked>?
4f45d9f to
6981267
Compare
Note that the Engine proxy strips all error information from the traces with noErrorTraces set. To get errors to show up in the ui, the proxy sends error counts inside of the aggregated stats reports. To get similar behavior inside of the apollo server metrics reporting, we always send a trace and mask out the PII.
6981267 to
6152862
Compare
We always send traces that includes an error node if the trace has an
error. In the case that sending errors is disabled, we replace the
message and remove the location.
closes #1613
The ui looks like:
TODO: