ref(logs): Process logs in all non-proxy Relays #4973
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dav1dde
force-pushed
the
dav1d/process-logs-early
branch
2 times, most recently
from
a9df6b2 to
9f30258
Compare
Dav1dde
changed the title
Dav1dde
force-pushed
the
dav1d/process-logs-early
branch
10 times, most recently
from
8aed8e2 to
4067cf8
Compare
Dav1dde
force-pushed
the
dav1d/process-logs-early
branch
from
4067cf8 to
0a9d954
Compare
loewenheim
reviewed
Jul 30, 2025
| if ctx.is_processing() { | ||
| let mut logs = process::expand(logs, ctx); | ||
| process::process(&mut logs, ctx); | ||
| self.limiter.enforce_quotas(&mut logs, ctx).await?; |
There was a problem hiding this comment.
This means quota enforcement now happens after expansion and processing, when it was the other way around before. I assume that's intentional?
There was a problem hiding this comment.
Not yet relevant, will be relevant once we have inbound filters. We don't want to count things we're going to drop.
Also technically wrong before.
| @@ -247,7 +247,7 @@ pub struct RequestMeta<D = PartialDsn> { | |||
| /// | |||
| /// NOTE: This is internal-only and not exposed to Envelope headers. | |||
| #[serde(skip)] | |||
| from_internal_relay: bool, | |||
| request_trust: Option<RequestTrust>, | |||
There was a problem hiding this comment.
Is there an actual advantage to making this optional? AFAICT None has the same meaning as Some(Untrusted) in all cases.
There was a problem hiding this comment.
Yes, copy_to will now handle missing correctly, where before it would not. Also technically a bug that existed before and now became obvious with more typing.
loewenheim
approved these changes
Jul 30, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first big step at supporting inbound filters for logs.
Expansion and normalization of logs must happen in the same stage as inbound filters, since inbound filters must run at the earliest point possible to minimize unnecessary processing/traffic the normalization must also happen then.
This change enables processing of logs in all Relays except Relays running in proxy mode. Proxy mode does not have access to the necessary processing config to apply inbound filters or do full processing.
As a side effect, static and other customer managed Relays will now (correctly) apply PII rules, as they always should've been.
Tests have been improved to always use a Relay chain, size assertions are done on a trusted and untrusted Relay chain.
I also replaced the
is_from_trusted_relaywith a proper enum to not run into an issue where somewhat important information is passed around as a boolean. Adds some type-safety.Things to be done in a follow-up:
Closes: #4990
Closes: INGEST-469