Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
jjbayer committed Jul 9, 2024
1 parent 7ebcb12 commit d921041
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 182 deletions.
50 changes: 5 additions & 45 deletions relay-event-schema/src/protocol/span/convert.rs
Original file line number Diff line number Diff line change
@@ -1,62 +1,24 @@
//! This module defines bidirectional field mappings between spans and transactions.
use crate::protocol::{
BrowserContext, Contexts, Event, ProfileContext, Span, SpanData, TraceContext,
};
use crate::protocol::{BrowserContext, Event, ProfileContext, Span, SpanData, TraceContext};

impl From<&Event> for Span {
fn from(event: &Event) -> Self {
let Event {
id,
level,
version,
ty,
fingerprint,
culprit,
transaction,
transaction_info,
time_spent,
logentry,
logger,
modules,

platform,
timestamp,
start_timestamp,
received,
server_name,
release,
dist,
environment,
site,
user,
request,
contexts,
breadcrumbs,
exceptions,
stacktrace,
template,
threads,
tags,
extra,
debug_meta,
client_sdk,
ingest_path,
errors,
key_id,
project,
grouping_config,
checksum,
csp,
hpkp,
expectct,
expectstaple,
spans,

measurements,
breakdowns,
scraping_attempts,
_metrics,
_metrics_summary,
other,
..
} = event;

let trace = event.context::<TraceContext>();
Expand All @@ -68,6 +30,7 @@ impl From<&Event> for Span {

// Overwrite specific fields:
let span_data = data.get_or_insert_with(Default::default);
span_data.segment_name = transaction.clone();
span_data.release = release.clone();
span_data.environment = environment.clone();
if let Some(browser) = event.context::<BrowserContext>() {
Expand Down Expand Up @@ -111,9 +74,6 @@ impl From<&Event> for Span {
#[cfg(test)]
mod tests {
use relay_protocol::Annotated;
use similar_asserts::assert_eq;

use crate::protocol::{SpanData, SpanId};

use super::*;

Expand Down
2 changes: 0 additions & 2 deletions relay-server/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ impl ServiceState {
store_forwarder: store.clone(),
},
metric_outcomes.clone(),
#[cfg(feature = "processing")]
buffer_guard.clone(),
)
.spawn_handler(processor_rx);

Expand Down
15 changes: 1 addition & 14 deletions relay-server/src/services/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ use crate::services::upstream::{
SendRequest, UpstreamRelay, UpstreamRequest, UpstreamRequestError,
};
use crate::statsd::{RelayCounters, RelayHistograms, RelayTimers};
#[cfg(feature = "processing")]
use crate::utils::BufferGuard;
use crate::utils::{
self, InvalidProcessingGroupType, ManagedEnvelope, SamplingResult, TypedEnvelope,
};
Expand Down Expand Up @@ -1129,8 +1127,6 @@ struct InnerProcessor {
#[cfg(feature = "processing")]
cardinality_limiter: Option<CardinalityLimiter>,
metric_outcomes: MetricOutcomes,
#[cfg(feature = "processing")]
buffer_guard: Arc<BufferGuard>,
}

impl EnvelopeProcessorService {
Expand All @@ -1142,7 +1138,6 @@ impl EnvelopeProcessorService {
#[cfg(feature = "processing")] redis: Option<RedisPool>,
addrs: Addrs,
metric_outcomes: MetricOutcomes,
#[cfg(feature = "processing")] buffer_guard: Arc<BufferGuard>,
) -> Self {
let geoip_lookup = config.geoip_path().and_then(|p| {
match GeoIpLookup::open(p).context(ServiceError::GeoIp) {
Expand Down Expand Up @@ -1184,8 +1179,6 @@ impl EnvelopeProcessorService {
.map(CardinalityLimiter::new),
metric_outcomes,
config,
#[cfg(feature = "processing")]
buffer_guard,
};

Self {
Expand Down Expand Up @@ -1836,13 +1829,7 @@ impl EnvelopeProcessorService {
if_processing!(self.inner.config, {
let global_config = self.inner.global_config.current();

span::process(
state,
self.inner.config.clone(),
&global_config,
&self.inner.addrs,
&self.inner.buffer_guard,
);
span::process(state, self.inner.config.clone(), &global_config);

self.enforce_quotas(state)?;
});
Expand Down
24 changes: 3 additions & 21 deletions relay-server/src/services/processor/span/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,14 @@ use relay_protocol::{Annotated, Empty};
use relay_quotas::DataCategory;
use relay_spans::{otel_to_sentry_span, otel_trace::Span as OtelSpan};

use crate::envelope::{ContentType, Envelope, Item, ItemType};
use crate::envelope::{ContentType, Item, ItemType};
use crate::metrics_extraction::metrics_summary;
use crate::services::outcome::{DiscardReason, Outcome};
use crate::services::processor::span::extract_transaction_span;
use crate::services::processor::{
dynamic_sampling, Addrs, ProcessEnvelope, ProcessEnvelopeState, ProcessingError,
ProcessingGroup, SpanGroup, TransactionGroup,
dynamic_sampling, ProcessEnvelopeState, ProcessingError, SpanGroup, TransactionGroup,
};
use crate::statsd::{RelayCounters, RelayHistograms};
use crate::utils::{sample, BufferGuard, ItemAction};
use crate::utils::{sample, ItemAction};
use relay_event_normalization::span::ai::extract_ai_measurements;
use thiserror::Error;

Expand All @@ -45,8 +43,6 @@ pub fn process(
state: &mut ProcessEnvelopeState<SpanGroup>,
config: Arc<Config>,
global_config: &GlobalConfig,
addrs: &Addrs,
buffer_guard: &BufferGuard,
) {
use relay_event_normalization::RemoveOtherProcessor;

Expand Down Expand Up @@ -630,20 +626,6 @@ fn validate(span: &mut Annotated<Span>) -> Result<(), ValidationError> {
Ok(())
}

fn convert_to_transaction(annotated_span: &Annotated<Span>) -> Option<Event> {
let span = annotated_span.value()?;

// HACK: This is an exception from the JS SDK v8 and we do not want to turn it into a transaction.
if let Some(span_op) = span.op.value() {
if span_op == "http.client" && span.parent_span_id.is_empty() {
return None;
}
}

relay_log::trace!("Extracting transaction for span {:?}", &span.span_id);
Event::try_from(span).ok()
}

#[cfg(test)]
mod tests {
use std::collections::BTreeMap;
Expand Down
5 changes: 0 additions & 5 deletions relay-server/src/statsd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -747,9 +747,6 @@ pub enum RelayCounters {
/// This metric is tagged with:
/// - `success`: whether deserializing the global config succeeded.
GlobalConfigFetched,
/// Counts how many transactions were created from segment spans.
#[cfg(feature = "processing")]
TransactionsFromSpans,
/// The number of attachments processed in the same envelope as a user_report_v2 event.
FeedbackAttachments,
/// All COGS tracked values.
Expand Down Expand Up @@ -799,8 +796,6 @@ impl CounterMetric for RelayCounters {
RelayCounters::MetricsTransactionNameExtracted => "metrics.transaction_name",
RelayCounters::OpenTelemetryEvent => "event.opentelemetry",
RelayCounters::GlobalConfigFetched => "global_config.fetch",
#[cfg(feature = "processing")]
RelayCounters::TransactionsFromSpans => "transactions_from_spans",
RelayCounters::FeedbackAttachments => "processing.feedback_attachments",
RelayCounters::CogsUsage => "cogs.usage",
RelayCounters::ProjectStateFlushMetricsNoProject => "project_state.metrics.no_project",
Expand Down
6 changes: 0 additions & 6 deletions relay-server/src/testutils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ use crate::services::outcome::TrackOutcome;
use crate::services::processor::{self, EnvelopeProcessorService};
use crate::services::project::ProjectState;
use crate::services::test_store::TestStore;
#[cfg(feature = "processing")]
use crate::utils::BufferGuard;

pub fn state_with_rule_and_condition(
sample_rate: Option<f64>,
Expand Down Expand Up @@ -144,8 +142,6 @@ pub fn create_test_processor(config: Config) -> EnvelopeProcessorService {
store_forwarder: None,
},
metric_outcomes,
#[cfg(feature = "processing")]
Arc::new(BufferGuard::new(usize::MAX)),
)
}

Expand All @@ -171,8 +167,6 @@ pub fn create_test_processor_with_addrs(
redis,
addrs,
metric_outcomes,
#[cfg(feature = "processing")]
Arc::new(BufferGuard::new(usize::MAX)),
)
}

Expand Down
89 changes: 0 additions & 89 deletions tests/integration/test_spans.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,18 +470,14 @@ def make_otel_span(start, end):
}


@pytest.mark.parametrize("extract_transaction", [False, True])
def test_span_ingestion(
mini_sentry,
relay_with_processing,
spans_consumer,
metrics_consumer,
transactions_consumer,
extract_transaction,
):
spans_consumer = spans_consumer()
metrics_consumer = metrics_consumer()
transactions_consumer = transactions_consumer()

relay = relay_with_processing(
options={
Expand All @@ -502,10 +498,6 @@ def test_span_ingestion(
project_config["config"]["transactionMetrics"] = {
"version": TRANSACTION_EXTRACT_MIN_SUPPORTED_VERSION
}
if extract_transaction:
project_config["config"]["features"].append(
"projects:extract-transaction-from-segment-span"
)

duration = timedelta(milliseconds=500)
now = datetime.now(timezone.utc)
Expand Down Expand Up @@ -693,32 +685,6 @@ def test_span_ingestion(

spans_consumer.assert_empty()

# If transaction extraction is enabled, expect transactions:
if extract_transaction:
expected_transactions = 3

transactions = [
transactions_consumer.get_event()[0] for _ in range(expected_transactions)
]

assert len(transactions) == expected_transactions
assert sorted([transaction["event_id"] for transaction in transactions]) == [
"0000000000000000a342abb1214ca181",
"0000000000000000b0429c44b67a3eb1",
"0000000000000000d342abb1214ca182",
]
for transaction in transactions:
# Not checking all individual fields here, most should be tested in convert.rs

# SDK gets taken from the header:
if sdk := transaction.get("sdk"):
assert sdk == {"name": "raven-node", "version": "2.6.3"}

# No errors during normalization:
assert not transaction.get("errors")

transactions_consumer.assert_empty()

metrics = [metric for (metric, _headers) in metrics_consumer.get_metrics()]
metrics.sort(key=lambda m: (m["name"], sorted(m["tags"].items()), m["timestamp"]))
for metric in metrics:
Expand Down Expand Up @@ -1079,61 +1045,6 @@ def test_span_extraction_with_metrics_summary(
assert metrics_summary["mri"] == mri


def test_extracted_transaction_gets_normalized(
mini_sentry, transactions_consumer, relay_with_processing, relay, relay_credentials
):
"""When normalization in processing relays has been disabled, an extracted
transaction still gets normalized.
This test was copied and adapted from test_store::test_relay_chain_normalization
"""
project_id = 42
project_config = mini_sentry.add_full_project_config(project_id)
project_config["config"]["features"] = [
"organizations:standalone-span-ingestion",
"projects:extract-transaction-from-segment-span",
"projects:relay-otel-endpoint",
]

transactions_consumer = transactions_consumer()

credentials = relay_credentials()
processing = relay_with_processing(
static_relays={
credentials["id"]: {
"public_key": credentials["public_key"],
"internal": True,
},
},
options={"processing": {"normalize": "disabled"}},
)
relay = relay(
processing,
credentials=credentials,
options={
"processing": {
"normalize": "full",
}
},
)

duration = timedelta(milliseconds=500)
end = datetime.now(timezone.utc) - timedelta(seconds=1)
start = end - duration
otel_payload = make_otel_span(start, end)

# Unset name to validate transaction normalization
del otel_payload["resourceSpans"][0]["scopeSpans"][0]["spans"][0]["name"]

relay.send_otel_span(project_id, json=otel_payload)

ingested, _ = transactions_consumer.get_event(timeout=10)

# "<unlabeled transaction>" was set by normalization:
assert ingested["transaction"] == "<unlabeled transaction>"


def test_span_no_extraction_with_metrics_summary(
mini_sentry,
relay_with_processing,
Expand Down

0 comments on commit d921041

Please sign in to comment.