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

fix(telemetry): fix supergraph query selector #6324

Merged
merged 2 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .changesets/fix_bnjjj_fix_880.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
### Fix telemetry instrumentation using supergraph query selector ([PR #6324](https://github.com/apollographql/router/pull/6324))

Query selector was raising error logs like `this is a bug and should not happen`. It's now fixed.
Now this configuration will work properly:
```yaml title=router.yaml
telemetry:
exporters:
metrics:
common:
views:
# Define a custom view because operation limits are different than the default latency-oriented view of OpenTelemetry
- name: oplimits.*
aggregation:
histogram:
buckets:
- 0
- 5
- 10
- 25
- 50
- 100
- 500
- 1000
instrumentation:
instruments:
supergraph:
oplimits.aliases:
value:
query: aliases
type: histogram
unit: number
description: "Aliases for an operation"
oplimits.depth:
value:
query: depth
type: histogram
unit: number
description: "Depth for an operation"
oplimits.height:
value:
query: height
type: histogram
unit: number
description: "Height for an operation"
oplimits.root_fields:
value:
query: root_fields
type: histogram
unit: number
description: "Root fields for an operation"
```

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/6324
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ where
})
}
None => {
::tracing::error!("cannot convert static instrument into a counter, this is an error; please fill an issue on GitHub");
failfast_debug!("cannot convert static instrument into a counter, this is an error; please fill an issue on GitHub");
}
}
}
Expand Down Expand Up @@ -1501,7 +1501,7 @@ where
});
}
None => {
::tracing::error!("cannot convert static instrument into a histogram, this is an error; please fill an issue on GitHub");
failfast_debug!("cannot convert static instrument into a histogram, this is an error; please fill an issue on GitHub");
}
}
}
Expand Down
25 changes: 0 additions & 25 deletions apollo-router/src/plugins/telemetry/config_new/selectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,25 +1069,6 @@ impl Selector for SupergraphSelector {
ctx: &Context,
) -> Option<opentelemetry::Value> {
match self {
SupergraphSelector::Query { query, .. } => {
let limits_opt = ctx
.extensions()
.with_lock(|lock| lock.get::<OperationLimits<u32>>().cloned());
match query {
Query::Aliases => {
limits_opt.map(|limits| opentelemetry::Value::I64(limits.aliases as i64))
}
Query::Depth => {
limits_opt.map(|limits| opentelemetry::Value::I64(limits.depth as i64))
}
Query::Height => {
limits_opt.map(|limits| opentelemetry::Value::I64(limits.height as i64))
}
Query::RootFields => limits_opt
.map(|limits| opentelemetry::Value::I64(limits.root_fields as i64)),
Query::String => None,
}
}
SupergraphSelector::ResponseData {
response_data,
default,
Expand Down Expand Up @@ -3289,12 +3270,6 @@ mod test {
.unwrap(),
4.into()
);
assert_eq!(
selector
.on_response_event(&crate::graphql::Response::builder().build(), &context)
.unwrap(),
4.into()
);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,31 @@ telemetry:

instrumentation:
instruments:
supergraph:
oplimits.aliases:
value:
query: aliases
type: histogram
unit: number
description: "Aliases for an operation"
oplimits.depth:
value:
query: depth
type: histogram
unit: number
description: "Depth for an operation"
oplimits.height:
value:
query: height
type: histogram
unit: number
description: "Height for an operation"
oplimits.root_fields:
value:
query: root_fields
type: histogram
unit: number
description: "Root fields for an operation"
graphql:
field.execution: true
list.length: true
Expand Down Expand Up @@ -38,7 +63,4 @@ telemetry:
condition:
eq:
- field_name: string
- "topProducts"



- "topProducts"
21 changes: 21 additions & 0 deletions apollo-router/tests/integration/telemetry/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,27 @@ async fn test_graphql_metrics() {
router.start().await;
router.assert_started().await;
router.execute_default_query().await;
router
.assert_log_not_contains("this is a bug and should not happen")
.await;
router
.assert_metrics_contains(
r#"oplimits_aliases_sum{otel_scope_name="apollo/router"} 0"#,
None,
)
.await;
router
.assert_metrics_contains(
r#"oplimits_root_fields_sum{otel_scope_name="apollo/router"} 1"#,
None,
)
.await;
router
.assert_metrics_contains(
r#"oplimits_depth_sum{otel_scope_name="apollo/router"} 2"#,
None,
)
.await;
router
.assert_metrics_contains(r#"graphql_field_list_length_sum{graphql_field_name="topProducts",graphql_field_type="Product",graphql_type_name="Query",otel_scope_name="apollo/router"} 3"#, None)
.await;
Expand Down