Skip to content

Commit

Permalink
ref: Use allow list for standalone spans, deserialize hosts directly
Browse files Browse the repository at this point in the history
  • Loading branch information
jjbayer committed Jul 29, 2024
1 parent b41ad06 commit 6768993
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 15 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions relay-dynamic-config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ relay-quotas = { workspace = true }
relay-sampling = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
url = { workspace = true }

[dev-dependencies]
similar-asserts = { workspace = true }
3 changes: 2 additions & 1 deletion relay-dynamic-config/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use relay_filter::GenericFiltersConfig;
use relay_quotas::Quota;
use serde::{de, Deserialize, Serialize};
use serde_json::Value;
use url::Host;

use crate::{defaults, ErrorBoundary, MetricExtractionGroup, MetricExtractionGroups};

Expand Down Expand Up @@ -233,7 +234,7 @@ pub struct Options {
deserialize_with = "default_on_error",
skip_serializing_if = "Vec::is_empty"
)]
pub http_span_allowed_hosts: Vec<String>,
pub http_span_allowed_hosts: Vec<Host>,

/// All other unknown options.
#[serde(flatten)]
Expand Down
3 changes: 2 additions & 1 deletion relay-event-normalization/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use relay_protocol::{
RemarkType, Value,
};
use smallvec::SmallVec;
use url::Host;
use uuid::Uuid;

use crate::normalize::request;
Expand Down Expand Up @@ -156,7 +157,7 @@ pub struct NormalizationConfig<'a> {
pub replay_id: Option<Uuid>,

/// Controls list of hosts to be excluded from scrubbing
pub span_allowed_hosts: &'a [String],
pub span_allowed_hosts: &'a [Host],
}

impl<'a> Default for NormalizationConfig<'a> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const DOMAIN_ALLOW_LIST: &[&str] = &["localhost"];
/// Returns `None` if no scrubbing can be performed.
pub(crate) fn scrub_span_description(
span: &Span,
span_allowed_hosts: &[String],
span_allowed_hosts: &[Host],
) -> (Option<String>, Option<Vec<sqlparser::ast::Statement>>) {
let Some(description) = span.description.as_str() else {
return (None, None);
Expand Down Expand Up @@ -167,7 +167,7 @@ fn scrub_supabase(string: &str) -> Option<String> {
Some(DB_SUPABASE_REGEX.replace_all(string, "{%s}").into())
}

fn scrub_http(string: &str, allow_list: &[String]) -> Option<String> {
fn scrub_http(string: &str, allow_list: &[Host]) -> Option<String> {
let (method, url) = string.split_once(' ')?;
if !HTTP_METHOD_EXTRACTOR_REGEX.is_match(method) {
return None;
Expand Down Expand Up @@ -227,8 +227,8 @@ fn scrub_file(description: &str) -> Option<String> {
/// assert_eq!(scrub_host(Host::Ipv4(Ipv4Addr::LOCALHOST), &[]), "127.0.0.1");
/// assert_eq!(scrub_host(Host::Ipv4(Ipv4Addr::new(8, 8, 8, 8)), &[String::from("8.8.8.8")]), "8.8.8.8");
/// ```
pub fn scrub_host<'a>(host: Host<&'a str>, allow_list: &'a [String]) -> Cow<'a, str> {
if allow_list.contains(&host.to_string()) {
pub fn scrub_host<'a>(host: Host<&'a str>, allow_list: &'a [Host]) -> Cow<'a, str> {
if allow_list.iter().any(|allowed_host| &host == allowed_host) {
return host.to_string().into();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use relay_event_schema::protocol::{
use relay_protocol::{Annotated, Value};
use sqlparser::ast::Visit;
use sqlparser::ast::{ObjectName, Visitor};
use url::Url;
use url::{Host, Url};

use crate::span::description::{
concatenate_host_and_port, scrub_domain_name, scrub_span_description,
Expand Down Expand Up @@ -179,7 +179,7 @@ impl std::fmt::Display for RenderBlockingStatus {
pub(crate) fn extract_span_tags_from_event(
event: &mut Event,
max_tag_value_size: usize,
http_scrubbing_allow_list: &[String],
http_scrubbing_allow_list: &[Host],
) {
// Temporarily take ownership to pass both an event reference and a mutable span reference to `extract_span_tags`.
let mut spans = std::mem::take(&mut event.spans);
Expand All @@ -203,7 +203,7 @@ pub fn extract_span_tags(
event: &Event,
spans: &mut [Annotated<Span>],
max_tag_value_size: usize,
span_allowed_hosts: &[String],
span_allowed_hosts: &[Host],
) {
// TODO: To prevent differences between metrics and payloads, we should not extract tags here
// when they have already been extracted by a downstream relay.
Expand Down Expand Up @@ -471,7 +471,7 @@ pub fn extract_tags(
full_display: Option<Timestamp>,
is_mobile: bool,
start_type: Option<&str>,
span_allowed_hosts: &[String],
span_allowed_hosts: &[Host],
) -> BTreeMap<SpanTagKey, String> {
let mut span_tags: BTreeMap<SpanTagKey, String> = BTreeMap::new();

Expand Down
4 changes: 2 additions & 2 deletions relay-server/src/services/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1484,7 +1484,7 @@ impl EnvelopeProcessorService {

let global_config = self.inner.global_config.current();
let ai_model_costs = global_config.ai_model_costs.clone().ok();
let http_span_allowed_hosts = global_config.options.http_span_allowed_hosts.clone();
let http_span_allowed_hosts = global_config.options.http_span_allowed_hosts.as_slice();

utils::log_transaction_name_metrics(&mut state.event, |event| {
let event_validation_config = EventValidationConfig {
Expand Down Expand Up @@ -1568,7 +1568,7 @@ impl EnvelopeProcessorService {
.envelope()
.dsc()
.and_then(|ctx| ctx.replay_id),
span_allowed_hosts: http_span_allowed_hosts.as_ref(),
span_allowed_hosts: http_span_allowed_hosts,
};

metric!(timer(RelayTimers::EventProcessingNormalization), {
Expand Down
3 changes: 2 additions & 1 deletion relay-server/src/services/processor/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use relay_dynamic_config::Feature;
use relay_event_normalization::span::tag_extraction;
use relay_event_schema::protocol::{Event, Span};
use relay_protocol::Annotated;
use url::Host;

use crate::services::processor::SpanGroup;
use crate::{services::processor::ProcessEnvelopeState, utils::ItemAction};
Expand Down Expand Up @@ -33,7 +34,7 @@ pub fn filter(state: &mut ProcessEnvelopeState<SpanGroup>) {
pub fn extract_transaction_span(
event: &Event,
max_tag_value_size: usize,
span_allowed_hosts: &[String],
span_allowed_hosts: &[Host],
) -> Option<Span> {
let mut spans = [Span::from(event).into()];

Expand Down
17 changes: 15 additions & 2 deletions relay-server/src/services/processor/span/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use relay_pii::PiiProcessor;
use relay_protocol::{Annotated, Empty};
use relay_quotas::DataCategory;
use relay_spans::{otel_to_sentry_span, otel_trace::Span as OtelSpan};
use url::Host;

use crate::envelope::{ContentType, Item, ItemType};
use crate::metrics_extraction::metrics_summary;
Expand Down Expand Up @@ -342,6 +343,8 @@ struct NormalizeSpanConfig<'a> {
user_agent: Option<String>,
/// Client hints parsed from the request.
client_hints: ClientHints<String>,
/// Hosts that are not replaced by "*" in HTTP span grouping.
allowed_hosts: &'a [Host],
}

impl<'a> NormalizeSpanConfig<'a> {
Expand All @@ -352,6 +355,7 @@ impl<'a> NormalizeSpanConfig<'a> {
managed_envelope: &ManagedEnvelope,
) -> Self {
let aggregator_config = config.aggregator_config_for(MetricNamespace::Spans);

Self {
received_at: managed_envelope.received_at(),
timestamp_range: aggregator_config.aggregator.timestamp_range(),
Expand All @@ -377,6 +381,7 @@ impl<'a> NormalizeSpanConfig<'a> {
.user_agent()
.map(String::from),
client_hints: managed_envelope.meta().client_hints().clone(),
allowed_hosts: global_config.options.http_span_allowed_hosts.as_slice(),
}
}
}
Expand Down Expand Up @@ -428,6 +433,7 @@ fn normalize(
tx_name_rules,
user_agent,
client_hints,
allowed_hosts,
} = config;

set_segment_attributes(annotated_span);
Expand Down Expand Up @@ -487,8 +493,15 @@ fn normalize(

// Tag extraction:
let is_mobile = false; // TODO: find a way to determine is_mobile from a standalone span.
let tags =
tag_extraction::extract_tags(span, max_tag_value_size, None, None, is_mobile, None, &[]);
let tags = tag_extraction::extract_tags(
span,
max_tag_value_size,
None,
None,
is_mobile,
None,
allowed_hosts,
);
span.sentry_tags = Annotated::new(
tags.into_iter()
.map(|(k, v)| (k.sentry_tag_key().to_owned(), Annotated::new(v)))
Expand Down

0 comments on commit 6768993

Please sign in to comment.