Include application ID in log entries.

remoting/webapp/crd/js/log_to_server.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * Module for sending log entries to the server.
  */
 
 'use strict';
 
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
 /**
  * @param {remoting.SignalStrategy} signalStrategy Signal strategy.
  * @constructor
  */
 remoting.LogToServer = function(signalStrategy) {
   /** @private */
   this.statsAccumulator_ = new remoting.StatsAccumulator();
   /** @private */
   this.sessionId_ = '';
   /** @private */
+  this.applicationId_ = '';

[anandc, 2015/04/23 00:38:32]

There are a few different ways we could track the app-ID information. It could
either be part of the logToServer object for a session, which seems the right
place for it. It could also be part of the dictionary in serverLogEntry, similar
to the way the browser-version and webapp-version are.
I've placed it in logToServer, because if it is in serverLogEntry, we'd have to
obtain its value every time a new entry is created. We do this already for the
the browser-version
(https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/cr...),
and that seems inefficient. 

[Jamie, 2015/04/23 00:54:38]

With my suggested change it's just a property access, so there's no efficiency
gain from precomputing it.

+  /** @private */
   this.sessionIdGenerationTime_ = 0;
   /** @private */
   this.sessionStartTime_ = new Date().getTime();
   /** @private */
   this.signalStrategy_ = signalStrategy;
   /** @private {string} */
   this.connectionType_ = '';
 
   this.setSessionId_();
+  this.setApplicationId_();
   signalStrategy.sendConnectionSetupResults(this);
 };
 
 // Constants used for generating a session ID.
 /** @private */
 remoting.LogToServer.SESSION_ID_ALPHABET_ =
     'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890';
 /** @private */
 remoting.LogToServer.SESSION_ID_LEN_ = 20;
 
@@ skipping 41 matching lines @@
 
 /**
  * @param {remoting.SignalStrategy.Type} strategyType
  * @param {remoting.FallbackSignalStrategy.Progress} progress
  */
 remoting.LogToServer.prototype.logSignalStrategyProgress =
     function(strategyType, progress) {
   this.maybeExpireSessionId_();
   var entry = remoting.ServerLogEntry.makeSignalStrategyProgress(
       this.sessionId_, strategyType, progress);
   this.log_(entry);

[anandc, 2015/04/23 00:38:32]

Is there a reason we're not setting the webapp and browser version for this log
entry, like we do in logClientSessionStateChange and logAccumulatedStatistics_?

[Jamie, 2015/04/23 00:54:38]

No; I just didn't think to include that information. I had a very particular
use-case in mind for this log information, and those fields are not interesting
for that case, but they should probably have been included.

TBH, this class is starting to show its age, and could probably stand to be
refactored so that these common fields are added automatically.

 };
 
 /**
  * Whether a session state is one of the states that occurs at the start of
  * a session.
  *
  * @private
  * @param {remoting.ClientSession.State} state
  * @return {boolean}
  */
@@ skipping 61 matching lines @@
  * Sends a log entry to the server.
  *
  * @private
  * @param {remoting.ServerLogEntry} entry
  */
 remoting.LogToServer.prototype.log_ = function(entry) {
   // Log the time taken to get to this point from the time this session started.
   var sessionDurationInSeconds =
       (new Date().getTime() - this.sessionStartTime_) / 1000.0;
   entry.addSessionDuration(sessionDurationInSeconds);
+  entry.addApplicationId(this.applicationId_);
 
   // Send the stanza to the debug log.
   console.log('Enqueueing log entry:');
   entry.toDebugLog(1);
 
   var stanza = '<cli:iq to="' + remoting.settings.DIRECTORY_BOT_JID + '" ' +
                        'type="set" xmlns:cli="jabber:client">' +
                  '<gr:log xmlns:gr="google:remoting">' +
                    entry.toStanza() +
                  '</gr:log>' +
                '</cli:iq>';
   this.signalStrategy_.sendMessage(stanza);
 };
 
 /**
  * Sets the session ID to a random string.
  *
  * @private
  */
 remoting.LogToServer.prototype.setSessionId_ = function() {
   this.sessionId_ = remoting.LogToServer.generateSessionId_();
   this.sessionIdGenerationTime_ = new Date().getTime();
 };
 
 /**
+ * Sets the application ID for this session.
+ *
+ * @private
+ */
+remoting.LogToServer.prototype.setApplicationId_ = function() {
+  this.applicationId_ = chrome.i18n.getMessage('@@extension_id');

[Jamie, 2015/04/23 00:54:38]

chrome.runtime.id would be a better way of getting this.

[anandc, 2015/04/23 01:26:59]

Done.

[rmsousa, 2015/04/23 01:55:51]

Drive-by:

For app remoting, there are cases (e.g. dev/test/staging app compiled locally),
where the chrome extensionId will just be the test app's default extensionId,
and the appId will be overriden via
remoting.settings.getAppRemotingApplicationId() .

I'm not sure which one we should use in that case - always using the extension
ID will ensure that we don't pollute the production logs with test entries.
Using the override application ID will ensure that we track each app separately,
regardless of environment.

+};
+
+/**
  * Clears the session ID.
  *
  * @private
  */
 remoting.LogToServer.prototype.clearSessionId_ = function() {
   this.sessionId_ = '';
   this.sessionIdGenerationTime_ = 0;
 };
 
 /**
  * Sets a new session ID, if the current session ID has reached its maximum age.
  *
  * This method also logs the old and new session IDs to the server, in separate
  * log entries.
  *
  * @private
  */
 remoting.LogToServer.prototype.maybeExpireSessionId_ = function() {
   if ((this.sessionId_ != '') &&
       (new Date().getTime() - this.sessionIdGenerationTime_ >=
       remoting.LogToServer.MAX_SESSION_ID_AGE)) {
     // Log the old session ID.
     var entry = remoting.ServerLogEntry.makeSessionIdOld(this.sessionId_);

[anandc, 2015/04/23 00:38:32]

Same question as above: why don't we set the webapp and browser versions for
this log-entry?

     this.log_(entry);
     // Generate a new session ID.
     this.setSessionId_();
     // Log the new session ID.
     entry = remoting.ServerLogEntry.makeSessionIdNew(this.sessionId_);
     this.log_(entry);
   }
 };
 
 /**
  * Generates a string that can be used as a session ID.
  *
  * @private
  * @return {string} a session ID
  */
 remoting.LogToServer.generateSessionId_ = function() {
   var idArray = [];
   for (var i = 0; i < remoting.LogToServer.SESSION_ID_LEN_; i++) {
     var index =
         Math.random() * remoting.LogToServer.SESSION_ID_ALPHABET_.length;
     idArray.push(
         remoting.LogToServer.SESSION_ID_ALPHABET_.slice(index, index + 1));
   }
   return idArray.join('');
 };