-
Notifications
You must be signed in to change notification settings - Fork 322
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
Add the Supplemental Data Identifier field to the Adobe Target extension #508 #509
Conversation
Looks good to me. I approve. |
@@ -58,6 +58,12 @@ | |||
"items": { | |||
"$ref": "https://ns.adobe.com/experience/target/activity" | |||
} | |||
}, | |||
"https://ns.adobe.com/experience/target/supplementalDataID": { |
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.
@vazdauta & @cdegroot-adobe ,
This isn't a Target specific field. It would also be populated by Analytics and in some cases by AdCloud. It would be more appropriate to be under - https://github.com/adobe/xdm/blob/master/extensions/adobe/experience/experienceevent.schema.json
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.
@cluby-adobe @vazdauta - Can you explain what this ID actually does please? I don't have enough context on what to do here. I want to get this closed in the next day or two please.
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.
Next hour or two if we want this to make 0.9.6 :)
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.
My research shows that this is mostly documented as a Target related feature. It is not documented in the core Analytics implementation documentation.
I am leaning to keeping this under Target. @cluby-adobe is this a blocking issue for you?
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.
It's used as a key to merge multiple ExperienceEvents together as one downstream. It's populated by Analytics, Target, and AdCloud is in the works. Currently the only downstream merger is Analytics' current data collection pipeline but in the Platform world we'll have multiple.
It for sure doesn't belong in the Target space and since it's populated by multiple solutions I don't think it belongs under any solution. I don't think I would put it in the core at this point because it's specific to Adobe Experience Cloud. So I think putting it under the adobe/experience namespace - https://github.com/adobe/xdm/blob/master/extensions/adobe/experience/experienceevent.schema.json - is the right place to put it.
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.
I like that idea, my only concern is that we would really like to get rid of that "union" schema at some point. I guess we could introduce a common "base event" for all the solutions when we do that refactor.
For now I am leaning towards just putting it onto the core experience event, but we would need to have a better name and more general description. I believe what we are saying is that sometimes a single action or activity results in multiple events, and they need to be annotated with a common value to link them all back together. Can we call this "actionID" or maybe a "eventLinkingID" or similar?
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.
@kstreeter
+1 on eventLinkingID for the final standard solution, but that is in the future.
I think we need to have a larger discussion in source event identity, Launch and a number of other things before we can make a decision on the standard. I bet we get it wrong if we do anything in the short time we have now. And we will still need the sdid extension anyway.
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.
@cdegroot-adobe ,why would we need SDID as well if that same value was carried in another field?
It doesn't seem like we are going to get consensus on how to handle this in time for the 0.9.7 milestone. I filed issue #528 to track for the future. |
#508