feat(ourlogs): Allow log ingestion behind a flag #4448
Conversation
This adds log ingestion (currently only working for the OTel log format) behind a feature flag 'organizations:ourlogs-ingestion'. This PR aims to be the minimum possible to support local and test-org ingestion before we move to dogfooding. Other notes: - We need to add two DataCategories because we need to track quantity (for current discarded breadcrumb ClientOutcome tracking) and also bytes for total log bytes ingested, which is one of the quota recommendations. - Eventually we will convert Breadcrumbs into logs as well, very similar to span extraction for spans on the event. How exactly that will work is still being discussed with product and sdk folks. - The name 'ourlogs' is an internal name to disambiguate between 'our log product' logs and internally created logs. User facing strings will be set to 'Log' to avoid exposing implementation details.
k-fish
force-pushed
the
feat/ourlogs/ingest-log-items
branch
from
dc3eba0 to
3f6f6f9
Compare
| pub struct OurLog { | ||
| /// Time when the event occurred. | ||
| #[metastructure(required = true, trim = false)] | ||
| pub timestamp_nanos: Annotated<u64>, |
There was a problem hiding this comment.
Other data types accept both unix timestamps and formatted date strings, although they do not have nanosecond precision:
relay/relay-event-schema/src/protocol/span.rs
Lines 16 to 18 in 4622914
Are nanos required for logs?
There was a problem hiding this comment.
OTel defines them as nanos, the consumers will consume nanos, and they are stored as nanos, we may need breadcrumbs to switch from floats to nanos but otherwise I think we are leaning towards keeping it the same format throughout instead of having slightly different intermediate formats.
This is pulling out data categories from #4448 as their own PR.
k-fish
changed the base branch from
master
to
feat/ourlogs/add-data-categories
k-fish
changed the base branch from
feat/ourlogs/add-data-categories
to
master
Co-authored-by: Joris Bayer <[email protected]>
There was a problem hiding this comment.
Looking good! It would be nice to have at least one integration test, see for example
relay/tests/integration/test_spans.py
Line 472 in bbe569d
| // We need to track the count and bytes separately for possible rate limits and quotas on both counts and bytes. | ||
| self.outcome_aggregator.send(TrackOutcome { | ||
| category: DataCategory::LogItem, | ||
| event_id: None, | ||
| outcome: Outcome::Accepted, | ||
| quantity: 1, | ||
| remote_addr: None, | ||
| scoping, | ||
| timestamp: received_at, | ||
| }); | ||
| self.outcome_aggregator.send(TrackOutcome { | ||
| category: DataCategory::LogByte, | ||
| event_id: None, | ||
| outcome: Outcome::Accepted, | ||
| quantity: payload_len as u32, | ||
| remote_addr: None, | ||
| scoping, | ||
| timestamp: received_at, | ||
| }); |
There was a problem hiding this comment.
I'm not a big fan of producing "accepted" outcomes in Relay. Ideally we would produce them as close to storage as possible, e.g. after storing the data in Snuba, so that we do not overcount items when data gets lost between Relay and storage. For spans, we moved the "accepted" outcomes to Relay because there was simply no consumer in sentry that could do it, and there was no precedence for creating outcomes in Snuba itself.
There was a problem hiding this comment.
Has that changed now? Is there prior art to doing it in a snuba consumer etc.? I was actually thinking about that while adding them here because of the possibility of rejecting the item later on.
Co-authored-by: Joris Bayer <[email protected]>
Co-authored-by: Joris Bayer <[email protected]>
Co-authored-by: Joris Bayer <[email protected]>
Co-authored-by: Joris Bayer <[email protected]>
### Summary This is pulling out data categories from #4448 as their own PR. --------- Co-authored-by: Joris Bayer <[email protected]>
This adds log ingestion (currently only working for the OTel log format) behind a feature flag 'organizations:ourlogs-ingestion'.
This PR aims to be the minimum possible to support local and test-org ingestion before we move to dogfooding.
Other notes:
DataCategorys because we need to track quantity (for current discarded breadcrumb client outcome tracking) and also bytes for total log bytes ingested, which is one of the quota recommendations.ourlogsis an internal name to disambiguate between 'our log product' logs (corny, I know) and internally created logs. User facing strings will be set to 'Log' to avoid exposing implementation details.