From 68fce74573188c62dc2d951d77deaf1c169d2c19 Mon Sep 17 00:00:00 2001 From: Bryn Cooke Date: Mon, 16 Dec 2024 23:30:03 +0000 Subject: [PATCH] Add `preview_datadog_agent_sampling ` (#6112) --- apollo-federation/src/error/mod.rs | 4 ++ .../src/query_graph/build_query_graph.rs | 39 ++++++++++++------- .../telemetry/metrics-exporters/overview.mdx | 2 +- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/apollo-federation/src/error/mod.rs b/apollo-federation/src/error/mod.rs index caeecf0a4a..08d337a4d5 100644 --- a/apollo-federation/src/error/mod.rs +++ b/apollo-federation/src/error/mod.rs @@ -133,6 +133,8 @@ pub enum SingleFederationError { UnknownOperation, #[error("Must provide operation name if query contains multiple operations")] OperationNameNotProvided, + #[error(r#"{message} in @fromContext substring "{context}""#)] + FromContextParseError { context: String, message: String }, #[error("Unsupported custom directive @{name} on fragment spread. Due to query transformations during planning, the router requires directives on fragment spreads to support both the FRAGMENT_SPREAD and INLINE_FRAGMENT locations.")] UnsupportedSpreadDirective { name: Name }, #[error("{message}")] @@ -305,6 +307,8 @@ impl SingleFederationError { SingleFederationError::InvalidGraphQL { .. } | SingleFederationError::InvalidGraphQLName(_) => ErrorCode::InvalidGraphQL, SingleFederationError::InvalidSubgraph { .. } => ErrorCode::InvalidGraphQL, + // Technically it's not invalid graphql, but it is invalid syntax inside graphql... + SingleFederationError::FromContextParseError { .. } => ErrorCode::InvalidGraphQL, // TODO(@goto-bus-stop): this should have a different error code: it's not invalid, // just unsupported due to internal limitations. SingleFederationError::UnsupportedSpreadDirective { .. } => ErrorCode::InvalidGraphQL, diff --git a/apollo-federation/src/query_graph/build_query_graph.rs b/apollo-federation/src/query_graph/build_query_graph.rs index 0c820c5622..c4c18c96a1 100644 --- a/apollo-federation/src/query_graph/build_query_graph.rs +++ b/apollo-federation/src/query_graph/build_query_graph.rs @@ -19,7 +19,6 @@ use strum::IntoEnumIterator; use crate::bail; use crate::error::FederationError; use crate::error::SingleFederationError; -use crate::internal_error; use crate::link::federation_spec_definition::get_federation_spec_definition_from_subgraph; use crate::link::federation_spec_definition::FederationSpecDefinition; use crate::link::federation_spec_definition::KeyDirectiveArguments; @@ -2364,6 +2363,14 @@ lazy_static! { Regex::new(r#"^([A-Za-z_](?-u:\w)*)((?s:.)*)$"#).unwrap(); } +fn context_parse_error(context: &str, message: &str) -> FederationError { + SingleFederationError::FromContextParseError { + context: context.to_string(), + message: message.to_string(), + } + .into() +} + fn parse_context(field: &str) -> Result<(String, String), FederationError> { // PORT_NOTE: The original JS regex, as shown below // /^(?:[\n\r\t ,]|#[^\n\r]*(?![^\n\r]))*\$(?:[\n\r\t ,]|#[^\n\r]*(?![^\n\r]))*([A-Za-z_]\w*(?!\w))([\s\S]*)$/ @@ -2380,15 +2387,16 @@ fn parse_context(field: &str) -> Result<(String, String), FederationError> { iter_into_single_item(CONTEXT_PARSING_LEADING_PATTERN.captures_iter(input)) .and_then(|c| c.get(1)) .map(|m| m.as_str()) - .ok_or_else(|| { - internal_error!(r#"Failed to skip any leading ignored tokens in @fromContext substring "{input}""#) - }) + .ok_or_else(|| context_parse_error(input, "Failed to skip any leading ignored tokens")) } let dollar_start = strip_leading_ignored_tokens(field)?; let mut dollar_iter = dollar_start.chars(); if dollar_iter.next() != Some('$') { - bail!(r#"Failed to find leading "$" in @fromContext substring "{dollar_start}""#); + return Err(context_parse_error( + dollar_start, + r#"Failed to find leading "$""#, + )); } let after_dollar = dollar_iter.as_str(); @@ -2396,26 +2404,29 @@ fn parse_context(field: &str) -> Result<(String, String), FederationError> { let Some(context_captures) = iter_into_single_item(CONTEXT_PARSING_CONTEXT_PATTERN.captures_iter(context_start)) else { - bail!( - r#"Failed to find context name token and selection in @fromContext substring "{context_start}""# - ); + return Err(context_parse_error( + dollar_start, + "Failed to find context name token and selection", + )); }; let context = match context_captures.get(1).map(|m| m.as_str()) { Some(context) if !context.is_empty() => context, _ => { - bail!( - r#"Expected to find non-empty context name in @fromContext substring "{context_start}""# - ); + return Err(context_parse_error( + context_start, + "Expected to find non-empty context name", + )); } }; let selection = match context_captures.get(2).map(|m| m.as_str()) { Some(selection) if !selection.is_empty() => selection, _ => { - bail!( - r#"Expected to find non-empty selection in @fromContext substring "{context_start}""# - ); + return Err(context_parse_error( + context_start, + "Expected to find non-empty selection", + )); } }; diff --git a/docs/source/reference/router/telemetry/metrics-exporters/overview.mdx b/docs/source/reference/router/telemetry/metrics-exporters/overview.mdx index efb22e3ccc..5ca04f389b 100644 --- a/docs/source/reference/router/telemetry/metrics-exporters/overview.mdx +++ b/docs/source/reference/router/telemetry/metrics-exporters/overview.mdx @@ -165,7 +165,7 @@ telemetry: static: # Always apply this attribute to all metrics for all subgraphs - name: kind - value: subgraph_request + value: subgraph_request # each subgraph request updates a metric separately errors: # Only work if it's a valid GraphQL error (for example if the subgraph returns an http error or if the router can't reach the subgraph) include_messages: true # Will include the error message in a message attribute extensions: # Include extensions data