Skip to content

Commit

Permalink
Suggestions from PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
pubmodmatt committed Oct 4, 2024
1 parent 997cd2b commit 994d598
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 28 deletions.
10 changes: 10 additions & 0 deletions apollo-router/src/plugins/telemetry/config_new/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ const NETWORK_LOCAL_PORT: Key = Key::from_static_str("network.local.port");
const NETWORK_PEER_ADDRESS: Key = Key::from_static_str("network.peer.address");
const NETWORK_PEER_PORT: Key = Key::from_static_str("network.peer.port");

pub(super) const HTTP_REQUEST_HEADERS: Key = Key::from_static_str("http.request.headers");
pub(super) const HTTP_REQUEST_URI: Key = Key::from_static_str("http.request.uri");
pub(super) const HTTP_REQUEST_VERSION: Key = Key::from_static_str("http.request.version");
pub(super) const HTTP_REQUEST_BODY: Key = Key::from_static_str("http.request.body");

pub(super) const HTTP_RESPONSE_HEADERS: Key = Key::from_static_str("http.response.headers");
pub(super) const HTTP_RESPONSE_STATUS: Key = Key::from_static_str("http.response.status");
pub(super) const HTTP_RESPONSE_VERSION: Key = Key::from_static_str("http.response.version");
pub(super) const HTTP_RESPONSE_BODY: Key = Key::from_static_str("http.response.body");

#[derive(Deserialize, JsonSchema, Clone, Debug, Default, Copy)]
#[serde(deny_unknown_fields, rename_all = "snake_case")]
pub(crate) enum DefaultAttributeRequirementLevel {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
use opentelemetry_api::Key;
use opentelemetry_api::KeyValue;
use opentelemetry_semantic_conventions::trace::HTTP_REQUEST_METHOD;
use parking_lot::Mutex;
use schemars::JsonSchema;
use serde::Deserialize;
use tower::BoxError;

use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_BODY;
use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_HEADERS;
use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_URI;
use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_VERSION;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_BODY;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_HEADERS;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_STATUS;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_VERSION;
use crate::plugins::telemetry::config_new::connector::http::attributes::ConnectorHttpAttributes;
use crate::plugins::telemetry::config_new::connector::http::selectors::ConnectorHttpSelector;
use crate::plugins::telemetry::config_new::events::log_event;
Expand Down Expand Up @@ -105,25 +114,25 @@ impl Instrumented
let headers = request.http_request.headers();

attrs.push(KeyValue::new(
Key::from_static_str("http.request.headers"),
HTTP_REQUEST_HEADERS,
opentelemetry::Value::String(format!("{:?}", headers).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.method"),
HTTP_REQUEST_METHOD,
opentelemetry::Value::String(format!("{}", request.http_request.method()).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.uri"),
HTTP_REQUEST_URI,
opentelemetry::Value::String(format!("{}", request.http_request.uri()).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.version"),
HTTP_REQUEST_VERSION,
opentelemetry::Value::String(
format!("{:?}", request.http_request.version()).into(),
),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.body"),
HTTP_REQUEST_BODY,
opentelemetry::Value::String(format!("{:?}", request.http_request.body()).into()),
));
log_event(self.request.level(), "connector.http.request", attrs, "");
Expand Down Expand Up @@ -158,21 +167,21 @@ impl Instrumented
let headers = response.http_response.headers();

attrs.push(KeyValue::new(
Key::from_static_str("http.response.headers"),
HTTP_RESPONSE_HEADERS,
opentelemetry::Value::String(format!("{:?}", headers).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.response.status"),
HTTP_RESPONSE_STATUS,
opentelemetry::Value::String(format!("{}", response.http_response.status()).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.response.version"),
HTTP_RESPONSE_VERSION,
opentelemetry::Value::String(
format!("{:?}", response.http_response.version()).into(),
),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.response.body"),
HTTP_RESPONSE_BODY,
opentelemetry::Value::String(format!("{:?}", response.http_response.body()).into()),
));
log_event(self.response.level(), "connector.http.response", attrs, "");
Expand Down
37 changes: 23 additions & 14 deletions apollo-router/src/plugins/telemetry/config_new/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::sync::Arc;
use http::HeaderValue;
use opentelemetry::Key;
use opentelemetry::KeyValue;
use opentelemetry_semantic_conventions::trace::HTTP_REQUEST_METHOD;
use parking_lot::Mutex;
use schemars::JsonSchema;
use serde::Deserialize;
Expand All @@ -19,6 +20,14 @@ use super::Stage;
use crate::plugins::telemetry::config_new::attributes::RouterAttributes;
use crate::plugins::telemetry::config_new::attributes::SubgraphAttributes;
use crate::plugins::telemetry::config_new::attributes::SupergraphAttributes;
use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_BODY;
use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_HEADERS;
use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_URI;
use crate::plugins::telemetry::config_new::attributes::HTTP_REQUEST_VERSION;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_BODY;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_HEADERS;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_STATUS;
use crate::plugins::telemetry::config_new::attributes::HTTP_RESPONSE_VERSION;
use crate::plugins::telemetry::config_new::conditions::Condition;
use crate::plugins::telemetry::config_new::connector::events::ConnectorEventsKind;
use crate::plugins::telemetry::config_new::connector::http::events::ConnectorHttpEvents;
Expand Down Expand Up @@ -255,25 +264,25 @@ impl Instrumented
let headers = request.router_request.headers();

attrs.push(KeyValue::new(
Key::from_static_str("http.request.headers"),
HTTP_REQUEST_HEADERS,
opentelemetry::Value::String(format!("{:?}", headers).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.method"),
HTTP_REQUEST_METHOD,
opentelemetry::Value::String(format!("{}", request.router_request.method()).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.uri"),
HTTP_REQUEST_URI,
opentelemetry::Value::String(format!("{}", request.router_request.uri()).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.version"),
HTTP_REQUEST_VERSION,
opentelemetry::Value::String(
format!("{:?}", request.router_request.version()).into(),
),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.body"),
HTTP_REQUEST_BODY,
opentelemetry::Value::String(format!("{:?}", request.router_request.body()).into()),
));
log_event(self.request.level(), "router.request", attrs, "");
Expand Down Expand Up @@ -305,19 +314,19 @@ impl Instrumented
#[cfg(not(test))]
let headers = response.response.headers();
attrs.push(KeyValue::new(
Key::from_static_str("http.response.headers"),
HTTP_RESPONSE_HEADERS,
opentelemetry::Value::String(format!("{:?}", headers).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.response.status"),
HTTP_RESPONSE_STATUS,
opentelemetry::Value::String(format!("{}", response.response.status()).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.response.version"),
HTTP_RESPONSE_VERSION,
opentelemetry::Value::String(format!("{:?}", response.response.version()).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.response.body"),
HTTP_RESPONSE_BODY,
opentelemetry::Value::String(format!("{:?}", response.response.body()).into()),
));
log_event(self.response.level(), "router.response", attrs, "");
Expand Down Expand Up @@ -383,29 +392,29 @@ impl Instrumented
#[cfg(not(test))]
let headers = request.supergraph_request.headers();
attrs.push(KeyValue::new(
Key::from_static_str("http.request.headers"),
HTTP_REQUEST_HEADERS,
opentelemetry::Value::String(format!("{:?}", headers).into()),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.method"),
HTTP_REQUEST_METHOD,
opentelemetry::Value::String(
format!("{}", request.supergraph_request.method()).into(),
),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.uri"),
HTTP_REQUEST_URI,
opentelemetry::Value::String(
format!("{}", request.supergraph_request.uri()).into(),
),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.version"),
HTTP_REQUEST_VERSION,
opentelemetry::Value::String(
format!("{:?}", request.supergraph_request.version()).into(),
),
));
attrs.push(KeyValue::new(
Key::from_static_str("http.request.body"),
HTTP_REQUEST_BODY,
opentelemetry::Value::String(
serde_json::to_string(request.supergraph_request.body())
.unwrap_or_default()
Expand Down
4 changes: 2 additions & 2 deletions apollo-router/src/services/connector_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use serde::Deserialize;
use serde::Serialize;
use tower::BoxError;
use tower::ServiceExt;
use tracing::warn;
use tracing::error;
use tracing::Instrument;

use super::connect::BoxService;
Expand Down Expand Up @@ -183,7 +183,7 @@ async fn execute(
.insert(CONNECTOR_INFO_CONTEXT_KEY, ConnectorInfo::from(connector))
.is_err()
{
warn!("Failed to store connector info in context - instruments may be inaccurate");
error!("Failed to store connector info in context - instruments may be inaccurate");
}
let original_subgraph_name = original_subgraph_name.clone();
let request_limit = request_limit.clone();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ You can configure events for each service in `router.yaml`. Events can be standa

<RouterServices />

The `router`, `supergraph`, `subgraph` and `conenctor` sections are used to define custom event configuration for each service:
The `router`, `supergraph`, `subgraph` and `connector` sections are used to define custom event configuration for each service:

```yaml title="future.router.yaml"
telemetry:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ Apollo Connectors for REST APIs make HTTP calls to the upstream HTTP API. These
|----------------------------------|-------------|------------------|-------------------------------------------------------------------|
| `subgraph_name` | No | `true`\|`false` | The name of the subgraph containing the connector |
| `connector_source ` | No | `name` | The name of the `@source` associated with this connector, if any |
| `connector_http_request_header` | Yes | `true`\|`false` | The name of a connector request header |
| `connector_http_response_header` | Yes | `true`\|`false` | The name of a connector response header |
| `connector_http_request_header` | Yes | | The name of a connector request header |
| `connector_http_response_header` | Yes | | The name of a connector response header |
| `connector_http_response_status` | No | `code`\|`reason` | The status of a connector response |
| `connector_http_method` | No | `true`\|`false` | The HTTP method of a connector request |
| `connector_url_template ` | No | `true`\|`false` | The URL template of a connector request |
Expand Down

0 comments on commit 994d598

Please sign in to comment.