[Error Counting] Add ID to graphql::Error #7712
Conversation
|
@rregitsky, please consider creating a changeset entry in |
rregitsky
changed the title
rregitsky
changed the title
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 2 changed, 0 removedBuild ID: 103874ef904d501652521a52 URL: https://www.apollographql.com/docs/deploy-preview/103874ef904d501652521a52 |
…efactor_PULSR_1504
…efactor_PULSR_1504
…LSR_1504' into rreg/err_count_error_refactor_PULSR_1504
|
Is there some background documentation/discussion on what these IDs are for, and why |
We will need a “strong identity” with which errors to count / not / count / redact in each router execution layer that is maintained in the response context. This means we’ll need to mark each error with some kind of internal error id which can then be added to a list of “already counted” errors. |
…efactor_PULSR_1504 # Conflicts: # apollo-router/src/plugins/connectors/handle_responses.rs # apollo-router/src/services/layers/persisted_queries/mod.rs
This makes two major changes to
graphql::Error:apollo_idUUID to errors to allow us to uniquely identify Errors. These IDs are randomly generated on construction unless one is specified by the input. Its setter is private so that Error construction is forced throughnew()where we randomly generate the ID. The getter is public throughapollo_id(). This value is not serialized as we'd prefer not to expose the value the user. This is not for security reasons, but more because it has no value outside of the router context.Errorconstruction into builders. Theapollo_idsetter is private so we are forced to avoid using the constructor.Other minor, but notable changes:
apollo.router.operations.errormeasurements with the_entitypath will now emit using_entitypath rather than being replaced with the entity's name. This is to prevent double counting when the error counting migration is introduced in part 3. We will attempt to fix this behavior in a fast follow.Defaulttrait fromErrorto force a randomapollo_idto be generatedextension_codeno longer required when building anErroras many places weren't setting one with the constructor. We can revisit making it required once all of those places have proper error codes.Part 2 of a split from #7357. Part 1 can be found here: #7699
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
A lot of (if not most) features benefit from built-in observability and
debug-level logs. Please read this guidance on metrics best-practices. ↩Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩