[WIP] Migrate Error Counting to Telemetry Plugin#7357
[WIP] Migrate Error Counting to Telemetry Plugin#7357
Conversation
|
@rregitsky, please consider creating a changeset entry in |
✅ Docs preview has no changesThe preview was not built because there were no changes. Build ID: 1039eab7bd16af8170a2dc30 |
apollo-router/src/graphql/mod.rs
Outdated
| locations, | ||
| path, | ||
| extensions, | ||
| apollo_id: Uuid::new_v4() |
There was a problem hiding this comment.
We might be able to refactor this particular error condition with the new plugin architecture such that we don't need to serde here in from_value_completion_value, worth a 2nd look.
There was a problem hiding this comment.
I'd like to hear more about the history of these errors. Why do we store them in extensions instead of errors? If we keep it in extensions, I don't see a good way to avoid serde
apollo-router/src/error.rs
Outdated
| .message(self.to_string()) | ||
| .locations(Vec::default()) | ||
| .and_path(path) | ||
| // Extension code is required, but is only used if extensions doesn't have it. We always |
There was a problem hiding this comment.
I didn't dig into it but why is it required? Is it b/c of the constructor param not being optional?
There was a problem hiding this comment.
Yeah exactly. I've decided to change it to optional because I don't really see a good reason why it needs to required. I get the idea that we prefer errors use them, but until we force it everywhere I don't think we can require it.
…PULSR-1504 # Conflicts: # apollo-router/src/services/router.rs
✅ Docs preview has no changesThe preview was not built because there were no changes. Build ID: dcdd3a4f3316307d3437b175 |
🛠️ Docs preview building...The preview is currently being built. Build ID: 8537aed013da71baafdcde7a |
3 participants
This is the primary PR where all changes were made. This has been split into 3 PRs to make it easier to digest:
router::RepsonsecreationErrorand use builder for allErrorcreationChecklist
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. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩