feat(logs): Implement container version 2 by Dav1dde · Pull Request #5887 · getsentry/relay

Dav1dde · 2026-04-24T09:39:36Z

Like #5881 but for logs. Refs: #5869

Integrations will no longer infer the user agent, for log drains this never made sense for OTLP in most cases I would argue it doesn't make sense, as the source often is a drain (e.g. cloudflare) or a collector.

Changes the signature from the integration expansion from a vec of items to a single item, this now leaves potentially a better way to deal with error cases (using actual errors), I will explore this in a follow-up/cleanup which also will address the span integrations (INGEST-883).

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 43e6944. Configure here.}

k-fish · 2026-04-27T16:14:04Z

+/// | Version |      client ip     | user agent |
+/// |---------|--------------------|------------|
+/// | none    | {{auto}}           | always     |
+/// | 2       | {{auto}}, settings | settings   |


Is there a way via settings to turn off the auto behaviour?

No, is there a reason this would be necessary? {{auto}} is an explicit opt-in from the client. That being said, happy to drop support for {{auto}} altogether and force the use of settings.

Do you know if any SDK even ever sent {{auto}}?

Checked with @chargome, SDKs do send it. We could drop support with version: 2. Gonna take a look, right now code wise, it's easier to always do it, then to conditionally turn it off.

@k-fish are you referring to the behavior when the value is missing, or to the behavior when the value is {{auto}}? The former can be disabled by settings infer_ip: never, the latter cannot be disabled other than not sending it (what @Dav1dde said).

k-fish · 2026-04-27T16:14:27Z

+///
+/// For example in version 2, the client ip is inferred if either the client address attribute is
+/// set to `auto` or the ingest settings specify `infer_ip` to `autp`.
+/// The user agent is only considered if ingest settings specify `infer_user_agent` as `auto`.


Do sdks default to auto? I think this will affect behaviour just want to check the default path is still extracting browser name

Old SDKs will not send a version, which defaults to never. New SDKs (starting with an upcoming version) imo should default to version 2 and auto if it's a "frontend" SDK

k-fish · 2026-04-27T16:17:02Z

                },
                "sentry.origin": {"stringValue": "auto.otlp.logs"},
-                "sentry.payload_size_bytes": {"intValue": "378"},
+                "sentry.payload_size_bytes": {"intValue": mock.ANY},


mock.ANY? shouldn't this be a value to ensure we don't drift?

That mirrors the vercel tests and I realized it is quite annoying to change the value in a lot of places when we have dedicated tests already making sure the counts are correct. Overall not a strong opinion, happy to bring it back if you/we think there is value in the assertion.

k-fish · 2026-04-27T16:17:12Z

+        // Log byte counts will change here as we go from an estimated count based on the body, to
+        // accurately counted bytes.
+        records.lenient(DataCategory::LogByte);



Do you have a feeling if this will cause a large deviation to peoples byte counts?

Do you have a feeling if this will cause a large deviation to peoples byte counts?

Right now, it's probably somewhat close and a relatively constant ratio, but in the future this may change drastically and that was the intention. E.g. if we come up with a format where we allow SDKs to inherit common attributes into each individual log (aka some kind of compression).

loewenheim

LGTM aside from a stylistic nit.

jjbayer · 2026-04-29T11:18:14Z

+        skip_serializing_if = "Option::is_none",
+        deserialize_with = "utils::none_on_error"
+    )]
+    pub infer_user_agent: Option<InferUserAgent>,


Why do these have to be Option? Could they fall back to an enum variant instead?

Option handles missing and error decently well for serialization and de-serialization.

Without the option, we either need to default to a value, in which case different protocol version no longer can have different behaviours (v1 missing is auto, v2 is never).

I considered an error variant, but it is significantly more annoying to deal with, requires a custom serialize impl (and skip_serializing_if function), with no upside (at this time).

jjbayer · 2026-04-29T11:20:36Z

+/// Specifically frontend SDKs may want to have the user agent and browser information automatically inferred
+/// from the HTTP header provided user agent.
+#[derive(Debug, Copy, Clone, Deserialize, Serialize)]
+pub enum InferUserAgent {


Any reason InferIp and InferUserAgent are separate types? Could they be united into something like InferBehavior?

I am not sure if these are good enough reasons tbh:

Allow easy independent changes to the protocol (similar to duplicating the container.rs)

Better doc strings

Slightly stricter typing.

Happy to change, really on the edge on this.

Two types are fine with me 👍

jjbayer · 2026-04-29T11:24:04Z

-            records.modify_by(DataCategory::LogItem, 1);
-            records.modify_by(DataCategory::LogByte, byte_size as isize);
+    let mut logs = Vec::new();
+    let produce = |log: OurLog| {


Is produce basically the merge_into that @elramen is writing (just less generic)?

Kinda, but I am not sure if we can use it here, this code is already in a try_modify. Gonna look into it in the follow-up.

Dav1dde requested a review from a team as a code owner April 24, 2026 09:39

Dav1dde force-pushed the dav1d/dav1d/eap-ingest-settings-logs branch from c6f6cd7 to 462e585 Compare April 24, 2026 09:40

Dav1dde self-assigned this Apr 24, 2026

cursor Bot reviewed Apr 24, 2026

View reviewed changes

Comment thread relay-server/src/processing/logs/integrations/mod.rs Outdated

Dav1dde force-pushed the dav1d/dav1d/eap-ingest-settings-logs branch 2 times, most recently from 58781d3 to 43e6944 Compare April 24, 2026 10:17

cursor Bot reviewed Apr 24, 2026

View reviewed changes

Comment thread relay-event-schema/src/protocol/ourlog/container.rs

Dav1dde force-pushed the dav1d/dav1d/eap-ingest-settings-logs branch from 43e6944 to 6e057b0 Compare April 24, 2026 10:32

sentry Bot reviewed Apr 24, 2026

View reviewed changes

Comment thread relay-server/src/processing/logs/mod.rs Outdated

Dav1dde force-pushed the dav1d/dav1d/eap-ingest-settings-logs branch from 6e057b0 to 5a6087d Compare April 24, 2026 10:38

Dav1dde requested a review from k-fish April 24, 2026 11:49

k-fish reviewed Apr 27, 2026

View reviewed changes

feat(logs): Implement container version 2

c6c5616

Dav1dde force-pushed the dav1d/dav1d/eap-ingest-settings-logs branch from 5a6087d to 895c13f Compare April 28, 2026 11:16

code review

0921b72

Dav1dde force-pushed the dav1d/dav1d/eap-ingest-settings-logs branch from 895c13f to 0921b72 Compare April 28, 2026 11:17

sentry Bot reviewed Apr 28, 2026

View reviewed changes

Comment thread relay-server/src/processing/logs/process.rs

fix nel settings

391c433

loewenheim reviewed Apr 29, 2026

View reviewed changes

Comment thread relay-server/src/processing/logs/integrations/mod.rs Outdated

review change, match

c5690db

sentry Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread relay-server/src/processing/logs/process.rs

test(kafka): Make Kafka configuration and consumers more resilient

42b68ae

loewenheim approved these changes Apr 29, 2026

View reviewed changes

jjbayer reviewed Apr 29, 2026

View reviewed changes

review words

7a05a7f

jjbayer approved these changes Apr 29, 2026

View reviewed changes

Merge branch 'master' into dav1d/dav1d/eap-ingest-settings-logs

48e5518

Conversation

Dav1dde commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

loewenheim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Dav1dde commented Apr 24, 2026 •

edited

Loading