Add preview_datadog_agent_sampling (#6112)

apollographql · Dec 18, 2024 · 68fce74 · 68fce74
1 parent 3bdcacd
commit 68fce74
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 15 deletions.
diff --git 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
@@ -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,42 +2387,46 @@ 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();
 
     let context_start = strip_leading_ignored_tokens(after_dollar)?;
     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
@@ -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