feat(eap): Produce logs to the items topic #4707

phacops · 2025-05-06T14:40:42Z

Instead of maintaining a separate schema, topic and consumer to ingest logs into the EAP, we're using the new items topic to ingest them.

To do:

target the right topic
change how the message is serialized

Needs getsentry/pypi#1427

Plan to deploy pause the deploys for DE/US, push to S4S, look at logs ingested after the deploy, revert/move to DE.

relay-server/Cargo.toml

relay-server/src/services/store.rs

Dav1dde

I'll have to look more into the specifics and explore options, but for now I do not think we should make protoc a dependency of Relay's build. I can't think of a good reason why this would be necessary. Ideally the proto crate, just contains the generated code instead of relying on a build.rs. opentelemetry-proto also does not force you to have protoc.

.github/workflows/build_binary.yml

relay-server/src/services/store.rs

This reverts commit a4dbeb8.

This reverts commit d57223f.

.github/workflows/ci.yml

Cargo.toml

relay-kafka/Cargo.toml

relay-kafka/src/config.rs

relay-ourlogs/src/ourlog.rs

relay-server/src/services/store.rs

Dav1dde · 2025-05-08T12:55:13Z

relay-server/src/services/store.rs

+                            LogAttributeValue::Int(value) => AnyValue {
+                                value: Some(Value::IntValue(value)),
+                            },


That was already a bug before I think, this will drop logs with an int value > i64::MAX.

I'll take care of that.

relay-server/src/services/store.rs

k-fish

The item_id is correctly using uuidv7 is my only real concern. We are currently using timestamp_precise is ordering, but we can switch to the item id if it's a stable replacement.

relay-ourlogs/src/ourlog.rs

relay-server/src/services/store.rs

k-fish · 2025-05-08T14:26:35Z

tests/integration/test_ourlogs.py

+                item_id=timestamp_nanos.to_bytes(
+                    length=16,
+                    byteorder="little",
+                    signed=False,
+                ),


Does this work? Should the uuidv7 have a random portion when NoContext is set? I think if it's set to 0 we may have collision problems

This doesn't work, I couldn't find a proper uuid7 library in Python to make the uuid based on a timestamp. That's why I replace it in the response lower.

Python 3.14 will have a function for this in the standard library.

I was also looking at the output (eg. 'itemId': 'quqO1GMfE61WcAAAAAAAAA=='), isn't the random portion not random here? It seems to be repeating. I am really only concerned that the uuidv7 implementation here wasn't working properly.

If the integration isn't passing maybe just confirm the timestamp portion of the actual log item_id matches the nanos in hex?

This is repeated because I'm using the item_id coming from Relay. We can't properly make a uuid7 from a custom timestamp in Python just yet so I am just ignoring this value.

The way the uuid7 is generated from the timestamp in Rust is correct though, and has this random portion.

k-fish

Works for me if the uuidv7 is generating itemid properly. You're missing timestamp precise in the integration tests still, but that's a simple fix.

relay-server/src/services/store.rs

k-fish · 2025-05-08T17:51:35Z

tests/integration/test_ourlogs.py

+                item_id=timestamp_nanos.to_bytes(
+                    length=16,
+                    byteorder="little",
+                    signed=False,
+                ),


I was also looking at the output (eg. 'itemId': 'quqO1GMfE61WcAAAAAAAAA=='), isn't the random portion not random here? It seems to be repeating. I am really only concerned that the uuidv7 implementation here wasn't working properly.

If the integration isn't passing maybe just confirm the timestamp portion of the actual log item_id matches the nanos in hex?

k-fish

Just running this on local, will post back any changes required after that

k-fish · 2025-05-09T15:35:58Z

tests/integration/test_ourlogs.py

+                    ),
+                    "string.attribute": AnyValue(string_value="some string"),
+                    "valid_string_with_other": AnyValue(string_value="test"),
+                    "sentry.timestamp_precise": AnyValue(int_value=timestamp_nanos),


Is this right or should this be a string_value? int_value is i64 which isn't correct for nanos

sentry.timestamp_precise is an integer in the current logs processor https://github.com/getsentry/snuba/blob/master/rust_snuba/src/processors/ourlogs.rs#L141-L144.

tobias-wilfert · 2025-05-13T12:11:26Z

So the way we do timestamps here is now causing CI to fail: https://github.com/getsentry/relay/actions/runs/14974415036/job/42062971176?pr=4727

FAILED tests/integration/test_ourlogs.py::test_ourlog_extraction_with_sentry_logs - AssertionError: assert [{'attributes...PE_LOG', ...}] == [{'attributes...PE_LOG', ...}]
  At index 0 diff: {'organizationId': '1', 'projectId': '42', 'traceId': '5b8efff798038103d269b633813fc60c', 'itemId': 'qp23BQ9zaoeRcJ7l1sSWAQ==', 'itemType': 'TRACE_ITEM_TYPE_LOG', 'timestamp': '2025-05-12T14:12:19Z', 'attributes': {'sentry.severity_text': {'stringValue': 'error'}, 'sentry.observed_timestamp_nanos': {'stringValue': '1747059139998272000'}, 'sentry.timestamp_precise': {'intValue': '1747059139998272000'}, 'sentry.span_id': {'stringValue': 'eee19b7ec3c1b175'}, 'sentry.timestamp_nanos': {'stringValue': '1747059139998272000'}, 'sentry.trace_flags': {'intValue': '0'}, 'sentry.severity_number': {'intValue': '17'}, 'sentry.body': {'stringValue': 'This is really bad'}}, 'clientSampleRate': 1.0, 'serverSampleRate': 1.0, 'retentionDays': 90, 'received': '2025-05-12T14:12:20Z'} != {'organizationId': '1', 'projectId': '42', 'traceId': '5b8efff798038103d269b633813fc60c', 'itemId': 'qp23BQ9zaoeRcJ7l1sSWAQ==', 'itemType': 'TRACE_ITEM_TYPE_LOG', 'timestamp': '2025-05-12T14:12:19Z', 'attributes': {'sentry.severity_text': {'stringValue': 'error'}, 'sentry.severity_number': {'intValue': '17'}, 'sentry.timestamp_precise': {'intValue': '1747059139998272000'}, 'sentry.trace_flags': {'intValue': '0'}, 'sentry.span_id': {'stringValue': 'eee19b7ec3c1b175'}, 'sentry.observed_timestamp_nanos': {'stringValue': '1747059139998272000'}, 'sentry.timestamp_nanos': {'stringValue': '1747059139998272000'}, 'sentry.body': {'stringValue': 'This is really bad'}}, 'clientSampleRate': 1.0, 'serverSampleRate': 1.0, 'retentionDays': 90, 'received': '2025-05-12T14:12:19Z'}
  Full diff:
    [
     {'attributes': {'sentry.body': {'stringValue': 'This is really bad'},
                     'sentry.observed_timestamp_nanos': {'stringValue': '1747059139998272000'},
                     'sentry.severity_number': {'intValue': '17'},
                     'sentry.severity_text': {'stringValue': 'error'},
                     'sentry.span_id': {'stringValue': 'eee19b7ec3c1b175'},
                     'sentry.timestamp_nanos': {'stringValue': '1747059139998272000'},
                     'sentry.timestamp_precise': {'intValue': '1747059139998272000'},
                     'sentry.trace_flags': {'intValue': '0'}},
      'clientSampleRate': 1.0,
      'itemId': 'qp23BQ9zaoeRcJ7l1sSWAQ==',
      'itemType': 'TRACE_ITEM_TYPE_LOG',
      'organizationId': '1',
      'projectId': '42',
  -   'received': '2025-05-12T14:12:19Z',
  ?                                 ^^
  +   'received': '2025-05-12T14:12:20Z',
  ?                                 ^^

tobias-wilfert · 2025-05-13T12:36:53Z

Looking into fixing this

feat(eap): Produce logs to the items topic

0aa91e4

Dav1dde reviewed May 6, 2025

View reviewed changes

relay-server/Cargo.toml Outdated Show resolved Hide resolved

relay-server/src/services/store.rs Outdated Show resolved Hide resolved

relay-server/src/services/store.rs Outdated Show resolved Hide resolved

relay-server/src/services/store.rs Outdated Show resolved Hide resolved

phacops added 4 commits May 6, 2025 12:01

Vendor sentry_protos

3e2b852

Gate sentry_protos dependency behind the processing feature

5332d1c

Use the proper enum value

d5ddc89

Do not use the macro

a401c2f

phacops requested a review from Dav1dde May 6, 2025 16:11

phacops added 2 commits May 6, 2025 14:54

Serialize in the function and return the raw bytes in serialize()

697763a

Read the items portion of the config

74e809d

phacops marked this pull request as ready for review May 6, 2025 19:19

phacops requested a review from a team as a code owner May 6, 2025 19:19

phacops added 7 commits May 6, 2025 15:58

Generate proper types for Protobuf schema validation

1be1453

Remove vendored in favor of installing protoc

9a779f8

Enable type_generation for sentry-kafka-schemas

8d21e2b

Install protoc to all workflows needing it

d57223f

Add missing workflows

a4dbeb8

Add a CHANGELOG entry

ad27999

Fix some tests

095c26f

Dav1dde requested changes May 7, 2025

View reviewed changes

.github/workflows/build_binary.yml Outdated Show resolved Hide resolved

relay-server/src/services/store.rs Outdated Show resolved Hide resolved

phacops added 7 commits May 7, 2025 17:47

Revert "Add missing workflows"

90462cc

This reverts commit a4dbeb8.

Revert "Install protoc to all workflows needing it"

f53c73a

This reverts commit d57223f.

Upgrade sentry_protos and sentry-kafka-schemas so we don't need protoc

dd26cab

Fix tests

08c7886

Move some logic to the normalization

d78d272

Fix normalization tests

57761af

Fix one more test

86ca4aa

phacops requested a review from Dav1dde May 8, 2025 00:31

Dav1dde reviewed May 8, 2025

View reviewed changes

phacops added 2 commits May 8, 2025 07:20

Remove another installation of protoc in CI

c9fe378

Order crates alphabetically and remove types_generation feature

1e3e6ac

Use to_owned() instead of to_string()

6dc69a6

phacops requested a review from Dav1dde May 8, 2025 11:46

phacops added 3 commits May 8, 2025 08:31

Disable default features again now the crate is fixed

387b59f

Really disable default features for sentry-kafka-schemas

02f7c1a

Can't make it with less default features than this

cf2475b

Dav1dde approved these changes May 8, 2025

View reviewed changes

phacops added 4 commits May 8, 2025 09:37

Remove Serialization trait for logs

abc0f44

Move Protobuf encoding to serialize

79ed892

Use to_owned() instead of to_string() where we can

b7eb2b6

Remove nanoseconds from this timestamp

b00ab99

k-fish reviewed May 8, 2025

View reviewed changes

phacops added 2 commits May 8, 2025 11:24

Remove other and add back sentry.timestamp_precise

ba2fdeb

Fix tests

0d25ca7

phacops requested a review from k-fish May 8, 2025 15:32

k-fish approved these changes May 8, 2025

View reviewed changes

phacops added 2 commits May 9, 2025 11:04

Fix integration tests

33ae11f

Update snapshots

5633637

k-fish reviewed May 9, 2025

View reviewed changes

phacops added 2 commits May 9, 2025 18:55

Add a better comment on why we replace the item_id

b90d31b

Fix linting

1cda6e9

phacops merged commit 2f7c8b6 into master May 12, 2025
27 checks passed

phacops deleted the pierre/eap-logs-to-items-topic branch May 12, 2025 12:20

tobias-wilfert mentioned this pull request May 13, 2025

fix(eap): Fix flakey timestamp assertion #4734

Merged

feat(eap): Produce logs to the items topic #4707

feat(eap): Produce logs to the items topic #4707

Uh oh!

Conversation

phacops commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dav1dde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

k-fish left a comment

Choose a reason for hiding this comment

Uh oh!

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!

k-fish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-fish left a comment

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!

tobias-wilfert commented May 13, 2025

Uh oh!

tobias-wilfert commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

phacops commented May 6, 2025 •

edited

Loading

tobias-wilfert commented May 13, 2025 •

edited

Loading