-
Notifications
You must be signed in to change notification settings - Fork 257
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
Bump OpenTelemetry api to 1.0.0 #828
Conversation
Update related OT libraries for testing.
@@ -18,8 +18,6 @@ | |||
}, | |||
"homepage": "https://github.com/apollographql/federation#readme", | |||
"dependencies": { | |||
"@opentelemetry/api": "^0.20.0", |
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.
The dependencies of this package doesn't really matter since it is not published. I think you may want to add it to the dev deps of the top level (also unpublished) package.json so that it's a non peer deps somewhere? But maybe it works if you don't? Perhaps if this works then it's good enough.
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.
In theory I guess as long as npm7 is used these dependencies are not needed? api will come in as a peer dependency, the others shouldn't be needed.
This PR adds instrumentation to emit the following spans: federation.request - total time to server a request. federation.validate - time to validate the federated query. federation.plan - time to plan the federated query. federation.execute - the total time executing query. federation.fetch - time fetching data from a subgraph. federation.postprocessing - time to render the response from all the subgraph data. The instrumentation looks a little messy due to the requirement to set error status on spans on exception, but also if errors are added to the response. It is a reissue of the following PRs #803 #828
This PR adds instrumentation to emit the following spans: federation.request - total time to server a request. federation.validate - time to validate the federated query. federation.plan - time to plan the federated query. federation.execute - the total time executing query. federation.fetch - time fetching data from a subgraph. federation.postprocessing - time to render the response from all the subgraph data. The instrumentation looks a little messy due to the requirement to set error status on spans on exception, but also if errors are added to the response. It is a reissue of the following PRs #803 #828
* Add OpenTelemetry support This PR adds instrumentation to emit the following spans: federation.request - total time to server a request. federation.validate - time to validate the federated query. federation.plan - time to plan the federated query. federation.execute - the total time executing query. federation.fetch - time fetching data from a subgraph. federation.postprocessing - time to render the response from all the subgraph data. The instrumentation looks a little messy due to the requirement to set error status on spans on exception, but also if errors are added to the response. It is a reissue of the following PRs #803 #828 * WIP on opentelemetry docs * First run at OT docs ready for review * Incorporate feedback from @BrynCooke * Add note to update @apollo/gateway * Release - apollo-federation-integration-testsuite@0.25.2 - @apollo/gateway@0.31.0 - @apollo/harmonizer@0.3.4 * Update changelog for publish Co-authored-by: bryncooke <bryncooke@gmail.com> Co-authored-by: Stephen Barlow <barlow.stephen+git@gmail.com> Co-authored-by: Stephen Barlow <stephen@apollographql.com>
Bump OpenTelemetry api to 1.0.0
Update related OT libraries for testing.
Note that the api dependenciy has been moved to a peer dependency in gateway-js.
Also dependencies have been remove from here, but perhaps there was a reason they were added in the first place?