Skip to content

Commit 2b55912

Browse files
Dav1ddeDav1dde
authored
Merge branch 'master' into dav1d/cleanup-attachment-env-forward
2 parents ac46fb7 + 4e4a3e1 commit 2b55912

File tree

15 files changed

+508
-53
lines changed

15 files changed

+508
-53
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@
1818
- Add support for Unix domain sockets for statsd metrics. ([#5668](https://github.com/getsentry/relay/pull/5668))
1919
- Support `deployment.environment` OTLP resource attribute for setting the Sentry environment. ([#5691](https://github.com/getsentry/relay/pull/5691))
2020
- Allow users to opt-out of DNS caching. ([#5700](https://github.com/getsentry/relay/pull/5700))
21+
- Update device classification for new iPad and iPhone models. ([#5704](https://github.com/getsentry/relay/pull/5704))
2122

2223
**Internal**:
2324

2425
- Strip performance metric specs from extraction while keeping extraction interfaces intact. ([#5674](https://github.com/getsentry/relay/pull/5674))
2526
- Allow deferred lengths to the `/upload` endpoint when the sender is trusted. ([#5658](https://github.com/getsentry/relay/pull/5658))
2627
- Use new processor architecture to process client reports. ([#5686](https://github.com/getsentry/relay/pull/5686))
28+
- Use new processor architecture to process standalone attachments. ([#5703](https://github.com/getsentry/relay/pull/5703))
2729
- Prevent timeouts on the `/upload` endpoint. ([#5692](https://github.com/getsentry/relay/pull/5692))
2830
- Handle traffic bursts in the objectstore service. ([#5689](https://github.com/getsentry/relay/pull/5689))
2931
- Disable `fetch_materials` on GoCD `pipeline-complete` stages. ([#5697](https://github.com/getsentry/relay/pull/5697))

‎relay-event-schema/src/protocol/device_class.rs‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,20 +109,20 @@ fn model_to_class(model: &str) -> Option<DeviceClass> {
109109
"iPhone18,2" => Some(DeviceClass::HIGH),
110110
"iPhone18,3" => Some(DeviceClass::HIGH),
111111
"iPhone18,4" => Some(DeviceClass::HIGH),
112+
"iPhone18,5" => Some(DeviceClass::HIGH),
112113

113114
// iPads
114115
"iPad1,1" => Some(DeviceClass::LOW),
115-
"iPad1,2" => Some(DeviceClass::LOW),
116116
"iPad2,1" => Some(DeviceClass::LOW),
117117
"iPad2,2" => Some(DeviceClass::LOW),
118118
"iPad2,3" => Some(DeviceClass::LOW),
119119
"iPad2,4" => Some(DeviceClass::LOW),
120-
"iPad3,1" => Some(DeviceClass::LOW),
121-
"iPad3,2" => Some(DeviceClass::LOW),
122-
"iPad3,3" => Some(DeviceClass::LOW),
123120
"iPad2,5" => Some(DeviceClass::LOW),
124121
"iPad2,6" => Some(DeviceClass::LOW),
125122
"iPad2,7" => Some(DeviceClass::LOW),
123+
"iPad3,1" => Some(DeviceClass::LOW),
124+
"iPad3,2" => Some(DeviceClass::LOW),
125+
"iPad3,3" => Some(DeviceClass::LOW),
126126
"iPad3,4" => Some(DeviceClass::LOW),
127127
"iPad3,5" => Some(DeviceClass::LOW),
128128
"iPad3,6" => Some(DeviceClass::LOW),
@@ -145,12 +145,12 @@ fn model_to_class(model: &str) -> Option<DeviceClass> {
145145
"iPad6,8" => Some(DeviceClass::MEDIUM),
146146
"iPad6,11" => Some(DeviceClass::LOW),
147147
"iPad6,12" => Some(DeviceClass::LOW),
148+
"iPad7,1" => Some(DeviceClass::MEDIUM),
148149
"iPad7,2" => Some(DeviceClass::MEDIUM),
149150
"iPad7,3" => Some(DeviceClass::MEDIUM),
150151
"iPad7,4" => Some(DeviceClass::MEDIUM),
151152
"iPad7,5" => Some(DeviceClass::MEDIUM),
152153
"iPad7,6" => Some(DeviceClass::MEDIUM),
153-
"iPad7,1" => Some(DeviceClass::MEDIUM),
154154
"iPad7,11" => Some(DeviceClass::MEDIUM),
155155
"iPad7,12" => Some(DeviceClass::MEDIUM),
156156
"iPad8,1" => Some(DeviceClass::MEDIUM),
@@ -173,8 +173,6 @@ fn model_to_class(model: &str) -> Option<DeviceClass> {
173173
"iPad11,7" => Some(DeviceClass::MEDIUM),
174174
"iPad12,1" => Some(DeviceClass::MEDIUM),
175175
"iPad12,2" => Some(DeviceClass::MEDIUM),
176-
"iPad14,1" => Some(DeviceClass::HIGH),
177-
"iPad14,2" => Some(DeviceClass::HIGH),
178176
"iPad13,1" => Some(DeviceClass::HIGH),
179177
"iPad13,2" => Some(DeviceClass::HIGH),
180178
"iPad13,4" => Some(DeviceClass::HIGH),
@@ -189,6 +187,8 @@ fn model_to_class(model: &str) -> Option<DeviceClass> {
189187
"iPad13,17" => Some(DeviceClass::HIGH),
190188
"iPad13,18" => Some(DeviceClass::HIGH),
191189
"iPad13,19" => Some(DeviceClass::HIGH),
190+
"iPad14,1" => Some(DeviceClass::HIGH),
191+
"iPad14,2" => Some(DeviceClass::HIGH),
192192
"iPad14,3" => Some(DeviceClass::HIGH),
193193
"iPad14,4" => Some(DeviceClass::HIGH),
194194
"iPad14,5" => Some(DeviceClass::HIGH),
@@ -197,12 +197,26 @@ fn model_to_class(model: &str) -> Option<DeviceClass> {
197197
"iPad14,9" => Some(DeviceClass::HIGH),
198198
"iPad14,10" => Some(DeviceClass::HIGH),
199199
"iPad14,11" => Some(DeviceClass::HIGH),
200+
"iPad15,3" => Some(DeviceClass::HIGH),
201+
"iPad15,4" => Some(DeviceClass::HIGH),
202+
"iPad15,5" => Some(DeviceClass::HIGH),
203+
"iPad15,6" => Some(DeviceClass::HIGH),
204+
"iPad15,7" => Some(DeviceClass::HIGH),
205+
"iPad15,8" => Some(DeviceClass::HIGH),
200206
"iPad16,1" => Some(DeviceClass::HIGH),
201207
"iPad16,2" => Some(DeviceClass::HIGH),
202208
"iPad16,3" => Some(DeviceClass::HIGH),
203209
"iPad16,4" => Some(DeviceClass::HIGH),
204210
"iPad16,5" => Some(DeviceClass::HIGH),
205211
"iPad16,6" => Some(DeviceClass::HIGH),
212+
"iPad16,8" => Some(DeviceClass::HIGH),
213+
"iPad16,9" => Some(DeviceClass::HIGH),
214+
"iPad16,10" => Some(DeviceClass::HIGH),
215+
"iPad16,11" => Some(DeviceClass::HIGH),
216+
"iPad17,1" => Some(DeviceClass::HIGH),
217+
"iPad17,2" => Some(DeviceClass::HIGH),
218+
"iPad17,3" => Some(DeviceClass::HIGH),
219+
"iPad17,4" => Some(DeviceClass::HIGH),
206220

207221
// If we don't know the model it's a new device and therefore must be high.
208222
_ => Some(DeviceClass::HIGH),

‎relay-server/src/processing/attachments/forward.rs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
use crate::Envelope;
2+
use crate::managed::{Managed, Rejected};
3+
use crate::processing::attachments::AttachmentsOutput;
4+
use crate::processing::{self, Forward};
5+
6+
impl Forward for AttachmentsOutput {
7+
fn serialize_envelope(
8+
self,
9+
_: processing::ForwardContext<'_>,
10+
) -> Result<Managed<Box<Envelope>>, Rejected<()>> {
11+
let Self(attachments) = self;
12+
Ok(attachments.map(|attachments, _| {
13+
Envelope::from_parts(attachments.headers, attachments.attachments)
14+
}))
15+
}
16+
17+
#[cfg(feature = "processing")]
18+
fn forward_store(
19+
self,
20+
s: processing::StoreHandle<'_>,
21+
ctx: processing::ForwardContext<'_>,
22+
) -> Result<(), Rejected<()>> {
23+
use crate::processing::attachments::Error;
24+
25+
let Self(attachments) = self;
26+
27+
let Some(event_id) = attachments.headers.event_id() else {
28+
return Err(attachments.reject_err(Error::NoEventId).map(drop));
29+
};
30+
31+
let use_objectstore = {
32+
let options = &ctx.global_config.options;
33+
crate::utils::sample(options.objectstore_attachments_sample_rate).is_keep()
34+
};
35+
36+
for attachment in attachments.split(|attachment| attachment.attachments) {
37+
let store_attachment = attachment.map(|attachment, _| {
38+
use crate::services::store::StoreAttachment;
39+
let quantities = attachment.quantities();
40+
StoreAttachment {
41+
event_id,
42+
attachment,
43+
quantities,
44+
}
45+
});
46+
if use_objectstore {
47+
s.send_to_objectstore(store_attachment);
48+
} else {
49+
s.send_to_store(store_attachment);
50+
}
51+
}
52+
53+
Ok(())
54+
}
55+
}

‎relay-server/src/processing/attachments/mod.rs‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
use std::sync::Arc;
2+
3+
use relay_quotas::RateLimits;
4+
5+
use crate::envelope::{EnvelopeHeaders, Item, ItemType, Items};
6+
use crate::managed::{Counted, Managed, ManagedEnvelope, OutcomeError, Quantities, Rejected};
7+
use crate::processing::{self, CountRateLimited, Output, QuotaRateLimiter};
8+
#[cfg(feature = "processing")]
9+
use crate::services::outcome::DiscardReason;
10+
use crate::services::outcome::Outcome;
11+
use crate::statsd::RelayCounters;
12+
13+
mod forward;
14+
mod process;
15+
16+
#[derive(Debug, thiserror::Error)]
17+
pub enum Error {
18+
/// The Attachment was rate limited.
19+
#[error("rate limited")]
20+
RateLimited(RateLimits),
21+
22+
/// The envelope did not contain an event ID.
23+
#[cfg(feature = "processing")]
24+
#[error("missing event ID")]
25+
NoEventId,
26+
}
27+
28+
impl OutcomeError for Error {
29+
type Error = Self;
30+
31+
fn consume(self) -> (Option<crate::services::outcome::Outcome>, Self::Error) {
32+
let outcome = match &self {
33+
Self::RateLimited(limits) => {
34+
let reason_code = limits.longest().and_then(|limit| limit.reason_code.clone());
35+
Some(Outcome::RateLimited(reason_code))
36+
}
37+
#[cfg(feature = "processing")]
38+
Self::NoEventId => Some(Outcome::Invalid(DiscardReason::Internal)),
39+
};
40+
(outcome, self)
41+
}
42+
}
43+
44+
impl From<RateLimits> for Error {
45+
fn from(value: RateLimits) -> Self {
46+
Self::RateLimited(value)
47+
}
48+
}
49+
50+
/// A processor for Attachments.
51+
pub struct AttachmentProcessor {
52+
limiter: Arc<QuotaRateLimiter>,
53+
}
54+
55+
impl AttachmentProcessor {
56+
/// Creates a new [`Self`].
57+
pub fn new(limiter: Arc<QuotaRateLimiter>) -> Self {
58+
Self { limiter }
59+
}
60+
}
61+
62+
impl processing::Processor for AttachmentProcessor {
63+
type UnitOfWork = SerializedAttachments;
64+
type Output = AttachmentsOutput;
65+
type Error = Error;
66+
67+
fn prepare_envelope(
68+
&self,
69+
envelope: &mut ManagedEnvelope,
70+
) -> Option<Managed<Self::UnitOfWork>> {
71+
debug_assert!(
72+
!envelope.envelope().items().any(Item::creates_event),
73+
"AttachmentProcessor should not receive items that create events"
74+
);
75+
76+
let attachments = envelope
77+
.envelope_mut()
78+
.take_items_by(|i| i.requires_event() && matches!(i.ty(), ItemType::Attachment));
79+
80+
if attachments.is_empty() {
81+
return None;
82+
}
83+
84+
let headers = envelope.envelope().headers().clone();
85+
let work = SerializedAttachments {
86+
headers,
87+
attachments,
88+
};
89+
Some(Managed::with_meta_from(envelope, work))
90+
}
91+
92+
async fn process(
93+
&self,
94+
attachments: Managed<Self::UnitOfWork>,
95+
ctx: processing::Context<'_>,
96+
) -> Result<processing::Output<Self::Output>, Rejected<Self::Error>> {
97+
for item in &attachments.attachments {
98+
let attachment_type_tag = match item.attachment_type() {
99+
Some(t) => &t.to_string(),
100+
None => "",
101+
};
102+
relay_statsd::metric!(
103+
counter(RelayCounters::StandaloneItem) += 1,
104+
processor = "new",
105+
item_type = item.ty().name(),
106+
attachment_type = attachment_type_tag,
107+
);
108+
}
109+
110+
let mut attachments = self.limiter.enforce_quotas(attachments, ctx).await?;
111+
process::scrub(&mut attachments, ctx)?;
112+
113+
Ok(Output::just(AttachmentsOutput(attachments)))
114+
}
115+
}
116+
117+
/// Serialized attachments extracted from an envelope.
118+
#[derive(Debug)]
119+
pub struct SerializedAttachments {
120+
/// Original envelope headers.
121+
headers: EnvelopeHeaders,
122+
/// A list of attachments.
123+
attachments: Items,
124+
}
125+
126+
impl Counted for SerializedAttachments {
127+
fn quantities(&self) -> Quantities {
128+
self.attachments.quantities()
129+
}
130+
}
131+
132+
impl CountRateLimited for Managed<SerializedAttachments> {
133+
type Error = Error;
134+
}
135+
136+
/// Output produced by the [`AttachmentProcessor`].
137+
#[derive(Debug)]
138+
pub struct AttachmentsOutput(Managed<SerializedAttachments>);

‎relay-server/src/processing/attachments/process.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
use crate::managed::{Managed, Rejected};
2+
use crate::processing::attachments::{Error, SerializedAttachments};
3+
use crate::processing::{self, utils};
4+
5+
/// Runs PiiProcessors on the attachments.
6+
pub fn scrub(
7+
attachments: &mut Managed<SerializedAttachments>,
8+
ctx: processing::Context<'_>,
9+
) -> Result<(), Rejected<Error>> {
10+
attachments.try_modify(|attachments, records| {
11+
utils::attachments::scrub(
12+
attachments.attachments.iter_mut(),
13+
ctx.project_info,
14+
Some(records),
15+
);
16+
Ok::<_, Error>(())
17+
})
18+
}

‎relay-server/src/processing/common.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::Envelope;
22
use crate::managed::{Managed, Rejected};
33
use crate::processing::ForwardContext;
4+
use crate::processing::attachments::AttachmentProcessor;
45
use crate::processing::check_ins::CheckInsProcessor;
56
use crate::processing::errors::ErrorsProcessor;
67
use crate::processing::logs::LogsProcessor;
@@ -68,4 +69,5 @@ outputs!(
6869
TraceAttachments => TraceAttachmentsProcessor,
6970
TraceMetrics => TraceMetricsProcessor,
7071
Replays => ReplaysProcessor,
72+
Attachments => AttachmentProcessor,
7173
);

‎relay-server/src/processing/errors/process.rs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,19 @@ pub fn normalize(
126126
}
127127

128128
pub fn scrub(error: &mut Managed<ExpandedError>, ctx: Context<'_>) -> Result<(), Rejected<Error>> {
129-
error.try_modify(|error, _| {
129+
error.try_modify(|error, records| {
130130
processing::utils::event::scrub(&mut error.event, ctx.project_info)?;
131-
processing::utils::attachments::scrub(error.attachments.iter_mut(), ctx.project_info);
131+
processing::utils::attachments::scrub(
132+
error.attachments.iter_mut(),
133+
ctx.project_info,
134+
Some(records),
135+
);
132136
if let Some(minidump) = error.data.minidump_mut() {
133-
processing::utils::attachments::scrub(std::iter::once(minidump), ctx.project_info);
137+
processing::utils::attachments::scrub(
138+
std::iter::once(minidump),
139+
ctx.project_info,
140+
Some(records),
141+
);
134142
}
135143

136144
Ok::<_, Error>(())

‎relay-server/src/processing/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub use self::common::*;
2323
pub use self::forward::*;
2424
pub use self::limits::*;
2525

26+
pub mod attachments;
2627
pub mod check_ins;
2728
pub mod client_reports;
2829
pub mod errors;

‎relay-server/src/processing/trace_attachments/store.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ use crate::processing::utils::store::{
1313
AttributeMeta, extract_client_sample_rate, extract_meta_attributes, proto_timestamp,
1414
quantities_to_trace_item_outcomes, uuid_to_item_id,
1515
};
16-
use crate::services::objectstore::StoreAttachment;
16+
use crate::services::objectstore::StoreTraceAttachment;
1717
use crate::services::outcome::{DiscardReason, Outcome};
1818

1919
/// Converts an expanded attachment to a storable unit.
2020
pub fn convert(
2121
attachment: Managed<ExpandedAttachment>,
2222
retention: Retention,
2323
server_sample_rate: Option<f64>,
24-
) -> Result<Managed<StoreAttachment>, Rejected<()>> {
24+
) -> Result<Managed<StoreTraceAttachment>, Rejected<()>> {
2525
let scoping = attachment.scoping();
2626
let received_at = attachment.received_at();
2727
attachment.try_map(|attachment, _record_keeper| {
@@ -42,7 +42,7 @@ pub fn convert(
4242
let trace_item = attachment_to_trace_item(meta, quantities, ctx)
4343
.ok_or(Outcome::Invalid(DiscardReason::InvalidTraceAttachment))?;
4444

45-
Ok::<_, Outcome>(StoreAttachment { trace_item, body })
45+
Ok::<_, Outcome>(StoreTraceAttachment { trace_item, body })
4646
})
4747
}
4848

‎relay-server/src/processing/transactions/process.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,9 @@ pub fn scrub(
431431
work: Managed<Box<ExpandedTransaction>>,
432432
ctx: Context<'_>,
433433
) -> Result<Managed<Box<ExpandedTransaction>>, Rejected<Error>> {
434-
work.try_map(|mut work, _| {
434+
work.try_map(|mut work, records| {
435435
utils::event::scrub(&mut work.event, ctx.project_info)?;
436-
utils::attachments::scrub(work.attachments.iter_mut(), ctx.project_info);
436+
utils::attachments::scrub(work.attachments.iter_mut(), ctx.project_info, Some(records));
437437
Ok::<_, Error>(work)
438438
})
439439
}

0 commit comments

Comments
 (0)