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 a "story" experiment ID to the performance service. #24039
Add a "story" experiment ID to the performance service. #24039
Conversation
src/service/performance-impl.js
Outdated
* @private | ||
*/ | ||
maybeAddStoryExperimentId_() { | ||
return isStoryDocument(getAmpdoc(this.win.document)).then(isStory => { |
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.
Looks like isStoryDocument
checks the doc body. You mentioned waiting for that might introduce latency. Is this no longer a concern?
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've been able to work around this concern thanks to the architecture of the performance service. It has a queuing system, where it stores all the metrics it receives until it's ready to communicate to the viewer.
This implementation makes it wait for the body to be parsed before emptying the queued metrics, which I think should be fine.
It ensures the story
experiment is added to the list before they're passed to the viewer with the first call to sendCSI
.
src/service/performance-impl.js
Outdated
* @private | ||
*/ | ||
maybeAddStoryExperimentId_() { | ||
return isStoryDocument(getAmpdoc(this.win.document)).then(isStory => { |
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.
Avoid using window.document
in general since it's incompatible with PWA case. You can grab the AmpDocService
directly and get the single doc there.
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.
Done
04c402e
to
eb6c760
Compare
eb6c760
to
b013583
Compare
…4039) * Add a story experiment ID to the performance service. * Use ampdocService instead of getAmpdoc.
Add a story experiment ID to the performance service so we can slice the performance metrics for AMP Story.
The only difference from the previous implementation is that the call to "messaging ready signal" is now queued so we don't have to delay it while we wait to know if the document is a story, but that is a no-op.