-
Notifications
You must be signed in to change notification settings - Fork 60
Forward ext.metadata (privTags) on the cross-platform serialization path #1472
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
base: main
Are you sure you want to change the base?
Changes from all commits
3b809f9
388801b
d4e50a5
50770ae
36be603
9227dfc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
|
|
||
| #include "IDecorator.hpp" | ||
| #include "EventProperties.hpp" | ||
| #include "CommonFields.h" | ||
| #include "CorrelationVector.hpp" | ||
| #include "utils/Utils.hpp" | ||
|
|
||
|
|
@@ -172,6 +173,23 @@ namespace MAT_NS_BEGIN { | |
| } | ||
| const auto &k = kv.first; | ||
| const auto &v = kv.second; | ||
|
|
||
| // Route the privacy tag (EventInfo.PrivTags) into ext.metadata.privTags so the | ||
| // cross-platform bond path carries it the same way the UTC path does. extMetadata | ||
| // is the single source of truth: a well-typed int64 value is moved out of Part C | ||
| // (the JSON path reads it from extMetadata too). Non-int64 values are left to be | ||
| // handled as an ordinary property below. | ||
| // TODO(privacy-parity): confirm the canonical Common Schema ext.metadata wire contract. | ||
| if (k == COMMONFIELDS_EVENT_PRIVTAGS && v.type == EventProperty::TYPE_INT64) | ||
| { | ||
| if (record.extMetadata.empty()) | ||
| { | ||
| record.extMetadata.push_back(::CsProtocol::MetaData()); | ||
| } | ||
| record.extMetadata[0].privTags = static_cast<uint64_t>(v.as_int64); | ||
|
Comment on lines
+185
to
+189
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed - extMetadata is now the single source of truth. The decorator moves a well-typed int64 EventInfo.PrivTags into ext.metadata.privTags and no longer leaves it in Part C (the continue is back), so the Bond payload no longer carries it twice. |
||
| continue; | ||
| } | ||
|
Comment on lines
+189
to
+191
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right - addressed in the same push. Since the decorator now moves
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update for the final approach: I removed the continue, so EventInfo.PrivTags is now ADDITIONALLY mirrored into ext.metadata.privTags but kept in Part C. The JSON path reads it from Part C unchanged, so nothing is dropped. |
||
|
|
||
| if (v.piiKind != PiiKind_None) | ||
| { | ||
| if (v.piiKind == PiiKind::CustomerContentKind_GenericData) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -330,6 +330,22 @@ struct M365a { | |
| } | ||
| }; | ||
|
|
||
| struct MetaData { | ||
| // 1: optional uint64 privTags | ||
| // TODO(privacy-parity): best-effort layout; confirm canonical Common Schema ext.metadata. | ||
| uint64_t privTags = 0; | ||
|
|
||
| bool operator==(MetaData const& other) const | ||
| { | ||
| return (privTags == other.privTags); | ||
| } | ||
|
|
||
| bool operator!=(MetaData const& other) const | ||
| { | ||
| return !(*this == other); | ||
| } | ||
| }; | ||
|
|
||
| struct Xbl { | ||
| // 5: optional map<string, string> claims | ||
| std::map<std::string, std::string> claims; | ||
|
|
@@ -1012,6 +1028,8 @@ struct Record { | |
| #endif | ||
| // 37: optional vector<M365a> extM365a | ||
| std::vector< ::CsProtocol::M365a> extM365a; | ||
| // 38: optional vector<MetaData> extMetadata | ||
| std::vector< ::CsProtocol::MetaData> extMetadata; | ||
| // 41: optional vector<Data> ext | ||
| std::vector< ::CsProtocol::Data> ext; | ||
|
Comment on lines
1029
to
1034
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged - adding extMetadata to the public CsProtocol::Record is an intended, necessary part of carrying privTags in ext.metadata on the bond path. It is a new optional trailing field (existing members are not reordered), but it is part of the same wire/schema change this PR explicitly marks as best-effort and not-for-merge until the canonical ext.metadata contract - and its ABI/compat implications - are confirmed. Flagging it alongside the ordinal caveat rather than resolving it here. |
||
| #ifdef HAVE_CS4_FULL | ||
|
|
@@ -1065,6 +1083,7 @@ struct Record { | |
| && (extCs == other.extCs) | ||
| #endif | ||
| && (extM365a == other.extM365a) | ||
| && (extMetadata == other.extMetadata) | ||
| && (ext == other.ext) | ||
| #ifdef HAVE_CS4_FULL | ||
| && (extMscv == other.extMscv) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged - the ordinal (38) and layout are intentionally best-effort and clearly marked (TODO(privacy-parity) + the PR's reviewer caveat). This is a deliberate choice for this PR: it is not meant to merge until the canonical ext.metadata wire contract is confirmed. Gating behind a feature flag is a reasonable alternative if preferred - happy to do that instead of relying on the caveat.