-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add more information in spans #1119
Conversation
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
✅ Deploy Preview for apollo-router-docs canceled.
|
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gut feeling is that this information should be in a span in the TowerSubgraphService.
Eventually it is very possible that we will have other types of subgraph service that do not actually make an http call.
@BrynCooke Not sure to understand your comment. At the moment I only put these records on the |
Yeah, I think the fetch span should stay as it is, and that a new span should go in TowerSubgraphService. Having this client span fetch means that anything in the subgraph service pipeline such as caching would also be marked as client. I guess the point I'm trying to make is that the existing fetch span is too early to be a client span. |
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>
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Related to #911