Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Noised sources with no reports are handled differently from noised ones with reports #1277

Open
apasel422 opened this issue May 8, 2024 · 3 comments
Assignees
Labels
bug Something isn't working debugging-monitoring Issues relating to debugging, locally or at scake documentation

Comments

@apasel422
Copy link
Collaborator

In step 2 of "trigger event-level attribution" we return early with a trigger-event-noise verbose debug report when the attributed source has a non-zero-report randomized response, but we do not return early when it has zero-report randomized response, which ends up causing subsequent early-return-verbose-debug-reports to be favored over the trigger-event-noise one. For example, step 5 can end up producing a trigger-event-no-matching-configurations verbose debug report, even though attribution would still have failed even if there had been a matching configuration, due to the handling of noise in step 22.

This seems inconsistent. Is there a reason the non-zero-report one always gets trigger-event-noise but the zero-report one only gets that verbose debug report some of the time? It seems like a strange, and perhaps unintentional, mix of non-noised behavior with noised behavior.

Similarly, verbose debug reports issued before this algorithm is called (e.g. trigger-no-matching-filter-data) are also favored over the one specifically for noise.

@apasel422 apasel422 added bug Something isn't working debugging-monitoring Issues relating to debugging, locally or at scake documentation labels May 8, 2024
@johnivdel
Copy link
Collaborator

The verbose debugging reports were added to the API after the noising behavior. So I don't think this is intentional, and agree that trigger-event-noise better reflects how the browser is handling the attribution.

I think a different question is whether we need to execute all the steps of that algorithm in the zero-report case. It seems to be the only difference this makes to the state maintained by storage is in steps 23 and 24. Which is also strange, as this suggests we track dedup keys for sources with an empty randomized response?

I don't think that is load-bearing, so step 23 which increments the number of event level reports is the only observable difference. This will eventually result in deactivating the source, which would not happen otherwise. Does that seem like a correct interpretation of the spec?

@apasel422
Copy link
Collaborator Author

Yes, it does seem that the spec maintains deduplication keys for the zero-report case, which IIUC is unnecessary.

It'd be great to reorganize all of this, and perhaps even explicitly list the preferential ordering of verbose debug reports when it is significant.

I also wonder if the trigger-event-noise verbose debug report should exist at all.

@apasel422
Copy link
Collaborator Author

CC @csharrison

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working debugging-monitoring Issues relating to debugging, locally or at scake documentation
Projects
None yet
Development

No branches or pull requests

2 participants