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

add support of chunk responses for selectors and instrument in telemetry #5022

Merged
merged 23 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6db5b02
add support of chunk responses for selectors and instrument in telemetry
bnjjj Apr 25, 2024
c357a08
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
bnjjj Apr 26, 2024
ce224f9
rename trait method and configuration
bnjjj Apr 26, 2024
539cbe7
update json schema
bnjjj Apr 26, 2024
75ff672
add support for error selectors and add more tests for event_response
bnjjj Apr 26, 2024
adeb7c8
fix lints
bnjjj Apr 26, 2024
e8d8b5d
add changelog
bnjjj Apr 26, 2024
0aa4a67
some fixes
bnjjj Apr 26, 2024
e9387f4
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
bnjjj Apr 29, 2024
d13c0eb
update json schema
bnjjj Apr 29, 2024
c3d6034
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
bnjjj Apr 29, 2024
dfcff47
Merge branch 'dev' into bnjjj/feat_chunk_resp_selector
bnjjj Apr 30, 2024
d9afda6
fix bugs and add more tests for cost and event
bnjjj Apr 30, 2024
ae2f437
add a new test + a fix
bnjjj Apr 30, 2024
23580ec
fix counter
bnjjj Apr 30, 2024
d753b7d
oops
bnjjj Apr 30, 2024
a64d91c
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
bnjjj May 13, 2024
4e3d05b
don't keep the lock too long
bnjjj May 13, 2024
82b0d3e
don't keep the lock too long
bnjjj May 13, 2024
b456548
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
bnjjj May 14, 2024
152c279
rename on_event_response to on_response_event
bnjjj May 14, 2024
c097e38
update description for event instruments
bnjjj May 14, 2024
43c959e
Merge branch 'dev' of github.com:apollographql/router into bnjjj/feat…
bnjjj May 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changesets/feat_bnjjj_feat_chunk_resp_selector.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
### Add support of event responses and critical errors for selectors and instrument in telemetry ([Issue #5027](https://github.com/apollographql/router/issues/5027))

Giving the ability to compute new attributes everytime we receive a new event in supergraph response. Would be really helpful to create observability for subscriptions and defer.

I also added the support of `on_error` for selectors and especially `error` selector I added for every services. Which will let you create some metrics,events or span attributes containing error message. By adding this we will now have to ability to create a counter of request timed out for subgraphs for example:

```yaml
telemetry:
instrumentation:
instruments:
subgraph:
requests.timeout:
value: unit
type: counter
unit: request
description: "subgraph requests containing subgraph timeout"
attributes:
subgraph.name: true
condition:
eq:
- "request timed out"
- error: reason
```

And thanks to the event support for selectors you'll be able to fetch data directly from the supergraph response body:

```yaml
telemetry:
instrumentation:
instruments:
acme.request.on_graphql_error:
value: event_unit
type: counter
unit: error
description: my description
condition:
eq:
- MY_ERROR_CODE
- response_errors: "$.[0].extensions.code"
attributes:
response_errors:
response_errors: "$.*"
```

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/5022
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,17 @@ expression: "&schema"
}
]
},
"ErrorRepr": {
"oneOf": [
{
"description": "The error reason",
"enum": [
"reason"
],
"type": "string"
}
]
},
"ErrorsConfiguration": {
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -1928,6 +1939,13 @@ expression: "&schema"
],
"type": "string"
},
{
"description": "Log the event on every chunks in the response",
"enum": [
"event_response"
],
"type": "string"
},
{
"description": "Log the event on error",
"enum": [
Expand Down Expand Up @@ -1968,6 +1986,38 @@ expression: "&schema"
],
"type": "object"
},
"Event_for_RouterSelector": {
"oneOf": [
{
"description": "For every chunk sent through http multipart connection",
Copy link
Contributor

Choose a reason for hiding this comment

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

This throws me off a bit, intuitively I read that as multipart file upload chunks. Also does this actually apply only to http multipart /responses/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm not really, I don't know how to explain it better because it's highly related to our internals. It's basically event coming in the supergraph response stream. Maybe a better description could be For every supergraph response payload (including subscription events and defer events) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @shorgi WDYT?

I'm going to preapprove the PR and let Edward give his opinion / suggest something :D

Copy link
Contributor

Choose a reason for hiding this comment

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

@bnjjj I like your updated suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated with my suggestion and enable auto merge in order to have this in the next release. Once we have feedback from @shorgi I'll open a new PR to change the description

"enum": [
"event_duration"
],
"type": "string"
},
{
"description": "For every chunk sent through http multipart connection",
"enum": [
"event_unit"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "For every chunk sent through http multipart connection",
"properties": {
"event_custom": {
"$ref": "#/definitions/RouterSelector",
"description": "#/definitions/RouterSelector"
}
},
"required": [
"event_custom"
],
"type": "object"
}
]
},
"Event_for_SubgraphAttributes_and_SubgraphSelector": {
"description": "An event that can be logged as part of a trace. The event has an implicit `type` attribute that matches the name of the event in the yaml and a message that can be used to provide additional information.",
"properties": {
Expand Down Expand Up @@ -1999,6 +2049,38 @@ expression: "&schema"
],
"type": "object"
},
"Event_for_SubgraphSelector": {
"oneOf": [
{
"description": "For every chunk sent through http multipart connection",
"enum": [
"event_duration"
],
"type": "string"
},
{
"description": "For every chunk sent through http multipart connection",
"enum": [
"event_unit"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "For every chunk sent through http multipart connection",
"properties": {
"event_custom": {
"$ref": "#/definitions/SubgraphSelector",
"description": "#/definitions/SubgraphSelector"
}
},
"required": [
"event_custom"
],
"type": "object"
}
]
},
"Event_for_SupergraphAttributes_and_SupergraphSelector": {
"description": "An event that can be logged as part of a trace. The event has an implicit `type` attribute that matches the name of the event in the yaml and a message that can be used to provide additional information.",
"properties": {
Expand Down Expand Up @@ -2030,6 +2112,38 @@ expression: "&schema"
],
"type": "object"
},
"Event_for_SupergraphSelector": {
"oneOf": [
{
"description": "For every chunk sent through http multipart connection",
"enum": [
"event_duration"
],
"type": "string"
},
{
"description": "For every chunk sent through http multipart connection",
"enum": [
"event_unit"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "For every chunk sent through http multipart connection",
"properties": {
"event_custom": {
"$ref": "#/definitions/SupergraphSelector",
"description": "#/definitions/SupergraphSelector"
}
},
"required": [
"event_custom"
],
"type": "object"
}
]
},
"Events": {
"additionalProperties": false,
"description": "Events are",
Expand Down Expand Up @@ -2744,6 +2858,10 @@ expression: "&schema"
"$ref": "#/definitions/Standard",
"description": "#/definitions/Standard"
},
{
"$ref": "#/definitions/Event_for_RouterSelector",
"description": "#/definitions/Event_for_RouterSelector"
},
{
"$ref": "#/definitions/RouterSelector",
"description": "#/definitions/RouterSelector"
Expand All @@ -2756,6 +2874,10 @@ expression: "&schema"
"$ref": "#/definitions/Standard",
"description": "#/definitions/Standard"
},
{
"$ref": "#/definitions/Event_for_SubgraphSelector",
"description": "#/definitions/Event_for_SubgraphSelector"
},
{
"$ref": "#/definitions/SubgraphSelector",
"description": "#/definitions/SubgraphSelector"
Expand All @@ -2768,6 +2890,10 @@ expression: "&schema"
"$ref": "#/definitions/Standard",
"description": "#/definitions/Standard"
},
{
"$ref": "#/definitions/Event_for_SupergraphSelector",
"description": "#/definitions/Event_for_SupergraphSelector"
},
{
"$ref": "#/definitions/SupergraphSelector",
"description": "#/definitions/SupergraphSelector"
Expand Down Expand Up @@ -4393,6 +4519,19 @@ expression: "&schema"
"on_graphql_error"
],
"type": "object"
},
{
"additionalProperties": false,
"properties": {
"error": {
"$ref": "#/definitions/ErrorRepr",
"description": "#/definitions/ErrorRepr"
}
},
"required": [
"error"
],
"type": "object"
}
]
},
Expand Down Expand Up @@ -5328,6 +5467,19 @@ expression: "&schema"
"static"
],
"type": "object"
},
{
"additionalProperties": false,
"properties": {
"error": {
"$ref": "#/definitions/ErrorRepr",
"description": "#/definitions/ErrorRepr"
}
},
"required": [
"error"
],
"type": "object"
}
]
},
Expand Down Expand Up @@ -5801,6 +5953,42 @@ expression: "&schema"
],
"type": "object"
},
{
"additionalProperties": false,
"properties": {
"default": {
"$ref": "#/definitions/AttributeValue",
"description": "#/definitions/AttributeValue",
"nullable": true
},
"response_data": {
"description": "The supergraph response body json path of the chunks.",
"type": "string"
}
},
"required": [
"response_data"
],
"type": "object"
},
{
"additionalProperties": false,
"properties": {
"default": {
"$ref": "#/definitions/AttributeValue",
"description": "#/definitions/AttributeValue",
"nullable": true
},
"response_errors": {
"description": "The supergraph response body json path of the chunks.",
"type": "string"
}
},
"required": [
"response_errors"
],
"type": "object"
},
{
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -5853,6 +6041,19 @@ expression: "&schema"
],
"type": "object"
},
{
"additionalProperties": false,
"properties": {
"error": {
"$ref": "#/definitions/ErrorRepr",
"description": "#/definitions/ErrorRepr"
}
},
"required": [
"error"
],
"type": "object"
},
{
"additionalProperties": false,
"description": "Cost attributes",
Expand Down
1 change: 1 addition & 0 deletions apollo-router/src/plugins/demand_control/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub(crate) mod cost_calculator;
pub(crate) mod strategy;

/// The cost calculation information stored in context for use in telemetry and other plugins that need to know what cost was calculated.
#[derive(Debug)]
pub(crate) struct CostContext {
pub(crate) estimated: f64,
pub(crate) actual: f64,
Expand Down
6 changes: 6 additions & 0 deletions apollo-router/src/plugins/telemetry/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,12 @@ pub(crate) enum AttributeValue {
Array(AttributeArray),
}

impl From<String> for AttributeValue {
fn from(value: String) -> Self {
AttributeValue::String(value)
}
}

impl From<&'static str> for AttributeValue {
fn from(value: &'static str) -> Self {
AttributeValue::String(value.to_string())
Expand Down