Skip to content
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

Remove experimental label from apollo usage reporting configs #5807

Merged
merged 12 commits into from
Sep 3, 2024
Merged
10 changes: 10 additions & 0 deletions .changesets/config_apollo_telemetry_config_rename.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
### ([#5807](https://github.com/apollographql/router/pull/5807))
bonnici marked this conversation as resolved.
Show resolved Hide resolved

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.
* `experimental_apollo_metrics_generation_mode` is now `apollo_metrics_generation_mode`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `experimental_apollo_metrics_generation_mode` is now `apollo_metrics_generation_mode`
* `experimental_apollo_metrics_generation_mode` is removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops thanks, this must have been a copy-paste error. I had something else to put here.

* `telemetry.apollo.experimental_apollo_signature_normalization_algorithm` is now `telemetry.apollo.signature_normalization_algorithm`
* `telemetry.apollo.experimental_apollo_metrics_reference_mode` has been removed since the Rust implementation has been the default for a few versions 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
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
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
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
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
Loading