Skip to content

Commit

Permalink
Telemetry enhancements (#850)
Browse files Browse the repository at this point in the history
* Move the apollo config, update changelog

* Split configuration for Apollo so that tracing can go in the tracing module and metrics can go in the metrics module.

* Introduce instrument layer
Move router_request span to telemetry plugin. There's no way to disable this plugin so it's OK.
Add logic for dynamic client name and client version header.

* Print errors if config could not be reloaded.

* Update test config files

* Integration test for jaeger

* Fix build for windows and osx.
OSX had an issue with max UDP packet size
Windows has all sorts of file related issues. On the plus side the special handling for windows in circle has been removed as the root cause of the hang on windows is now known to be: rust-lang/rust#45572

* Add logic to kill node processes on test termination

* Remove all piping from external commands, it's not safe unless we read via another thread (which we don't)

* Removed instructions for developing without docker compose. It is only going to get more complicated aw we add more integration tests. Docker compose is the way to go.

* Router span name moved to constant.


Co-authored-by: bryn <bryn@apollographql.com>
Co-authored-by: Gary Pennington <gary@apollographql.com>
  • Loading branch information
3 people committed Apr 22, 2022
1 parent a4b78d3 commit 9a62ca1
Show file tree
Hide file tree
Showing 72 changed files with 1,225 additions and 700 deletions.
57 changes: 29 additions & 28 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,33 @@ commands:
command: |
sudo apt-get update
sudo apt-get install -y libssl-dev cmake
- run:
name: Download jaeger
command: |
curl -L https://github.com/jaegertracing/jaeger/releases/download/v1.33.0/jaeger-1.33.0-linux-amd64.tar.gz --output jaeger.tar.gz
tar -xf jaeger.tar.gz
mv jaeger-1.33.0-linux-amd64 jaeger
macos_install_baseline:
steps:
- run: echo "HOMEBREW_NO_AUTO_UPDATE=1" >> $BASH_ENV
- run: echo "export OPENSSL_ROOT_DIR=/usr/local/opt/openssl@1.1" >> $BASH_ENV
- run: test -e "$OPENSSL_ROOT_DIR"
- run: brew install cmake
- run:
name: Download jaeger
command: |
curl -L https://github.com/jaegertracing/jaeger/releases/download/v1.33.0/jaeger-1.33.0-darwin-amd64.tar.gz --output jaeger.tar.gz
tar -xf jaeger.tar.gz
mv jaeger-1.33.0-darwin-amd64 jaeger
windows_install_baseline:
steps:
- run:
name: Download jaeger
shell: bash.exe
command: |
curl -L https://github.com/jaegertracing/jaeger/releases/download/v1.33.0/jaeger-1.33.0-windows-amd64.tar.gz --output jaeger.tar.gz
tar -xf jaeger.tar.gz
mv jaeger-1.33.0-windows-amd64 jaeger
# This job sets up our nodejs dependencies,
# and makes sure everything is ready to run integration tests
Expand All @@ -67,6 +88,12 @@ commands:
- run:
name: Assert npm version
command: test "$(npm --version)" = "${NPM_VERSION}"
- run:
# The jaeger exporter won't work without this
name: Increase udp packet size
command: |
sudo sysctl net.inet.udp.maxdgram=65536
sudo sysctl net.inet.udp.maxdgram
linux_prepare_node_env:
steps:
#TODO[igni]: check for node version before we try to install it
Expand Down Expand Up @@ -122,33 +149,6 @@ commands:
command: |
if ((npm --version) -Ne "${Env:NPM_VERSION}") { exit 1 }
windows_prepare_test_env:
steps:
- restore_cache:
keys:
- subgraph-node-modules-v2-windows-{{ checksum "dockerfiles/federation-demo/federation-demo/package.json" }}-{{ checksum "dockerfiles/federation-demo/federation-demo/package-lock.json" }}
- subgraph-node-modules-v2-windows-{{ checksum "dockerfiles/federation-demo/federation-demo/package.json" }}
- run:
name: npm clean-install
working_directory: dockerfiles/federation-demo/federation-demo
command: npm clean-install
- save_cache:
key: subgraph-node-modules-v2-windows-{{ checksum "dockerfiles/federation-demo/federation-demo/package.json" }}-{{ checksum "dockerfiles/federation-demo/federation-demo/package-lock.json" }}
paths:
- dockerfiles/federation-demo/federation-demo/node_modules

# TODO: normally xtask can run the federation by itself and it
# works on GitHub Actions on Windows. Unfortunately it
# doesn't work here on CircleCI on Windows only.
- run:
name: start federation-demo (background)
working_directory: dockerfiles/federation-demo/federation-demo
command: npm start
background: true
- run:
name: wait for federation demo to start
command: npx wait-on tcp:4001 tcp:4002 tcp:4003 tcp:4004 tcp:4100

windows_prepare_rust_env:
steps:
- run:
Expand Down Expand Up @@ -480,6 +480,7 @@ jobs:
condition:
equal: [*rust_windows_executor, << parameters.platform >>]
steps:
- windows_install_baseline
- windows_prepare_node_env
- windows_prepare_rust_env
- windows_build_workspace
Expand Down Expand Up @@ -516,8 +517,8 @@ jobs:
condition:
equal: [*rust_windows_executor, << parameters.platform >>]
steps:
- windows_install_baseline
- windows_prepare_node_env
- windows_prepare_test_env
- windows_prepare_rust_env
- windows_test_workspace
- when:
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ crates/*/Cargo.lock
# Docs
/docs/node_modules/
/docs/.cache/
/docs/public

# Dynamically retrieve proto file
apollo-spaceport/proto/reports.proto
#
# Prevent accidental commits of router.yaml in the root
/router.yaml
/jaeger/
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 6 additions & 20 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,14 @@ Use `cargo build --all-targets` to build the project.
Some tests run against the existing Node.js implementation of the Apollo Router. This
requires that the `federation-demo` project is running:

* If you have Docker and Docker Compose installed:

```
docker-compose up -d
```

**Note:** `-d` is for running into background. You can remove `-d` if you
have issues and you want to see the logs or if you want to run the service
in foreground.

* Otherwise:
```
docker-compose up -d
```

You will need Node.js and npm to be installed. It is known to be working on
Node.js 16 and npm 7.18.
**Note:** `-d` is for running into background. You can remove `-d` if you
have issues and you want to see the logs or if you want to run the service
in foreground.

```shell
git submodule sync --recursive
git submodule update --recursive --init
cd dockerfiles/federation-demo/federation-demo
npm install;
npm run start
```

### Run Apollo Router against the docker-compose or Node.js setup

Expand Down
20 changes: 20 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,30 @@ In addition, the `activate` lifecycle hook is now not marked as deprecated, and

## 🚀 Features

### Configurable client identification headers [PR #850](https://github.com/apollographql/router/pull/850)
The router uses the HTTP headers `apollographql-client-name` and `apollographql-client-version` to identify clients in Studio telemetry. Those headers can now be overriden in the configuration:
```yaml title="router.yaml"
telemetry:
apollo:
# Header identifying the client name. defaults to apollographql-client-name
client_name_header: <custom_client_header_name>
# Header identifying the client version. defaults to apollographql-client-version
client_version_header: <custom_version_header_name>
```
## 🐛 Fixes
### Configuration errors on hot-reload are output [PR #850](https://github.com/apollographql/router/pull/850)
If a configuration file had errors on reload these were silently swallowed. These are now added to the logs.
## 🛠 Maintenance
### End to end integration tests for Jaeger [PR #850](https://github.com/apollographql/router/pull/850)
Jaeger tracing end to end test including client->router->subgraphs
### Router tracing span cleanup [PR #850](https://github.com/apollographql/router/pull/850)
Spans generated by the Router are now aligned with plugin services.
### Simplified CI for windows [PR #850](https://github.com/apollographql/router/pull/850)
All windows processes are spawned via xtask rather than a separate CircleCI stage.
## 📚 Documentation
### Enhanced rust docs ([PR #819](https://github.com/apollographql/router/pull/819))
Many more rust docs have been added.
94 changes: 94 additions & 0 deletions apollo-router-core/src/layers/instrument.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
//! Instrumentation layer that allows services to be wrapped in a span.
//!
//! See [`Layer`] and [`Service`] for more details.
//!
//! Using ServiceBuilderExt:
//! ```rust
//! # use tower::ServiceBuilder;
//! # use tower_service::Service;
//! # use tracing::info_span;
//! # use apollo_router_core::ServiceBuilderExt;
//! # fn test<T>(service: impl Service<T>) {
//! let instrumented = ServiceBuilder::new()
//! .instrument(|_request| info_span!("query_planning"))
//! .service(service);
//! # }
//! ```
//! Now calls to the wrapped service will be wrapped in a span. You can attach attributes to the span from the request.
//!

use futures::future::BoxFuture;
use futures::FutureExt;
use std::marker::PhantomData;
use std::task::{Context, Poll};
use tower::Layer;
use tower_service::Service;
use tracing::Instrument;

/// [`Layer`] for instrumentation.
pub struct InstrumentLayer<F, Request>
where
F: Fn(&Request) -> tracing::Span,
{
span_fn: F,
phantom: PhantomData<Request>,
}

impl<F, Request> InstrumentLayer<F, Request>
where
F: Fn(&Request) -> tracing::Span,
{
pub fn new(span_fn: F) -> InstrumentLayer<F, Request> {
Self {
span_fn,
phantom: Default::default(),
}
}
}

impl<F, S, Request> Layer<S> for InstrumentLayer<F, Request>
where
S: Service<Request>,
F: Fn(&Request) -> tracing::Span + Clone,
{
type Service = InstrumentService<F, S, Request>;

fn layer(&self, inner: S) -> Self::Service {
InstrumentService {
inner,
span_fn: self.span_fn.clone(),
phantom: Default::default(),
}
}
}

/// [`Service`] for instrumentation.
pub struct InstrumentService<F, S, Request>
where
S: Service<Request>,
F: Fn(&Request) -> tracing::Span,
{
inner: S,
span_fn: F,
phantom: PhantomData<Request>,
}

impl<F, S, Request> Service<Request> for InstrumentService<F, S, Request>
where
F: Fn(&Request) -> tracing::Span,
S: Service<Request>,
<S as Service<Request>>::Future: Send + 'static,
{
type Response = S::Response;
type Error = S::Error;
type Future = BoxFuture<'static, Result<Self::Response, Self::Error>>;

fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
self.inner.poll_ready(cx)
}

fn call(&mut self, req: Request) -> Self::Future {
let span = (self.span_fn)(&req);
self.inner.call(req).instrument(span).boxed()
}
}
1 change: 1 addition & 0 deletions apollo-router-core/src/layers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ pub mod cache;
pub mod deduplication;
pub mod ensure_query_presence;
pub mod forbid_http_get_mutations;
pub mod instrument;
Loading

0 comments on commit 9a62ca1

Please sign in to comment.