Pass nocurls as additional info to Viewer for latency tracking.#7709
Pass nocurls as additional info to Viewer for latency tracking.#7709lannka merged 8 commits intoampproject:masterfrom
Conversation
src/service/performance-impl.js
Outdated
| return this.enabledExperiments_; | ||
| } | ||
| const experiments = []; | ||
| if (this.win.location.hostname == 'cdn.ampproject.org') { |
There was a problem hiding this comment.
Are you sending a similar CL for "was-prerendered"?
There was a problem hiding this comment.
Why hard coding cdn.ampproject.org
There was a problem hiding this comment.
No reason, good catch.
Do you have some kind of filter for catching this? :)
We should add a presubmit check for this.
There was a problem hiding this comment.
@dknecht this is for our own tracking, so we can slice the traffic from cdn.ampproject.org.
There was a problem hiding this comment.
Useful for others.
config.urls.cdn should help
There was a problem hiding this comment.
@cramforce yeah gmail filter. Not very accurate.
@erwinmombay has mentioned a presubmit check also.
|
@cramforce comment addressed. PTAL |
cramforce
left a comment
There was a problem hiding this comment.
LG, but please add comment for what is going on.
| return this.enabledExperiments_; | ||
| } | ||
| const experiments = []; | ||
| if (this.getHostname_() == urls.cdn.split('://')[1]) { |
src/service/performance-impl.js
Outdated
| const experiments = []; | ||
| // Check if it's the legacy CDN domain. | ||
| if (this.getHostname_() == urls.cdn.split('://')[1]) { | ||
| experiments.push('nocurls'); |
There was a problem hiding this comment.
Lets pick a name that makes sense like legacy-cdn-domain
| const experiments = []; | ||
| // Check if it's the legacy CDN domain. | ||
| if (this.getHostname_() == urls.cdn.split('://')[1]) { | ||
| experiments.push('legacy-cdn-domain'); |
There was a problem hiding this comment.
can we list these "experiments" down somewhere, hard to track down at some point
There was a problem hiding this comment.
i planned to have all experiments coded in this method. we shouldn't have too many simultaneously, since they quickly take URL length.
if we really get a handful experiments running, we can find other way to manage them. how does that sound?
| } | ||
| const experiments = []; | ||
| // Check if it's the legacy CDN domain. | ||
| if (this.getHostname_() == urls.cdn.split('://')[1]) { |
* Pass nocurls as additional info to Viewer for latency tracking. * Add test. * use urls.cdn * Add comment. * ... * ... * rename experiment name * fix test
…roject#7709) * Pass nocurls as additional info to Viewer for latency tracking. * Add test. * use urls.cdn * Add comment. * ... * ... * rename experiment name * fix test
No description provided.