Skip to content
This repository was archived by the owner on Dec 1, 2025. It is now read-only.

Spec: Aggregate error reporting#172

Merged
alexmturner merged 7 commits into
mainfrom
error_reporting
Mar 3, 2025
Merged

Spec: Aggregate error reporting#172
alexmturner merged 7 commits into
mainfrom
error_reporting

Conversation

@alexmturner
Copy link
Copy Markdown
Collaborator

@alexmturner alexmturner commented Feb 24, 2025

Adds support for aggregate error reporting. Additional changes are required to the embedding APIs' specs to support the feature.


Preview | Diff

Adds support for aggregate error reporting. Additional changes are
required to the embedding APIs' specs to support the feature.
@alexmturner
Copy link
Copy Markdown
Collaborator Author

@dmcardle, could you PTAL? Thanks!

Copy link
Copy Markdown
Contributor

@dmcardle dmcardle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Alex! Just made one pass through and left some questions and nits.

Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs
Copy link
Copy Markdown
Collaborator Author

@alexmturner alexmturner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Dan!

Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs
Copy link
Copy Markdown
Contributor

@dmcardle dmcardle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless of my last round of comments, this LGTM at this point :)

@alexmturner
Copy link
Copy Markdown
Collaborator Author

@dmcardle
Copy link
Copy Markdown
Contributor

Uhh not sure why my recent replies aren't both showing up on this thread, but here they are:

Gah! Another one for the gripe list! I noticed this a couple days ago, too.

Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs Outdated
Comment thread spec.bs
Copy link
Copy Markdown
Contributor

@dmcardle dmcardle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, those edits look great. SLGTM!

Comment thread spec.bs Outdated
Comment thread spec.bs
Comment thread spec.bs
@alexmturner
Copy link
Copy Markdown
Collaborator Author

Great, thanks for reviewing!

@alexmturner alexmturner merged commit e5804a7 into main Mar 3, 2025
@alexmturner alexmturner deleted the error_reporting branch March 3, 2025 21:25
github-actions Bot added a commit that referenced this pull request Mar 3, 2025
SHA: e5804a7
Reason: push, by alexmturner

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
alexmturner added a commit to WICG/shared-storage that referenced this pull request Mar 7, 2025
Adds support for the new Private Aggregation error reporting feature to
Shared Storage. See the related PAA spec change:
patcg-individual-drafts/private-aggregation-api#172
Also see the explainer:
https://github.com/patcg-individual-drafts/private-aggregation-api/blob/main/error_reporting.md

Slightly reorganizes the PAA integration with this spec for readability.
aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 10, 2025
Replaces usage of "non-internal" to "external" to better match the spec:
patcg-individual-drafts/private-aggregation-api#172

Bug: 381788013
Change-Id: If139ff27eb74f6467adf2649c7bc9b0ccb7aefd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6335576
Reviewed-by: Dan McArdle <dmcardle@chromium.org>
Auto-Submit: Alex Turner <alexmt@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@google.com>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/main@{#1430543}
alexmturner added a commit to WICG/shared-storage that referenced this pull request Mar 19, 2025
Adds support for the new Private Aggregation error reporting feature to
Shared Storage. See the related PAA spec change:
patcg-individual-drafts/private-aggregation-api#172
Also see the explainer:
https://github.com/patcg-individual-drafts/private-aggregation-api/blob/main/error_reporting.md

Slightly reorganizes the PAA integration with this spec for readability.
@alexmturner
Copy link
Copy Markdown
Collaborator Author

alexmturner commented Mar 23, 2025

Note that spec PRs for integrating with Shared Storage and Protected Audience can be found here:

aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 28, 2025
Adds support for the kPrivateAggregationApiErrorReporting feature*,
including the new flow involving two calls to the interface: first,
`InspectBudgetAndLock()`; and then, `ConsumeBudget()`. The old flow
(when the feature is disabled) is also maintained for now.

When the feature is disabled (which it is by default), this cl should
have no effect. These new calls are also not yet integrated with the
manager; this will be done in a follow-up cl.

*This feature is described further here:
https://github.com/patcg-individual-drafts/private-aggregation-api/blob/main/error_reporting.md
It was specified here:
patcg-individual-drafts/private-aggregation-api#172

Bug: 381788013
Change-Id: Iccd452826b887e1de39a33ab5344de2de6774978
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6304832
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Dan McArdle <dmcardle@chromium.org>
Commit-Queue: Alex Turner <alexmt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1439738}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants