Start logging host-version info in the Chromoting client.

Start logging host-version info in the Chromoting client.

BUG=479831
Committed: https://crrev.com/8e4f6ca3435b8aa13fa3c981a96cca08e3d51515
Cr-Commit-Position: refs/heads/master@{#326910}

[anandc, 2015-04-24 19:54:26]

anandc@chromium.org changed reviewers:
+ jamiewalch@chromium.org, kelvinp@chromium.org

[anandc, 2015-04-24 19:54:27]

Hi Jamie & Kelvin, PTAL. Thank you.

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting_activity.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting_activity.js:50:
session.getLogger().setHostVersion(host.hostVersion);
This only works for me2me hosts. IT2Me hosts currently only get the host-name.
There is no host-version info available in the response that we parse:
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/server_log_entry.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/server_log_entry.js:373:
remoting.ServerLogEntry.prototype.addHostFields = function() {
This is actually information about the local host, because we are parsing the
user-agent to obtain this. See extractHostDataFrom_() below.
Is there a reason we are logging that, instead of the remote-host info?

[Jamie, 2015-04-24 20:34:02]

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting_activity.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting_activity.js:50:
session.getLogger().setHostVersion(host.hostVersion);
On 2015/04/24 19:54:27, anandc wrote:
> This only works for me2me hosts. IT2Me hosts currently only get the host-name.
> There is no host-version info available in the response that we parse:
>
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...

As long as we clear the host version for IT2Me (rather than, say, leaving the
version of the most recent IT2Me host), then that's fine for this CL. Can you
file a bug for adding host version information or IT2Me?

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/log_to_server.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/log_to_server.js:184: // For example, the signal-strategy
log-entries won't have host version info.
Is this true even for multiple connections from the same instance of the app? I
don't see the host version being explicitly cleared anywhere.

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/server_log_entry.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/server_log_entry.js:373:
remoting.ServerLogEntry.prototype.addHostFields = function() {
On 2015/04/24 19:54:27, anandc wrote:
> This is actually information about the local host, because we are parsing the
> user-agent to obtain this. See extractHostDataFrom_() below.
> Is there a reason we are logging that, instead of the remote-host info? 

The function is poorly-named. It should be called something like
addClientOsFields. Feel free to change it if you like.

[anandc, 2015-04-24 21:36:35]

Thanks Jamie. PTAL.

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_remoting_activity.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_remoting_activity.js:50:
session.getLogger().setHostVersion(host.hostVersion);
On 2015/04/24 20:34:02, Jamie wrote:
> On 2015/04/24 19:54:27, anandc wrote:
> > This only works for me2me hosts. IT2Me hosts currently only get the
host-name.
> > There is no host-version info available in the response that we parse:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...
> 
> As long as we clear the host version for IT2Me (rather than, say, leaving the
> version of the most recent IT2Me host), then that's fine for this CL. Can you
> file a bug for adding host version information or IT2Me?

https://code.google.com/p/chromium/issues/detail?id=481189

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/log_to_server.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/log_to_server.js:184: // For example, the signal-strategy
log-entries won't have host version info.
On 2015/04/24 20:34:02, Jamie wrote:
> Is this true even for multiple connections from the same instance of the app?
I
> don't see the host version being explicitly cleared anywhere.

Yes. It's because a new logToServer object is created for each new session:
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...
and the constructor for logToServer sets it to blank.
Also confirmed by testing multiple connections, to the same machine and to
different machines, IT2Me and Me2Me, simultaneous and sequential connections,
within the same instance of the app.

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/server_log_entry.js (right):

https://codereview.chromium.org/1103913002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/server_log_entry.js:373:
remoting.ServerLogEntry.prototype.addHostFields = function() {
On 2015/04/24 20:34:02, Jamie wrote:
> On 2015/04/24 19:54:27, anandc wrote:
> > This is actually information about the local host, because we are parsing
the
> > user-agent to obtain this. See extractHostDataFrom_() below.
> > Is there a reason we are logging that, instead of the remote-host info? 
> 
> The function is poorly-named. It should be called something like
> addClientOsFields. Feel free to change it if you like.

Done.

[Jamie, 2015-04-24 21:43:27]

lgtm

[anandc, 2015-04-24 21:44:48]

The CQ bit was checked by anandc@chromium.org

[commit-bot: I haz the power, 2015-04-24 21:46:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1103913002/40001

[commit-bot: I haz the power, 2015-04-24 22:32:05]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-24 22:33:23]

Patchset 3 (id:??) landed as
https://crrev.com/8e4f6ca3435b8aa13fa3c981a96cca08e3d51515
Cr-Commit-Position: refs/heads/master@{#326910}