diff --git a/.changesets/fix_spawn_blocking_parser.md b/.changesets/fix_spawn_blocking_parser.md deleted file mode 100644 index 0fd442df94..0000000000 --- a/.changesets/fix_spawn_blocking_parser.md +++ /dev/null @@ -1,5 +0,0 @@ -### use spawn_blocking for query parsing & validation ([PR #5235](https://github.com/apollographql/router/pull/5235)) - -Moves query parsing and validation in a tokio blocking task to prevent all executor threads from blocking on large queries. - -By [@xuorig](https://github.com/xuorig) in https://github.com/apollographql/router/pull/5235 \ No newline at end of file diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index a409df3756..ff28cbc4c5 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -269,10 +269,7 @@ where } in all_cache_keys { let context = Context::new(); - let doc = match query_analysis - .parse_document(&query, operation.as_deref()) - .await - { + let doc = match query_analysis.parse_document(&query, operation.as_deref()) { Ok(doc) => doc, Err(_) => continue, }; @@ -313,10 +310,7 @@ where }) .await; if entry.is_first() { - let doc = match query_analysis - .parse_document(&query, operation.as_deref()) - .await - { + let doc = match query_analysis.parse_document(&query, operation.as_deref()) { Ok(doc) => doc, Err(error) => { let e = Arc::new(QueryPlannerError::SpecError(error)); diff --git a/apollo-router/src/services/layers/query_analysis.rs b/apollo-router/src/services/layers/query_analysis.rs index 71eae898e7..4beb5a1e21 100644 --- a/apollo-router/src/services/layers/query_analysis.rs +++ b/apollo-router/src/services/layers/query_analysis.rs @@ -11,7 +11,6 @@ use http::StatusCode; use lru::LruCache; use router_bridge::planner::UsageReporting; use tokio::sync::Mutex; -use tokio::task; use crate::apollo_studio_interop::generate_extended_references; use crate::apollo_studio_interop::ExtendedReferenceStats; @@ -74,32 +73,12 @@ impl QueryAnalysisLayer { } } - pub(crate) async fn parse_document( + pub(crate) fn parse_document( &self, query: &str, operation_name: Option<&str>, ) -> Result { - let query = query.to_string(); - let operation_name = operation_name.map(|o| o.to_string()); - let schema = self.schema.clone(); - let conf = self.configuration.clone(); - - // Must be created *outside* of the spawn_blocking or the span is not connected to the - // parent - let span = tracing::info_span!(QUERY_PARSING_SPAN_NAME, "otel.kind" = "INTERNAL"); - - task::spawn_blocking(move || { - span.in_scope(|| { - Query::parse_document( - &query, - operation_name.as_deref(), - schema.as_ref(), - conf.as_ref(), - ) - }) - }) - .await - .expect("parse_document task panicked") + Query::parse_document(query, operation_name, &self.schema, &self.configuration) } pub(crate) async fn supergraph_request( @@ -148,7 +127,8 @@ impl QueryAnalysisLayer { let res = match entry { None => { - match self.parse_document(&query, op_name.as_deref()).await { + let span = tracing::info_span!(QUERY_PARSING_SPAN_NAME, "otel.kind" = "INTERNAL"); + match span.in_scope(|| self.parse_document(&query, op_name.as_deref())) { Err(errors) => { (*self.cache.lock().await).put( QueryAnalysisKey { diff --git a/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header-2.snap b/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header-2.snap index c1fe319e7a..a573b5890f 100644 --- a/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header-2.snap +++ b/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header-2.snap @@ -228,54 +228,6 @@ resourceSpans: code: 0 schemaUrl: "" schemaUrl: "" - - resource: - attributes: - - key: apollo.client.host - value: - stringValue: "[redacted]" - - key: apollo.client.uname - value: - stringValue: "[redacted]" - - key: apollo.graph.ref - value: - stringValue: test - - key: apollo.router.id - value: - stringValue: "[redacted]" - - key: apollo.schema.id - value: - stringValue: "[redacted]" - - key: apollo.user.agent - value: - stringValue: "[redacted]" - droppedAttributesCount: 0 - scopeSpans: - - scope: - name: apollo-router - version: "[version]" - attributes: [] - droppedAttributesCount: 0 - spans: - - traceId: "[trace_id]" - spanId: "[span_id]" - traceState: "" - parentSpanId: "[span_id]" - flags: 0 - name: parse_query - kind: 1 - startTimeUnixNano: "[start_time]" - endTimeUnixNano: "[end_time]" - attributes: [] - droppedAttributesCount: 0 - events: [] - droppedEventsCount: 0 - links: [] - droppedLinksCount: 0 - status: - message: "" - code: 0 - schemaUrl: "" - schemaUrl: "" - resource: attributes: - key: apollo.client.host diff --git a/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header.snap b/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header.snap index c1fe319e7a..a573b5890f 100644 --- a/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header.snap +++ b/apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header.snap @@ -228,54 +228,6 @@ resourceSpans: code: 0 schemaUrl: "" schemaUrl: "" - - resource: - attributes: - - key: apollo.client.host - value: - stringValue: "[redacted]" - - key: apollo.client.uname - value: - stringValue: "[redacted]" - - key: apollo.graph.ref - value: - stringValue: test - - key: apollo.router.id - value: - stringValue: "[redacted]" - - key: apollo.schema.id - value: - stringValue: "[redacted]" - - key: apollo.user.agent - value: - stringValue: "[redacted]" - droppedAttributesCount: 0 - scopeSpans: - - scope: - name: apollo-router - version: "[version]" - attributes: [] - droppedAttributesCount: 0 - spans: - - traceId: "[trace_id]" - spanId: "[span_id]" - traceState: "" - parentSpanId: "[span_id]" - flags: 0 - name: parse_query - kind: 1 - startTimeUnixNano: "[start_time]" - endTimeUnixNano: "[end_time]" - attributes: [] - droppedAttributesCount: 0 - events: [] - droppedEventsCount: 0 - links: [] - droppedLinksCount: 0 - status: - message: "" - code: 0 - schemaUrl: "" - schemaUrl: "" - resource: attributes: - key: apollo.client.host diff --git a/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id-2.snap b/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id-2.snap index 4e1e59aac6..9db10c85be 100644 --- a/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id-2.snap +++ b/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id-2.snap @@ -228,54 +228,6 @@ resourceSpans: code: 0 schemaUrl: "" schemaUrl: "" - - resource: - attributes: - - key: apollo.client.host - value: - stringValue: "[redacted]" - - key: apollo.client.uname - value: - stringValue: "[redacted]" - - key: apollo.graph.ref - value: - stringValue: test - - key: apollo.router.id - value: - stringValue: "[redacted]" - - key: apollo.schema.id - value: - stringValue: "[redacted]" - - key: apollo.user.agent - value: - stringValue: "[redacted]" - droppedAttributesCount: 0 - scopeSpans: - - scope: - name: apollo-router - version: "[version]" - attributes: [] - droppedAttributesCount: 0 - spans: - - traceId: "[trace_id]" - spanId: "[span_id]" - traceState: "" - parentSpanId: "[span_id]" - flags: 0 - name: parse_query - kind: 1 - startTimeUnixNano: "[start_time]" - endTimeUnixNano: "[end_time]" - attributes: [] - droppedAttributesCount: 0 - events: [] - droppedEventsCount: 0 - links: [] - droppedLinksCount: 0 - status: - message: "" - code: 0 - schemaUrl: "" - schemaUrl: "" - resource: attributes: - key: apollo.client.host diff --git a/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id.snap b/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id.snap index 4e1e59aac6..9db10c85be 100644 --- a/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id.snap +++ b/apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id.snap @@ -228,54 +228,6 @@ resourceSpans: code: 0 schemaUrl: "" schemaUrl: "" - - resource: - attributes: - - key: apollo.client.host - value: - stringValue: "[redacted]" - - key: apollo.client.uname - value: - stringValue: "[redacted]" - - key: apollo.graph.ref - value: - stringValue: test - - key: apollo.router.id - value: - stringValue: "[redacted]" - - key: apollo.schema.id - value: - stringValue: "[redacted]" - - key: apollo.user.agent - value: - stringValue: "[redacted]" - droppedAttributesCount: 0 - scopeSpans: - - scope: - name: apollo-router - version: "[version]" - attributes: [] - droppedAttributesCount: 0 - spans: - - traceId: "[trace_id]" - spanId: "[span_id]" - traceState: "" - parentSpanId: "[span_id]" - flags: 0 - name: parse_query - kind: 1 - startTimeUnixNano: "[start_time]" - endTimeUnixNano: "[end_time]" - attributes: [] - droppedAttributesCount: 0 - events: [] - droppedEventsCount: 0 - links: [] - droppedLinksCount: 0 - status: - message: "" - code: 0 - schemaUrl: "" - schemaUrl: "" - resource: attributes: - key: apollo.client.host