Skip to content

Commit

Permalink
Remove experimental label from apollo usage reporting configs (#5807)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bonnici
bonnici authored Sep 3, 2024
2 parents 0f4622c + 1d677bd commit 7499c16
Show file tree
Hide file tree
Showing 22 changed files with 89 additions and 589 deletions.
10 changes: 10 additions & 0 deletions .changesets/config_apollo_telemetry_config_rename.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
### Remove experimental label from apollo usage reporting configs ([#5807](https://github.com/apollographql/router/pull/5807))

All known issues related to the new Apollo usage report generation have been resolved so we are renaming some experimental options to be non-experimental.
* `telemetry.apollo.experimental_apollo_metrics_reference_mode` is now `telemetry.apollo.metrics_reference_mode`
* `telemetry.apollo.experimental_apollo_signature_normalization_algorithm` is now `telemetry.apollo.signature_normalization_algorithm`
* `experimental_apollo_metrics_generation_mode` has been removed since the Rust implementation has been the default since v1.49.0 and it is generating reports identical to the router-bridge implementation

Previous configuration will warn but still work.

By [@bonnici](https://github.com/bonnici) in https://github.com/apollographql/router/pull/5807
59 changes: 0 additions & 59 deletions apollo-router/src/apollo_studio_interop/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,65 +169,6 @@ pub(crate) struct ComparableUsageReporting {
pub(crate) result: UsageReporting,
}

/// Enum specifying the result of a comparison.
#[derive(Debug)]
pub(crate) enum UsageReportingComparisonResult {
/// The UsageReporting instances are the same
Equal,
/// The stats_report_key in the UsageReporting instances are different
StatsReportKeyNotEqual,
/// The referenced_fields in the UsageReporting instances are different. When comparing referenced
/// fields, we ignore the ordering of field names.
ReferencedFieldsNotEqual,
/// Both the stats_report_key and referenced_fields in the UsageReporting instances are different.
BothNotEqual,
}

impl ComparableUsageReporting {
/// Compare this to another UsageReporting.
pub(crate) fn compare(&self, other: &UsageReporting) -> UsageReportingComparisonResult {
let sig_equal = self.result.stats_report_key == other.stats_report_key;
let refs_equal = self.compare_referenced_fields(&other.referenced_fields_by_type);
match (sig_equal, refs_equal) {
(true, true) => UsageReportingComparisonResult::Equal,
(false, true) => UsageReportingComparisonResult::StatsReportKeyNotEqual,
(true, false) => UsageReportingComparisonResult::ReferencedFieldsNotEqual,
(false, false) => UsageReportingComparisonResult::BothNotEqual,
}
}

fn compare_referenced_fields(
&self,
other_ref_fields: &HashMap<String, ReferencedFieldsForType>,
) -> bool {
let self_ref_fields = &self.result.referenced_fields_by_type;
if self_ref_fields.len() != other_ref_fields.len() {
return false;
}

for (name, self_refs) in self_ref_fields.iter() {
let maybe_other_refs = other_ref_fields.get(name);
if let Some(other_refs) = maybe_other_refs {
if self_refs.is_interface != other_refs.is_interface {
return false;
}

let self_field_names_set: HashSet<_> =
self_refs.field_names.clone().into_iter().collect();
let other_field_names_set: HashSet<_> =
other_refs.field_names.clone().into_iter().collect();
if self_field_names_set != other_field_names_set {
return false;
}
} else {
return false;
}
}

true
}
}

/// Generate a ComparableUsageReporting containing the stats_report_key (a normalized version of the operation signature)
/// and referenced fields of an operation. The document used to generate the signature and for the references can be
/// different to handle cases where the operation has been filtered, but we want to keep the same signature.
Expand Down
210 changes: 0 additions & 210 deletions apollo-router/src/apollo_studio_interop/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -765,213 +765,3 @@ async fn test_enums_from_response_fragments() {
let generated = enums_from_response(query_str, op_name, schema_str, response_str);
assert_enums_from_response!(&generated);
}

#[test(tokio::test)]
async fn test_compare() {
let source = ComparableUsageReporting {
result: UsageReporting {
stats_report_key: "# -\n{basicResponseQuery{field1 field2}}".into(),
referenced_fields_by_type: HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),
(
"SomeResponse".into(),
ReferencedFieldsForType {
field_names: vec!["field1".into(), "field2".into()],
is_interface: false,
},
),
]),
},
};

// Same signature and ref fields should match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: source.result.stats_report_key.clone(),
referenced_fields_by_type: source.result.referenced_fields_by_type.clone(),
}),
UsageReportingComparisonResult::Equal
));

// Reordered signature should not match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: "# -\n{basicResponseQuery{field2 field1}}".into(),
referenced_fields_by_type: source.result.referenced_fields_by_type.clone(),
}),
UsageReportingComparisonResult::StatsReportKeyNotEqual
));

// Different signature should not match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: "# NamedQuery\nquery NamedQuery {basicResponseQuery{field1 field2}}"
.into(),
referenced_fields_by_type: source.result.referenced_fields_by_type.clone(),
}),
UsageReportingComparisonResult::StatsReportKeyNotEqual
));

// Reordered parent type should match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: source.result.stats_report_key.clone(),
referenced_fields_by_type: HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),
(
"SomeResponse".into(),
ReferencedFieldsForType {
field_names: vec!["field1".into(), "field2".into()],
is_interface: false,
},
),
])
}),
UsageReportingComparisonResult::Equal
));

// Reordered fields should match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: source.result.stats_report_key.clone(),
referenced_fields_by_type: HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),
(
"SomeResponse".into(),
ReferencedFieldsForType {
field_names: vec!["field2".into(), "field1".into()],
is_interface: false,
},
),
])
}),
UsageReportingComparisonResult::Equal
));

// Added parent type should not match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: source.result.stats_report_key.clone(),
referenced_fields_by_type: HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),
(
"SomeResponse".into(),
ReferencedFieldsForType {
field_names: vec!["field1".into(), "field2".into()],
is_interface: false,
},
),
(
"OtherType".into(),
ReferencedFieldsForType {
field_names: vec!["otherField".into()],
is_interface: false,
},
),
])
}),
UsageReportingComparisonResult::ReferencedFieldsNotEqual
));

// Added field should not match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: source.result.stats_report_key.clone(),
referenced_fields_by_type: HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),
(
"SomeResponse".into(),
ReferencedFieldsForType {
field_names: vec!["field1".into(), "field2".into(), "field3".into()],
is_interface: false,
},
),
])
}),
UsageReportingComparisonResult::ReferencedFieldsNotEqual
));

// Missing parent type should not match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: source.result.stats_report_key.clone(),
referenced_fields_by_type: HashMap::from([(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),])
}),
UsageReportingComparisonResult::ReferencedFieldsNotEqual
));

// Missing field should not match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: source.result.stats_report_key.clone(),
referenced_fields_by_type: HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),
(
"SomeResponse".into(),
ReferencedFieldsForType {
field_names: vec!["field1".into()],
is_interface: false,
},
),
])
}),
UsageReportingComparisonResult::ReferencedFieldsNotEqual
));

// Both different should not match
assert!(matches!(
source.compare(&UsageReporting {
stats_report_key: "# -\n{basicResponseQuery{field2 field1}}".into(),
referenced_fields_by_type: HashMap::from([(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["basicResponseQuery".into()],
is_interface: false,
},
),])
}),
UsageReportingComparisonResult::BothNotEqual
));
}
4 changes: 2 additions & 2 deletions apollo-router/src/configuration/metrics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,9 @@ impl InstrumentData {
apollo.router.config.apollo_telemetry_options,
"$.telemetry.apollo",
opt.signature_normalization_algorithm,
"$.experimental_apollo_signature_normalization_algorithm",
"$.signature_normalization_algorithm",
opt.metrics_reference_mode,
"$.experimental_apollo_metrics_reference_mode"
"$.metrics_reference_mode"
);

// We need to update the entry we just made because the selected strategy is a named object in the config.
Expand Down
10 changes: 10 additions & 0 deletions apollo-router/src/configuration/migrations/0027-apollo_telemetry_experimental.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
description: Some Apollo telemetry options are no longer experimental
actions:
- type: delete
path: experimental_apollo_metrics_generation_mode
- type: move
from: telemetry.apollo.experimental_apollo_signature_normalization_algorithm
to: telemetry.apollo.signature_normalization_algorithm
- type: move
from: telemetry.apollo.experimental_apollo_metrics_reference_mode
to: telemetry.apollo.metrics_reference_mode
Loading

0 comments on commit 7499c16

Please sign in to comment.