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

Telemetry enhancements #850

Merged
merged 22 commits into from
Apr 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

After the plugin utils etc stuff merges, you'll see the minimal style of documentation that is required. You can lift some of the documentation from that PR and use it to document the new components (such as this instrument layer/service) that you are adding.

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've added some minimal docs, I'm happy to revisit in a larger docs effort.

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