No content error reporting#6306
No content error reporting#6306glevitzky wants to merge 7 commits intoampproject:masterfrom google:no-content-error-reporting
Conversation
|
Are the messages for debugging? I feel those details are not necessary. You can set break point, instead of dumping everything to the console. |
|
No, this is for our internal purposes to get a better understanding of why some ads aren't filling. |
|
@glevitzky so it's for debugging right? In that case, we should print readable and concise error messages to console, and leverage source maps ability to take deeper look. Adding too much into error messages doesn't just increase the binary size, but also gets the console error bloated. |
|
Sorry, I wasn't being clear. We need this to be reported back to us, not just logged to the console. |
|
@glevitzky Any idea how often this error would appear? Do we need to debounce it? |
| null, | ||
| 'slotId': this.baseInstance_ | ||
| .element.getAttribute('data-amp-slot-index'), | ||
| 'clientId': cid, |
There was a problem hiding this comment.
You cannot log these to our error logging.
This should probably be logged to a log owned by the ad provider.
There was a problem hiding this comment.
I was afraid of that. So let's consider alternatives. The issue is that we'd like to be able to capture enough info to reconstruct (if possible) the ad request that generated the no-fill. We'd like to be able to figure out whether there's a specific class of ads that is failing.
There's a creative-specific ID that is returned in the ad response, but we don't have access to it from outside the frame. We were hoping to catch the inputs that are used to construct the ad request so that we can reconstruct it on the backend, if necessary.
Is there an existing mechanism to log to ad network-provided destinations? amp-analytics, perhaps? Can we wire it to fire in this kind of case?
There was a problem hiding this comment.
Not sure I understand what the issue is. Is this a privacy concern related to clientId?
There was a problem hiding this comment.
Our error reporting infrastructure is not designed to store user data.
Maybe some ad-hoc mechanism would work here for you: Just new Image().src = $URL-YouBuild
There was a problem hiding this comment.
The issue is that this is in code that's supposed to be ad-network-agnostic. We'll need to find a way that preserves that while also sending us the signals we need.
There was a problem hiding this comment.
@cramforce Any update on our request for a more restricted set of data to be emitted for analysis?
There was a problem hiding this comment.
Reporting non-userdata would be fine. But our error reporting backend has very limited graphing capabilities. Wouldn't you want this reported into your system?
There was a problem hiding this comment.
What would be the easiest way to have this reported to our system? @tdrl Could this be a candidate for a CSI ping (potentially with further winnowing of the data)?
Edit: Nevermind on the CSI pings; I forgot that the lifecycle reporter is not available to us in this case.
There was a problem hiding this comment.
Something like it would be appropriate. Of course, only for your own ads.
| user().warn('AMP-AD', e); | ||
| getAdCid(this.baseInstance_).then(cid => { | ||
| const tagType = this.baseInstance_.element.getAttribute('type'); | ||
| const TAG = `AMP-AD-${tagType}`; |
There was a problem hiding this comment.
This should probably be moved to class-level and used uniformly in all messages. ( @keithwrightbos just did that for amp-a4a.js, but I don't think he made that change here. Might as well be consistent.)
There was a problem hiding this comment.
After searching through the repository, this seems to be fairly standard practice, if not convention. Wouldn't a class-level constant require that TAG always have a fixed value for all instances of amp-ad-xorigin-iframe-handler?
There was a problem hiding this comment.
Sorry, I misspoke. I meant instance-level, so that it could be reused everywhere within the instance of the class corresponding to a single slot. Regardless, it's a very minor thing, so don't stress it.
| null, | ||
| 'slotId': this.baseInstance_ | ||
| .element.getAttribute('data-amp-slot-index'), | ||
| 'clientId': cid, |
There was a problem hiding this comment.
I was afraid of that. So let's consider alternatives. The issue is that we'd like to be able to capture enough info to reconstruct (if possible) the ad request that generated the no-fill. We'd like to be able to figure out whether there's a specific class of ads that is failing.
There's a creative-specific ID that is returned in the ad response, but we don't have access to it from outside the frame. We were hoping to catch the inputs that are used to construct the ad request so that we can reconstruct it on the backend, if necessary.
Is there an existing mechanism to log to ad network-provided destinations? amp-analytics, perhaps? Can we wire it to fire in this kind of case?
|
@glevitzky are you still on this? |
|
@lannka Yes, I'd like to keep this open. There is still a demand from the Glade team for this kind of error reporting. |
|
@glevitzky this PR is too old. there might be large merge conflicts. I'll close it for now, and you can reopen when you're back on it. |
Adds more detail to reported error in no-content case.