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

Entry point improvements #1227

Merged
merged 5 commits into from Jun 14, 2022
Merged

Entry point improvements #1227

merged 5 commits into from Jun 14, 2022

Conversation

BrynCooke
Copy link
Contributor

@BrynCooke BrynCooke commented Jun 13, 2022

ApolloRouterBuilder has been migrated to buildstructor for consistency with other code.
Calls to ApolloRouterBuilder::default() should be migrated to ApolloRouter::builder.
FederatedServerHandle has been renamed to ApolloRouterHandle.

Migration tips:

  • The ability to supply your own RouterServiceFactory has been removed.
  • StateListener. This made the internal state machine unnecessarily complex. listen_address() remains on ApolloRouterHandle.
  • FederatedServerHandle#shutdown() has been removed. Instead, dropping ApolloRouterHandle will cause the router to shutdown.
  • FederatedServerHandle#ready() has been renamed to FederatedServerHandle#listen_address(), it will return the address when the router is ready to serve requests.

@BrynCooke BrynCooke force-pushed the router-main-api branch 2 times, most recently from 63d3ce3 to 5811493 Compare June 13, 2022 21:50
`ApolloRouterBuilder` has been migrated to `buildstructor` for consistency with other code.
Calls to `ApolloRouterBuilder::default()` should be migrated to `ApolloRouter::builder`.
`FederatedServerHandle` has been renamed to `ApolloRouterHandle`.

Removed functionality:
* The ability to supply your own `RouterServiceFactory`. This may be added back if there is a concrete use case for it.
* `StateListener`. This made the internal state machine unnecessarily complex. `ready()` remains on `ApolloRouterHandle`.
* `ApolloRouterHandle#shutdown()` has been removed. Instead dropping `ApolloRouterHandle` will cause the router to shutdown.
Copy link
Contributor

@garypen garypen left a comment

Choose a reason for hiding this comment

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

Some comments and small suggestions for change.

apollo-router/src/router.rs Outdated Show resolved Hide resolved
apollo-router/src/state_machine.rs Outdated Show resolved Hide resolved
apollo-router/src/testdata/supergraph_config.yaml Outdated Show resolved Hide resolved
NEXT_CHANGELOG.md Outdated Show resolved Hide resolved
Rename ready to listen_address.
Copy link
Contributor

@garypen garypen left a comment

Choose a reason for hiding this comment

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

I like renaming ready() to listen_address(). Should also rename the variable.

apollo-router/src/router.rs Outdated Show resolved Hide resolved
apollo-router/src/router.rs Outdated Show resolved Hide resolved
@BrynCooke BrynCooke requested a review from garypen June 14, 2022 09:19
@BrynCooke BrynCooke merged commit ebe7a70 into api-1.0 Jun 14, 2022
@BrynCooke BrynCooke deleted the router-main-api branch June 14, 2022 10:06
@BrynCooke BrynCooke linked an issue Jun 14, 2022 that may be closed by this pull request
3 tasks
@BrynCooke BrynCooke self-assigned this Jun 14, 2022
@o0Ignition0o o0Ignition0o mentioned this pull request Jul 5, 2022
o0Ignition0o added a commit that referenced this pull request Jul 5, 2022
# [0.10.0] - 2022-07-05
## ❗ BREAKING ❗

### Change configuration for custom attributes for metrics in telemetry plugin ([PR #1300](#1300)

```diff
telemetry:
  metrics:
    common:
      attributes:
-        static:
-          - name: "version"
-            value: "v1.0.0"
-        from_headers:
-          - named: "content-type"
-            rename: "payload_type"
-            default: "application/json"
-          - named: "x-custom-header-to-add"
+        router:
+          static:
+            - name: "version"
+              value: "v1.0.0"
+          request:
+            header:
+              - named: "content-type"
+                rename: "payload_type"
+                default: "application/json"
+              - named: "x-custom-header-to-add"
```

By [@bnjjj](https://github.com/bnjjj) in #1300

### Rename http_compat to http_ext ([PR #1291](#1291)

The module provides extensions to the `http` crate which are specific to the way we use that crate in the router. This change also cleans up the provided extensions and fixes a few potential sources of error (by removing them)
such as the Request::mock() fn.

By [@garypen](https://github.com/garypen) in #1291

### Rework the entire public API structure ([PR #1216](#1216),  [PR #1242](#1242),  [PR #1267](#1267),  [PR #1277](#1277), [PR #1303](#1303))

* Many items have been removed from the public API and made private.
  If you still need some of them, please file an issue.

* Many reexports have been removed, 
  notably from the crate root and all of the `prelude` module.
  Corresponding items need to be imported from another location instead,
  usually the module that define them.

* Some items have moved and need to be imported from a different location.

For example, here are the changes made to `examples/add-timestamp-header/src/main.rs`:

```diff
-use apollo_router::{plugin::utils, Plugin, RouterRequest, RouterResponse};
+use apollo_router::plugin::test;
+use apollo_router::plugin::Plugin;
+use apollo_router::services::{RouterRequest, RouterResponse};
```
```diff
-let mut mock = utils::test::MockRouterService::new();
+let mut mock = test::MockRouterService::new();
```
```diff
-if let apollo_router::ResponseBody::GraphQL(response) =
+if let apollo_router::services::ResponseBody::GraphQL(response) =
     service_response.next_response().await.unwrap()
 {
```

If you’re unsure where a given item needs to be imported from when porting code,
unfold the listing below and use your browser’s search function (CTRL+F or ⌘+F).

<details>
<summary>
  Output of <code>./scripts/public_items.sh</code> for 0.10.0
</summary>
<pre>
use apollo_router::ApolloRouter;
use apollo_router::Configuration;
use apollo_router::ConfigurationKind;
use apollo_router::Context;
use apollo_router::Error;
use apollo_router::Executable;
use apollo_router::Request;
use apollo_router::Response;
use apollo_router::Schema;
use apollo_router::SchemaKind;
use apollo_router::ShutdownKind;
use apollo_router::error::CacheResolverError;
use apollo_router::error::FetchError;
use apollo_router::error::JsonExtError;
use apollo_router::error::Location;
use apollo_router::error::ParseErrors;
use apollo_router::error::PlannerErrors;
use apollo_router::error::QueryPlannerError;
use apollo_router::error::SchemaError;
use apollo_router::error::ServiceBuildError;
use apollo_router::error::SpecError;
use apollo_router::graphql::Error;
use apollo_router::graphql::NewErrorBuilder;
use apollo_router::graphql::Request;
use apollo_router::graphql::Response;
use apollo_router::json_ext::Object;
use apollo_router::json_ext::Path;
use apollo_router::json_ext::PathElement;
use apollo_router::layers::ServiceBuilderExt;
use apollo_router::layers::ServiceExt;
use apollo_router::layers::async_checkpoint::AsyncCheckpointLayer;
use apollo_router::layers::async_checkpoint::AsyncCheckpointService;
use apollo_router::layers::cache::CachingLayer;
use apollo_router::layers::cache::CachingService;
use apollo_router::layers::instrument::InstrumentLayer;
use apollo_router::layers::instrument::InstrumentService;
use apollo_router::layers::map_future_with_context::MapFutureWithContextLayer;
use apollo_router::layers::map_future_with_context::MapFutureWithContextService;
use apollo_router::layers::sync_checkpoint::CheckpointLayer;
use apollo_router::layers::sync_checkpoint::CheckpointService;
use apollo_router::main;
use apollo_router::mock_service;
use apollo_router::plugin::DynPlugin;
use apollo_router::plugin::Handler;
use apollo_router::plugin::Plugin;
use apollo_router::plugin::PluginFactory;
use apollo_router::plugin::plugins;
use apollo_router::plugin::register_plugin;
use apollo_router::plugin::serde::deserialize_header_name;
use apollo_router::plugin::serde::deserialize_header_value;
use apollo_router::plugin::serde::deserialize_option_header_name;
use apollo_router::plugin::serde::deserialize_option_header_value;
use apollo_router::plugin::serde::deserialize_regex;
use apollo_router::plugin::test::IntoSchema;
use apollo_router::plugin::test::MockExecutionService;
use apollo_router::plugin::test::MockQueryPlanningService;
use apollo_router::plugin::test::MockRouterService;
use apollo_router::plugin::test::MockSubgraph;
use apollo_router::plugin::test::MockSubgraphService;
use apollo_router::plugin::test::NewPluginTestHarnessBuilder;
use apollo_router::plugin::test::PluginTestHarness;
use apollo_router::plugins::csrf::CSRFConfig;
use apollo_router::plugins::csrf::Csrf;
use apollo_router::plugins::rhai::Conf;
use apollo_router::plugins::rhai::Rhai;
use apollo_router::plugins::telemetry::ROUTER_SPAN_NAME;
use apollo_router::plugins::telemetry::Telemetry;
use apollo_router::plugins::telemetry::apollo::Config;
use apollo_router::plugins::telemetry::config::AttributeArray;
use apollo_router::plugins::telemetry::config::AttributeValue;
use apollo_router::plugins::telemetry::config::Conf;
use apollo_router::plugins::telemetry::config::GenericWith;
use apollo_router::plugins::telemetry::config::Metrics;
use apollo_router::plugins::telemetry::config::MetricsCommon;
use apollo_router::plugins::telemetry::config::Propagation;
use apollo_router::plugins::telemetry::config::Sampler;
use apollo_router::plugins::telemetry::config::SamplerOption;
use apollo_router::plugins::telemetry::config::Trace;
use apollo_router::plugins::telemetry::config::Tracing;
use apollo_router::query_planner::OperationKind;
use apollo_router::query_planner::QueryPlan;
use apollo_router::query_planner::QueryPlanOptions;
use apollo_router::register_plugin;
use apollo_router::services::ErrorNewExecutionResponseBuilder;
use apollo_router::services::ErrorNewQueryPlannerResponseBuilder;
use apollo_router::services::ErrorNewRouterResponseBuilder;
use apollo_router::services::ErrorNewSubgraphResponseBuilder;
use apollo_router::services::ExecutionRequest;
use apollo_router::services::ExecutionResponse;
use apollo_router::services::ExecutionService;
use apollo_router::services::FakeNewExecutionRequestBuilder;
use apollo_router::services::FakeNewExecutionResponseBuilder;
use apollo_router::services::FakeNewRouterRequestBuilder;
use apollo_router::services::FakeNewRouterResponseBuilder;
use apollo_router::services::FakeNewSubgraphRequestBuilder;
use apollo_router::services::FakeNewSubgraphResponseBuilder;
use apollo_router::services::NewExecutionRequestBuilder;
use apollo_router::services::NewExecutionResponseBuilder;
use apollo_router::services::NewExecutionServiceBuilder;
use apollo_router::services::NewQueryPlannerRequestBuilder;
use apollo_router::services::NewQueryPlannerResponseBuilder;
use apollo_router::services::NewRouterRequestBuilder;
use apollo_router::services::NewRouterResponseBuilder;
use apollo_router::services::NewRouterServiceBuilder;
use apollo_router::services::NewSubgraphRequestBuilder;
use apollo_router::services::NewSubgraphResponseBuilder;
use apollo_router::services::PluggableRouterServiceBuilder;
use apollo_router::services::QueryPlannerContent;
use apollo_router::services::QueryPlannerRequest;
use apollo_router::services::QueryPlannerResponse;
use apollo_router::services::ResponseBody;
use apollo_router::services::RouterRequest;
use apollo_router::services::RouterResponse;
use apollo_router::services::RouterService;
use apollo_router::services::SubgraphRequest;
use apollo_router::services::SubgraphResponse;
use apollo_router::services::SubgraphService;
use apollo_router::services::http_ext::FakeNewRequestBuilder;
use apollo_router::services::http_ext::IntoHeaderName;
use apollo_router::services::http_ext::IntoHeaderValue;
use apollo_router::services::http_ext::NewRequestBuilder;
use apollo_router::services::http_ext::Request;
use apollo_router::services::http_ext::Response;
use apollo_router::subscriber::RouterSubscriber;
use apollo_router::subscriber::is_global_subscriber_set;
use apollo_router::subscriber::replace_layer;
use apollo_router::subscriber::set_global_subscriber;
</pre>
</details>

By [@SimonSapin](https://github.com/SimonSapin)

### Entry point improvements ([PR #1227](#1227)) ([PR #1234](#1234)) ([PR #1239](#1239)) ([PR #1263](#1263))

The interfaces around the entry point have been improved for naming consistency and to enable reuse when customization is required. 
Most users will continue to use:
```rust
apollo_router::main()  
```

However, if you want to specify extra customization to configuration/schema/shutdown then you may use `Executable::builder()` to override behavior. 

```rust
use apollo_router::Executable;
Executable::builder()
  .router_builder_fn(|configuration, schema| ...) // Optional
  .start().await?
```

Migration tips:
* Calls to `ApolloRouterBuilder::default()` should be migrated to `ApolloRouter::builder`.
* `FederatedServerHandle` has been renamed to `ApolloRouterHandle`.
* The ability to supply your own `RouterServiceFactory` has been removed.
* `StateListener`. This made the internal state machine unnecessarily complex. `listen_address()` remains on `ApolloRouterHandle`.
* `FederatedServerHandle::shutdown()` has been removed. Instead, dropping `ApolloRouterHandle` will cause the router to shutdown.
* `FederatedServerHandle::ready()` has been renamed to `FederatedServerHandle::listen_address()`, it will return the address when the router is ready to serve requests.
* `FederatedServerError` has been renamed to `ApolloRouterError`.
* `main_rt` should be migrated to `Executable::builder()`

By [@BrynCooke](https://github.com/bryncooke) in #1227 #1234 #1239 #1263

### Non-GraphQL response body variants removed from `RouterResponse` ([PR #1307](#1307), [PR #1328](#1328))

The `ResponseBody` enum has been removed.
It had variants for GraphQL and non-GraphQL responses.

It was used:

* In `RouterResponse` which now uses `apollo_router::graphql::Response` instead
* In `Handler` for plugin custom endpoints which now uses `bytes::Bytes` instead

Various type signatures will need changes such as:

```diff
- RouterResponse<BoxStream<'static, ResponseBody>>
+ RouterResponse<BoxStream<'static, graphql::Response>>
```

Necessary code changes might look like:

```diff
- return ResponseBody::GraphQL(response);
+ return response;
```
```diff
- if let ResponseBody::GraphQL(graphql_response) = res {
-     assert_eq!(&graphql_response.errors[0], expected_error);
- } else {
-     panic!("expected a graphql response");
- }
+ assert_eq!(&res.errors[0], expected_error);
```

By [@SimonSapin](https://github.com/SimonSapin)

### Fixed control flow in helm chart for volume mounts & environment variables ([PR #1283](#1283))

You will now be able to actually use the helm chart without being on a managed graph. 

By [@LockedThread](https://github.com/LockedThread) in #1283


### Deny unknown fields on configuration [PR #1278](#1278)
Do not silently skip some bad configuration, now if you add an unknown configuration field at the root of your configuration file it will return an error.

By [@bnjjj](https://github.com/bnjjj) in #1278

## 🚀 Features ( 🚀 )

### Add support to add custom attributes on metrics. [PR #1300](#1300)

Previously, it was only possible to add custom attributes coming from headers from router request. Now you are able to add custom attributes coming from headers and body of router response/request and subgraph response/request. You also have the ability to add an attribute coming from the context. Example:

```yaml
telemetry:
  metrics:
    common:
      attributes:
        router:
          static:
            - name: "version"
              value: "v1.0.0"
          request:
            header:
              - named: "content-type"
                rename: "payload_type"
                default: "application/json"
              - named: "x-custom-header-to-add"
          response:
            body:
              # Take element from the response body of the router located at this path
              - path: .errors[0].extensions.status
                name: error_from_body
          context:
            # Take element from context in plugin chains and add it in attributes
            - named: my_key
        subgraph:
          all:
            static:
              # Always insert on all metrics for all subgraphs
              - name: kind
                value: subgraph_request
          subgraphs:
            my_subgraph_name: # Apply these rules only for the subgraph named `my_subgraph_name`
              request:
                header:
                  - named: "x-custom-header"
                body:
                  # Take element from the request body of the router located at this path (here it's the query)
                  - path: .query
                    name: query
                    default: UNKNOWN
```

By [@bnjjj](https://github.com/bnjjj) in #1300

### Add support for modifying variables from a plugin. [PR #1257](#1257)

Previously, it was not possible to modify variables in a `Request` from a plugin. This is now supported in both Rust and Rhai plugins.

By [@garypen](https://github.com/garypen) in #1257

## 🐛 Fixes


### extend fix for compression support to DIY dockerfiles ([PR #1352](#1352))

Compression support is now shown in the DIY dockerfiles (followup to [PR #1279](#1279)).

By [@garypen](https://github.com/garypen) in #1352

### Improve URL parsing in endpoint configuration ([PR #1341](#1341))

Specifying an endpoint in this form '127.0.0.1:431' resulted in an error: 'relative URL without a base'. The fix enhances the URL parsing logic to check for these errors and re-parses with a default scheme 'http://' so that parsing succeeds.

By [@garypen](https://github.com/garypen) in #1341

### Improve configuration validation and environment expansion ([PR #1331](#1331))

Environment expansion now covers the entire configuration file, and supports non string types.
This means that it is now possible to use environment variable in the server section of the yaml, and also in numeric and boolean fields.

Environment variables will always be shown in their original form in error messages preventing leakage of secrets.

These changes allow more of the configuration file to be validated via json schema, as previously we just skipped errors where fields contained env variables.

By [@BrynCooke](https://github.com/bryncooke) in #1331

### Fix input coercion for a list ([PR #1327](#1327))

The router is now following coercion rules for List regarding [the specs](https://spec.graphql.org/June2018/#sec-Type-System.List). Especially it fixes the case when for an input type `[Int]` only `1` was provided as a value. It's now working and it's coerced to `[1]`.

By [@bnjjj](https://github.com/bnjjj) in #1327

### Returns HTTP 400 bad request instead of 500 when it's a query plan error ([PR #1321](#1321))

By [@bnjjj](https://github.com/bnjjj) in #1321

### Re-enable the subgraph error redaction functionality ([PR #1317](#1317))

In a re-factoring the "include_subgraph_errors" plugin was disabled. This meant that subgraph error handling was not working as intended. This change re-enables it and improves the functionality with additional logging. As part of the fix, the plugin initialisation mechanism was improved to ensure that plugins start in the required sequence.

By [@garypen](https://github.com/garypen) in #1317

### Restrict static introspection to only `__schema` and `__type` ([PR #1299](#1299))
Queries with selected field names starting with `__` are recognized as introspection queries. This includes `__schema`, `__type` and `__typename`. However, `__typename` is introspection at query time which is different from `__schema` and `__type` because two of the later can be answered with queries with empty input variables. This change will restrict introspection to only `__schema` and `__type`.

By [@dingxiangfei2009](https://github.com/dingxiangfei2009) in #1299

### Fix scaffold support ([PR #1293](#1293))

By [@garypen](https://github.com/garypen) in #1293

### Support introspection object types ([PR #1240](#1240))

Introspection queries can use a set of object types defined in the specification. The query parsing code was not recognizing them,
resulting in some introspection queries not working.

By [@Geal](https://github.com/Geal) in #1240

### Update the scaffold template so it works with streams ([#1247](#1247))

Release v0.9.4 changed the way we deal with Response objects, which can now be streams.
This Pull request updates the scaffold template so it generates plugins that are compatible with the new Plugin API.

By [@o0Ignition0o](https://github.com/o0Ignition0o) in #1248


### Fix fragment selection on interfaces ([PR #1295](#1295))

Fragments type conditions were not checked correctly on interfaces, resulting in invalid null fields added to the response
or valid data being nullified.

By [@Geal](https://github.com/Geal) in #1295

### Fix fragment selection on queries ([PR #1296](#1296))

The schema object can specify objects for queries, mutations or subscriptions that are not named `Query`, `Mutation` or
`Subscription`. Response formatting now supports it.

By [@Geal](https://github.com/Geal) in #1296

### Fix fragment selection on unions ([PR #1346](#1346))

Fragments type conditions were not checked correctly on unions, resulting in data being absent.

By [@Geal](https://github.com/Geal) in #1346

### Reduce `poll_ready` calls in query deduplication ([PR #1350](#1350))

The query deduplication service was making assumptions on the underlying service's behaviour, which could result in
subgraph services panicking.

By [@Geal](https://github.com/Geal) in #1350

## 🛠 Maintenance ( 🛠️ )

### chore: Run scaffold tests in CI and xtask only ([PR #1345](#1345))

Run the scaffold tests in CI and through xtask, to keep a steady feedback loop while developping against `cargo test`.

By [@o0Ignition0o](https://github.com/o0Ignition0o) in #1345

### Update rhai to latest release (1.8.0)  ([PR #1337](#1337)

We had been depending on a pinned git version which had a fix we required. This now updates to the latest release.

By [@garypen](https://github.com/garypen) in #1337

### Remove typed-builder ([PR #1218](#1218))
Migrate all typed-builders code to buildstructor
By [@BrynCooke](https://github.com/bryncooke) in #1218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API 1.0 - ApolloRouterBuilder revisit
3 participants