I2I: amp-link-rewriter #20653
Comments
parsingeye
added
the
INTENT TO IMPLEMENT
|
Hello, so I was discussing with @lannka , and we noticed that we are getting a lot more interesting in affiliate linking within AMP. And in fact, we had this proposal in the past, but got de-prioritized: #9276 Could you look through that, and see if it would work for your use case? We were thinking it may be better to make a more general solution on our end, that platforms can integrate with 😄 Thank you! |
|
We have looked at existing proposals and implementations, but they just didn't fit our use case. In the case of amp-link-rewriter, sending CORS request won't work for us as we don't have the endpoint or any infrastructure for it. We cannot change anything on the server side in the foreseeable future due to resource limits. Also, one of the essential things is the ability to include/exclude certain portions of the page. We also allow our publishers to use custom attributes, they define, placed on anchors to exclude them from rewriting (e.g., rel="pass"). It is crucial for publishers to be able to control this easily through extension. We could make our extension more general and call it amp-local-link-rewriter or amp-static-link-rewriter if others have an interest in using it similarly. |
|
@parsingeye Ah, makes sense. Thank you for the consideration! Was mostly asking for @lannka 's reference. Thank you! 😄 |
|
How should we proceed? |
|
cc @lannka for the next steps. But I think next step (if I understand correctly), is to present at a design review, that way the team in general can give feedback on the design and things. You can sign up for a design review, by commenting on one of the upcoming design review issues. And then present the design doc at that meeting 😄 |
|
@parsingeye thanks for your patience. I took a look at your design and I do feel there is good chance this can be made generic so your contribution can benefit many others. I left a couple of comments in the doc, we can try to get some questions answered there. |
|
/cc @jpettitt |
|
@parsingeye any progress on the spec design? |
|
JSON config spec proposal added to the end of the document https://docs.google.com/document/d/1slfb3znYHg4wdLwsvBqTFgB2K4esiltLAY1xq7g1ZpI An example (in a more readable format) <amp-digidip>
<script type="application/json">
{
"rewritePattern": "https://visit.digidip.net?pid=110&url={{href}}",
"ignoreHosts": ["youtube.com", "mobile.vodafone.de"],
"include": [
{
"attribute": "id",
"value": "comments"
},
{
"attribute": "class",
"value": "sidebar"
}
],
"exclude": {
"attribute": "rel",
"value": "skip",
"scanParents": false
}
}
</script>
</amp-digidip> |
|
@parsingeye I'm thinking of a more concise and flexible spec: {
"output": "https://visit.digidip.net?pid=110&url=${href}&cid=${customerId}",
"section": [
"#product-listing-1",
"#product-listing-2",
],
"attribute": {
"href": "((?!(https:\/\/youtube\.com)|(https:\/\/mobile\.vodafone\.de)).)*",
"id": "comments",
"class": "sidebar",
"rel": "(?!(skip))*",
},
"vars": {
"customerId": "12345"
}
}
Regex is less readable, but handles more diverse future requirements. It also makes the extension logic much simpler (smaller binary size). As you mentioned in the design review, the config is to be machine generated right? |
|
Although it is less readable, we like this approach and agree that will make the ext. logic somewhat simpler. If no one else has any objections to this we would like to start implementing proposed extension config. |
|
One additional thing we would like to add that is not related to configuration, but not mentioned before is how to make anchor attributes available for "output". If you have anchor like this: <a href="https://ma.rs" rev="pub5" data-val="merchant5">Launch!</a>extension would make data available in for of the {
"rev": "pub5",
"data-val": "merchant5"
}... and it can be used in configuration like this {
"output": "https://visit.digidip.net?url=${href}&referral=${anchorAttributes.rev}"
} |
|
@parsingeye glad to know it works for your use cases. I would like to have whitelist for other non 'data-' attributes for privacy reasons, so that the pub has control over what to share with vendors. The initial whitelist would have @PhilWinchester do you see the above proposed spec could meet needs from Smartlinks? |
|
@lannka are you suggesting that "non 'data-' attributes" whitelist should be configurable or just initially hard-coded to "id, href, rel, rev"? Regarding "data-" attributes, we noticed that amp-analytics is using data-vars-. Example: <span id="test1" class="box" data-vars-event-id="22"> Click here to generate an event </span>And in the output url the token would be of the format ${eventId} (follows camelcase). Would you suggest using the same pattern? |
I'd suggest we start with a fixed set.
Yep, I was thinking about the |
|
@lannka Those rules make sense for us. I'm assuming we'd still have access to the AMP element itself and could access certain elements (ie title, location, user agent, etc). The only other attributes that would be nice is The use case I'm thinking of is passing back certain information relevant to the new link that the publisher would then use to display on their site. |
@PhilWinchester we can make some of the AMP URL MACROs still available.
What do you use
I would avoid any DOM mutation at this point for security reasons. |
|
Based on our prev. discussion, this is the list of variables available for usage inside output pattern.
Example of this in action would be: Given the anchor tag: <a href="https:/ma.rs" rev="RE334" data-vars-merchant-id="432">Lift off!</a>... and config "vars": {
"customerId": "12345"
}output attribute can look like this "output": "https://vendor.com?url=${href}&user=${rev}&merchant=${data.merchantId}&customer=${data.customerId}"
... and the result of the rewrite for that perticular anchor with that configuration would be:
Let me know what you think so we can start finalizing our implementation. |
|
We have PR that closes this issue #21285 |
|
@parsingeye thanks for contributing! since the feature has been there for a while, want to follow up to see if it is launched at your side and how things are going. |
|
@lannka Yes, Thanks for all your help and support! |
|
@parsingeye good to hear that! |
|
@lannka Yes, that is the plan. That will make config much easier for publishers. |
Hi AMP community,
I'm Dragan Stefanov, CTO at digidip GmbH.
Digidip is the premium content monetization platform. It provides instant access to over 40,000 merchant affiliate programs without the hassle of network sign-ups, approvals or creating affiliate links.
Design document
https://docs.google.com/document/d/1slfb3znYHg4wdLwsvBqTFgB2K4esiltLAY1xq7g1ZpI
Motivation
Our main goal is to provide publishers with ways to increase their revenues without relying on ads and without damaging reader's experience.
Since AMP gained huge popularity among publishers, constant request is to provide digidip AMP extension.
With this extension they would easily accelerate their monetization efforts.
Requirements
Core functionality to be built as an AMP extension is pretty simple:
Potential functionality for future AMP development:
Apart from the above requirements, ease of implementation is critical for our publishers, so we propose a single tag with only one mandatory attribute (publisherID).
/cc @dvoytenko @aghassemi @lannka
The text was updated successfully, but these errors were encountered: