Skip to content

Commit

Permalink
fix(spans): Restrict usage of otel endpoint (#3597)
Browse files Browse the repository at this point in the history
Span ingestion was limited in general by the
`organizations:standalone-span-ingestion` feature, but was GA'd for INP
spans. However, we still want to restrict access to the experimental
OTel `/spans/` endpoint, which is why this PR introduces a new feature
`projects:relay-otel-endpoint`.
  • Loading branch information
jjbayer committed May 16, 2024
1 parent e29d796 commit da1d616
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- Add support for `event.` in the `Span` `Getter` implementation. ([#3577](https://github.com/getsentry/relay/pull/3577))
- Ensure `chunk_id` and `profiler_id` are UUIDs and sort samples. ([#3588](https://github.com/getsentry/relay/pull/3588))
- Add a calculated measurement based on the AI model and the tokens used. ([#3554](https://github.com/getsentry/relay/pull/3554))
- Restrict usage of OTel endpoint. ([#3597](github.com/getsentry/relay/pull/3597))


## 24.4.2
Expand Down
7 changes: 6 additions & 1 deletion relay-dynamic-config/src/feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::BTreeSet;
use serde::{Deserialize, Serialize};

/// Features exposed by project config.
#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
pub enum Feature {
/// Enables ingestion of Session Replays (Replay Recordings and Replay Events).
///
Expand Down Expand Up @@ -56,6 +56,11 @@ pub enum Feature {
/// Serialized as `organizations:standalone-span-ingestion`.
#[serde(rename = "organizations:standalone-span-ingestion")]
StandaloneSpanIngestion,
/// Enable standalone span ingestion via the `/spans/` OTel endpoint.
///
/// Serialized as `projects:relay-otel-endpoint`.
#[serde(rename = "projects:relay-otel-endpoint")]
OtelEndpoint,
/// Enable processing and extracting data from profiles that would normally be dropped by dynamic sampling.
///
/// This is required for [slowest function aggregation](https://github.com/getsentry/snuba/blob/b5311b404a6bd73a9e1997a46d38e7df88e5f391/snuba/snuba_migrations/functions/0001_functions.py#L209-L256). The profile payload will be dropped on the sentry side.
Expand Down
2 changes: 2 additions & 0 deletions relay-server/src/endpoints/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use axum_extra::protobuf::Protobuf;
use bytes::Bytes;

use relay_config::Config;
use relay_dynamic_config::Feature;
use relay_spans::otel_trace::TracesData;

use crate::endpoints::common;
Expand Down Expand Up @@ -36,6 +37,7 @@ where
};

let mut envelope = Envelope::from_request(None, meta);
envelope.require_feature(Feature::OtelEndpoint);
for resource_span in trace.resource_spans {
for scope_span in resource_span.scope_spans {
for span in scope_span.spans {
Expand Down
21 changes: 20 additions & 1 deletion relay-server/src/envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use uuid::Uuid;

use bytes::Bytes;
use chrono::{DateTime, Utc};
use relay_dynamic_config::ErrorBoundary;
use relay_dynamic_config::{ErrorBoundary, Feature};
use relay_event_normalization::{normalize_transaction_name, TransactionNameRule};
use relay_event_schema::protocol::{Event, EventId, EventType};
use relay_protocol::{Annotated, Value};
Expand Down Expand Up @@ -1039,6 +1039,13 @@ pub struct EnvelopeHeaders<M = RequestMeta> {
#[serde(default, skip_serializing_if = "Option::is_none")]
trace: Option<ErrorBoundary<DynamicSamplingContext>>,

/// A list of features required to process this envelope.
///
/// This is an internal field that should only be set by Relay.
/// It is a serializable header such that it persists when spooling.
#[serde(default, skip_serializing_if = "SmallVec::is_empty")]
required_features: SmallVec<[Feature; 1]>,

/// Other attributes for forward compatibility.
#[serde(flatten)]
other: BTreeMap<String, Value>,
Expand Down Expand Up @@ -1079,6 +1086,7 @@ impl EnvelopeHeaders<PartialMeta> {
retention: self.retention,
sent_at: self.sent_at,
trace: self.trace,
required_features: self.required_features,
other: self.other,
})
}
Expand Down Expand Up @@ -1121,6 +1129,7 @@ impl Envelope {
sent_at: None,
other: BTreeMap::new(),
trace: None,
required_features: smallvec::smallvec![],
},
items: Items::new(),
})
Expand Down Expand Up @@ -1276,6 +1285,16 @@ impl Envelope {
self.headers.trace = Some(ErrorBoundary::Ok(dsc));
}

/// Features required to process this envelope.
pub fn required_features(&self) -> &[Feature] {
&self.headers.required_features
}

/// Add a feature requirement to this envelope.
pub fn require_feature(&mut self, feature: Feature) {
self.headers.required_features.push(feature)
}

/// Returns the specified header value, if present.
#[cfg_attr(not(feature = "processing"), allow(dead_code))]
pub fn get_header<K>(&self, name: &K) -> Option<&Value>
Expand Down
5 changes: 5 additions & 0 deletions relay-server/src/services/outcome.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use chrono::{DateTime, SecondsFormat, Utc};
use relay_base_schema::project::ProjectId;
use relay_common::time::UnixTimestamp;
use relay_config::{Config, EmitOutcomes};
use relay_dynamic_config::Feature;
use relay_event_schema::protocol::{ClientReport, DiscardedEvent, EventId};
use relay_filter::FilterStatKey;
#[cfg(feature = "processing")]
Expand Down Expand Up @@ -377,6 +378,9 @@ pub enum DiscardReason {

/// (Relay) A span is not valid after normalization.
InvalidSpan,

/// (Relay) A required feature is not enabled.
FeatureDisabled(Feature),
}

impl DiscardReason {
Expand Down Expand Up @@ -421,6 +425,7 @@ impl DiscardReason {
DiscardReason::InvalidReplayVideoEvent => "invalid_replay_video",
DiscardReason::Profiling(reason) => reason,
DiscardReason::InvalidSpan => "invalid_span",
DiscardReason::FeatureDisabled(_) => "feature_disabled",
}
}
}
Expand Down
26 changes: 21 additions & 5 deletions relay-server/src/services/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use smallvec::SmallVec;
use tokio::time::Instant;
use url::Url;

use crate::envelope::Envelope;
use crate::metrics::MetricOutcomes;
use crate::services::outcome::{DiscardReason, Outcome, TrackOutcome};
#[cfg(feature = "processing")]
Expand Down Expand Up @@ -330,17 +331,23 @@ impl ProjectState {
Ok(())
}

/// Determines whether the given request should be accepted or discarded.
/// Determines whether the given envelope should be accepted or discarded.
///
/// Returns `Ok(())` if the request should be accepted. Returns `Err(DiscardReason)` if the
/// request should be discarded, by indicating the reason. The checks preformed for this are:
/// Returns `Ok(())` if the envelope should be accepted. Returns `Err(DiscardReason)` if the
/// envelope should be discarded, by indicating the reason. The checks preformed for this are:
///
/// - Allowed origin headers
/// - Disabled or unknown projects
/// - Disabled project keys (DSN)
pub fn check_request(&self, meta: &RequestMeta, config: &Config) -> Result<(), DiscardReason> {
/// - Feature flags
pub fn check_envelope(
&self,
envelope: &Envelope,
config: &Config,
) -> Result<(), DiscardReason> {
// Verify that the stated project id in the DSN matches the public key used to retrieve this
// project state.
let meta = envelope.meta();
if !self.is_valid_project_id(meta.project_id(), config) {
return Err(DiscardReason::ProjectId);
}
Expand All @@ -359,6 +366,15 @@ impl ProjectState {
// Check for invalid or disabled projects.
self.check_disabled(config)?;

// Check feature.
if let Some(disabled_feature) = envelope
.required_features()
.iter()
.find(|f| !self.has_feature(**f))
{
return Err(DiscardReason::FeatureDisabled(*disabled_feature));
}

Ok(())
}

Expand Down Expand Up @@ -1040,7 +1056,7 @@ impl Project {
scoping = state.scope_request(envelope.envelope().meta());
envelope.scope(scoping);

if let Err(reason) = state.check_request(envelope.envelope().meta(), &self.config) {
if let Err(reason) = state.check_envelope(envelope.envelope(), &self.config) {
envelope.reject(Outcome::Invalid(reason));
return Err(reason);
}
Expand Down
48 changes: 48 additions & 0 deletions tests/integration/test_spans.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

import pytest

from requests import HTTPError
from sentry_relay.consts import DataCategory
from sentry_sdk.envelope import Envelope, Item, PayloadRef

from .test_store import make_transaction
Expand Down Expand Up @@ -434,6 +436,7 @@ def test_span_ingestion(
project_config["config"]["features"] = [
"organizations:standalone-span-ingestion",
"projects:span-metrics-extraction",
"projects:relay-otel-endpoint",
]
project_config["config"]["transactionMetrics"] = {"version": 1}
if extract_transaction:
Expand Down Expand Up @@ -833,6 +836,50 @@ def test_span_ingestion(
metrics_consumer.assert_empty()


def test_otel_endpoint_disabled(mini_sentry, relay):
relay = relay(
mini_sentry,
{
"outcomes": {
"emit_outcomes": True,
"batch_size": 1,
"batch_interval": 1,
"source": "relay",
}
},
)
project_id = 42
project_config = mini_sentry.add_full_project_config(project_id)["config"]
project_config["features"] = ["organizations:standalone-span-ingestion"]

end = datetime.now(timezone.utc) - timedelta(seconds=1)
start = end - timedelta(milliseconds=500)
relay.send_otel_span(
project_id,
json=make_otel_span(start, end),
)

(outcome,) = mini_sentry.captured_outcomes.get()["outcomes"]
assert outcome["category"] == DataCategory.SPAN
assert outcome["outcome"] == 3 # invalid
assert outcome["reason"] == "feature_disabled"

# Second attempt will cause a 403 response:
with pytest.raises(HTTPError) as exc_info:
relay.send_otel_span(
project_id,
json=make_otel_span(start, end),
)
response = exc_info.value.response
assert response.status_code == 403
assert response.json() == {
"detail": "event submission rejected with_reason: FeatureDisabled(OtelEndpoint)"
}

# No envelopes were received:
assert mini_sentry.captured_events.empty()


def test_span_extraction_with_metrics_summary(
mini_sentry,
relay_with_processing,
Expand Down Expand Up @@ -926,6 +973,7 @@ def test_extracted_transaction_gets_normalized(
project_config["config"]["features"] = [
"organizations:standalone-span-ingestion",
"projects:extract-transaction-from-segment-span",
"projects:relay-otel-endpoint",
]

transactions_consumer = transactions_consumer()
Expand Down

0 comments on commit da1d616

Please sign in to comment.