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

Add the Supplemental Data Identifier field to the Adobe Target extension #508 #509

Merged
merged 4 commits into from
Sep 28, 2018

Conversation

vazdauta
Copy link
Contributor

@kstreeter kstreeter added this to the 0.9.6 milestone Sep 18, 2018
@cdegroot-adobe
Copy link
Contributor

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": {
Copy link
Contributor

@cluby-adobe cluby-adobe Sep 26, 2018

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

Copy link
Contributor

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.

Copy link
Collaborator

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 :)

Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Collaborator

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?

Copy link
Contributor

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.

Copy link
Collaborator

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?

@kstreeter
Copy link
Collaborator

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.

@kstreeter kstreeter merged commit 8b57a6b into adobe:master Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants