feat(PII): Add dynamic PII derivation #5107
Conversation
This adds the ability to "dynamically" define PII to `metastructure`. It works like this: if `compute_pii` is a function `fn(&ProcessingState) -> Pii`, you can annotate a container or field with `#[metastructure(pii = "compute_pii")]` (in addition to the existing values of `"true"`, `"false"`, and `"maybe"`) which will cause the PII value of the field/container to depend on the current `ProcessingState`. See the new `test_dynamic_pii` test for a simple example of this feature in action. The intended use of this is to allow the PII status of `AttributeValue::value` to be fetched from `relay-conventions` based on the attribute's name.
| PiiExtended::Static(Pii::True) => Some(Cow::Borrowed(&PII_TRUE_FIELD_ATTRS)), | ||
| PiiExtended::Static(Pii::False) => None, | ||
| PiiExtended::Static(Pii::Maybe) => Some(Cow::Borrowed(&PII_MAYBE_FIELD_ATTRS)), | ||
| PiiExtended::Dynamic(_) => None, |
There was a problem hiding this comment.
Is this safe? I would have expected FieldAttrs::pii to also be of type PiiExtended and to be derived for dynamic fields as well.
There was a problem hiding this comment.
I would have expected FieldAttrs::pii to also be of type PiiExtended
Sorry, I don't follow. It is of that type. This function controls how field attributes are passed from a field to its children (currently, Pii::True and Pii::Maybe are inherited, but Pii::False isn't; I'm not actually sure how much sense that makes). I decided to make it so the function to dynamically compute the PII value should not be inherited by child fields because the child field would get passed a different state, so the same function wouldn't necessarily make sense.
But since this is a function on ProcessingState, we can actually realize the dynamic PII value and pass that to the child fields, which I think is a lot more natural.
There was a problem hiding this comment.
Sorry, I don't follow. It is of that type.
My bad.
I decided to make it so the function to dynamically compute the PII value should not be inherited by child fields because the child field would get passed a different state, so the same function wouldn't necessarily make sense.
Makes sense!
jjbayer
left a comment
jjbayer
left a comment
There was a problem hiding this comment.
Nice! It will be interesting to see if this works on deeply nested real-world types, but IMO we can just merge this and see about that in the next PR.
This implements fetching PII information from `relay-conventions` for attributes, using the dynamic machinery introduced in #5107. Unfortunately, in the `AttributeMode::ValueOnly` case (i.e. default/legacy rules), we have to do some state manipulation and can't rely on the derived machinery. ref: #5096. ref: RELAY-152.
This implements fetching PII information from `relay-conventions` for attributes, using the dynamic machinery introduced in #5107. Unfortunately, in the `AttributeMode::ValueOnly` case (i.e. default/legacy rules), we have to do some state manipulation and can't rely on the derived machinery. ref: #5096. ref: RELAY-152.
This implements fetching PII information from `relay-conventions` for attributes, using the dynamic machinery introduced in #5107. Unfortunately, in the `AttributeMode::ValueOnly` case (i.e. default/legacy rules), we have to do some state manipulation and can't rely on the derived machinery. Also, this PR reverts the dependency between `relay-conventions` and `relay-event-schema` (now `event-schema` depends on `conventions`), which means that `conventions` now has its own `Pii` type since it can't use the one from `event-schema` anymore. ref: #5096. ref: RELAY-152.
This adds the ability to "dynamically" define PII to
metastructure. It works like this: ifcompute_piiis a functionfn(&ProcessingState) -> Pii, you can annotate a container or field with#[metastructure(pii = "compute_pii")](in addition to the existing values of"true","false", and"maybe") which will cause the PII value of the field/container to depend on the currentProcessingState. See the newtest_dynamic_piitest for a simple example of this feature in action.The intended use of this is to allow the PII status of
AttributeValue::valueto be fetched fromrelay-conventionsbased on the attribute's name.ref: #5096. ref: RELAY-152.