Skip to content

Commit

Permalink
Added Apollo Tracing support for @defer. (#2190)
Browse files Browse the repository at this point in the history
You can now view traces in Apollo Studio as normal.
Also improved testing and fixed missing variables in Apollo tracing.
Fixes #1600 #2186
 

<!--
First, 🌠 thank you 🌠 for considering a contribution to Apollo!

Some of this information is also included in the /CONTRIBUTING.md file
at the
root of this repository.  We suggest you read it!

  https://github.com/apollographql/router/blob/HEAD/CONTRIBUTING.md

Here are some important details to keep in mind:

* ⏰ Your time is important
To save your precious time, if the contribution you are making will
take more than an hour, please make sure it has been discussed in an
        issue first. This is especially true for feature requests!

* 💡 Features
Feature requests can be created and discussed within a GitHub Issue.
Be sure to search for existing feature requests (and related issues!)
prior to opening a new request. If an existing issue covers the need,
please upvote that issue by using the 👍 emote, rather than opening a
        new issue.

* 🕷 Bug fixes
These can be created and discussed in this repository. When fixing a
bug,
please _try_ to add a test which verifies the fix. If you cannot, you
should
still submit the PR but we may still ask you (and help you!) to create a
test.

* 📖 Contribution guidelines
Follow https://github.com/apollographql/router/blob/HEAD/CONTRIBUTING.md
when submitting a pull request. Make sure existing tests still pass, and
add
        tests for all new behavior.

* ✏️ Explain your pull request
Describe the big picture of your changes here to communicate to what
        your pull request is meant to accomplish. Provide 🔗 links 🔗 to
associated issues! Documentation in the docs/ directory should be
updated
        as necessary.  Finally, a /CHANGELOG.md entry should be added.

We hope you will find this to be a positive experience! Contribution can
be
intimidating and we hope to alleviate that pain as much as possible.
Without
following these guidelines, you may be missing context that can help you
succeed
with your contribution, which is why we encourage discussion first.
Ultimately,
there is no guarantee that we will be able to merge your pull-request,
but by
following these guidelines we can try to avoid disappointment.

-->

Co-authored-by: bryn <[email protected]>
  • Loading branch information
BrynCooke and bryn authored Dec 2, 2022
1 parent fda9f15 commit 728552f
Show file tree
Hide file tree
Showing 17 changed files with 5,370 additions and 419 deletions.
18 changes: 18 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ supergraph:
By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/2155
### `@defer` Apollo tracing support ([Issue #1600](https://github.com/apollographql/router/issues/1600))

Added Apollo tracing support for queries that use `@defer`. You can now view traces in Apollo Studio as normal.

By [@bryncooke](https://github.com/bryncooke) in https://github.com/apollographql/router/pull/2190

## 🐛 Fixes

### Fix panic when dev mode enabled with empty config file ([Issue #2182](https://github.com/apollographql/router/issues/2182))
Expand All @@ -246,6 +252,18 @@ If you're running the Router with dev mode with an empty config file, it will no

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/2165

### Fix missing apollo tracing variables ([Issue #2186](https://github.com/apollographql/router/issues/2186))

Send variable values had no effect. This is now fixed.
```yaml
telemetry:
apollo:
send_variable_values: all
```

By [@bryncooke](https://github.com/bryncooke) in https://github.com/apollographql/router/pull/2190


### fix build_docker_image.sh script when using default repo ([PR #2163](https://github.com/apollographql/router/pull/2163))

Adding the `-r` flag recently broke the existing functionality to build from the default repo using `-b`. This fixes that.
Expand Down
2 changes: 1 addition & 1 deletion apollo-router/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ uname = "0.1.1"
uname = "0.1.1"

[dev-dependencies]
insta = { version = "1.21.2", features = ["json", "redactions"] }
insta = { version = "1.21.2", features = ["json", "redactions", "yaml"] }
introspector-gadget = "0.1.0"
maplit = "1.0.2"
memchr = { version = "2.5.0", default-features = false }
Expand Down
14 changes: 12 additions & 2 deletions apollo-router/src/plugins/telemetry/apollo_exporter.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
//! Configuration for apollo telemetry exporter.
#[cfg(test)]
use std::sync::Arc;
#[cfg(test)]
use std::sync::Mutex;
// This entire file is license key functionality
use std::time::Duration;

Expand Down Expand Up @@ -31,20 +35,26 @@ pub(crate) const POOL_TIMEOUT: Duration = Duration::from_secs(5);
pub(crate) enum Sender {
Noop,
Spaceport(mpsc::Sender<SingleReport>),
#[cfg(test)]
InMemory(Arc<Mutex<Vec<SingleReport>>>),
}

impl Sender {
pub(crate) fn send(&self, metrics: SingleReport) {
pub(crate) fn send(&self, report: SingleReport) {
match &self {
Sender::Noop => {}
Sender::Spaceport(channel) => {
if let Err(err) = channel.to_owned().try_send(metrics) {
if let Err(err) = channel.to_owned().try_send(report) {
tracing::warn!(
"could not send metrics to spaceport, metric will be dropped: {}",
err
);
}
}
#[cfg(test)]
Sender::InMemory(storage) => {
storage.lock().expect("mutex poisoned").push(report);
}
}
}
}
Expand Down
39 changes: 6 additions & 33 deletions apollo-router/src/plugins/telemetry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ const CLIENT_VERSION: &str = "apollo_telemetry::client_version";
const ATTRIBUTES: &str = "apollo_telemetry::metrics_attributes";
const SUBGRAPH_ATTRIBUTES: &str = "apollo_telemetry::subgraph_metrics_attributes";
pub(crate) const STUDIO_EXCLUDE: &str = "apollo_telemetry::studio::exclude";
pub(crate) const FTV1_DO_NOT_SAMPLE: &str = "apollo_telemetry::studio::ftv1_do_not_sample";
pub(crate) const LOGGING_DISPLAY_HEADERS: &str = "apollo_telemetry::logging::display_headers";
pub(crate) const LOGGING_DISPLAY_BODY: &str = "apollo_telemetry::logging::display_body";
const DEFAULT_SERVICE_NAME: &str = "apollo-router";
Expand Down Expand Up @@ -280,31 +279,9 @@ impl Plugin for Telemetry {

fn execution_service(&self, service: execution::BoxService) -> execution::BoxService {
ServiceBuilder::new()
.instrument(move |req: &ExecutionRequest| {
// disable ftv1 sampling for deferred queries
let do_not_sample_reason = if req.query_plan.root.contains_condition_or_defer() {
req.context.insert(FTV1_DO_NOT_SAMPLE, true).unwrap();
"query is deferred"
} else {
""
};
let query = req
.supergraph_request
.body()
.query
.clone()
.unwrap_or_default();
let operation_name = req
.supergraph_request
.body()
.operation_name
.clone()
.unwrap_or_default();
.instrument(move |_req: &ExecutionRequest| {
info_span!("execution",
graphql.document = query.as_str(),
graphql.operation.name = operation_name.as_str(),
"otel.kind" = %SpanKind::Internal,
ftv1.do_not_sample_reason = do_not_sample_reason
)
})
.service(service)
Expand Down Expand Up @@ -707,7 +684,7 @@ impl Telemetry {
apollo_private.http.request_headers = field::Empty
);

if is_span_sampled(&request.context) {
if is_span_sampled() {
span.record(
"apollo_private.graphql.variables",
&Self::filter_variables_values(
Expand Down Expand Up @@ -792,7 +769,7 @@ impl Telemetry {
}
})
.fold(BTreeMap::new(), |mut acc, (name, value)| {
acc.entry(name).or_insert_with(Vec::new).push(value);
acc.insert(name, value);
acc
});

Expand Down Expand Up @@ -1185,7 +1162,7 @@ impl Telemetry {
has_errors: bool,
duration: Duration,
) {
if is_span_sampled(context) {
if is_span_sampled() {
::tracing::trace!("span is sampled then skip the apollo metrics");
return;
}
Expand Down Expand Up @@ -1294,12 +1271,8 @@ fn handle_error<T: Into<opentelemetry::global::Error>>(err: T) {
}

#[inline]
pub(crate) fn is_span_sampled(context: &Context) -> bool {
pub(crate) fn is_span_sampled() -> bool {
Span::current().context().span().span_context().is_sampled()
&& !context
.get(FTV1_DO_NOT_SAMPLE)
.unwrap_or_default()
.unwrap_or(false)
}

register_plugin!("apollo", "telemetry", Telemetry);
Expand All @@ -1315,7 +1288,7 @@ enum ApolloFtv1Handler {
impl ApolloFtv1Handler {
fn request_ftv1(&self, mut req: SubgraphRequest) -> SubgraphRequest {
if let ApolloFtv1Handler::Enabled = self {
if is_span_sampled(&req.context) {
if is_span_sampled() {
req.subgraph_request.headers_mut().insert(
"apollo-federation-include-trace",
HeaderValue::from_static("ftv1"),
Expand Down
3 changes: 1 addition & 2 deletions apollo-router/src/plugins/telemetry/tracing/apollo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::plugins::telemetry::tracing::TracingConfigurator;
use crate::spaceport::Trace;

impl TracingConfigurator for Config {
fn apply(&self, builder: Builder, trace_config: &config::Trace) -> Result<Builder, BoxError> {
fn apply(&self, builder: Builder, _trace_config: &config::Trace) -> Result<Builder, BoxError> {
tracing::debug!("configuring Apollo tracing");
Ok(match self {
Config {
Expand All @@ -28,7 +28,6 @@ impl TracingConfigurator for Config {

let exporter = apollo_telemetry::Exporter::builder()
.expose_trace_id_config(expose_trace_id.clone())
.trace_config(trace_config.clone())
.endpoint(endpoint.clone())
.apollo_key(key)
.apollo_graph_ref(reference)
Expand Down
Loading

0 comments on commit 728552f

Please sign in to comment.