Add activity recording capability to gcm internals page. User can refresh, start/stop recording, an…

google_apis/gcm/gcm_stats_recorder.h

+// Copyright 2014 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.
+
+#ifndef GOOGLE_APIS_GCM_GCM_STATS_RECORDER_H_
+#define GOOGLE_APIS_GCM_GCM_STATS_RECORDER_H_
+
+#include <deque>
+#include <map>

[jianli, 2014/03/18 23:53:23]

not used

[juyik, 2014/03/20 01:09:53]

Done.

+#include <string>
+#include <vector>
+
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/stl_util.h"

[jianli, 2014/03/18 23:53:23]

not used?

[juyik, 2014/03/20 01:09:53]

Done.

+#include "base/time/time.h"
+#include "google_apis/gcm/base/gcm_export.h"
+
+namespace gcm {
+
+// Records GCM internal stats and activities for debugging purpose. Recording
+// can be turned on/off by calling SetRecording(...) function. It is turned off
+// by default.
+// This class is not thread safe. It is meant to be owned by a gcm client
+// instance.
+class GCM_EXPORT GCMStatsRecorder {

[fgorski, 2014/03/18 21:28:37]

Tests for recorder?

[juyik, 2014/03/20 01:09:53]

Done.

+ public:
+  // Type of the activity. Each activity is one step in a larger operation (
+  // register, send, receive, etc).
+  // Any change made to this enum must propagate to GetTypeString() function.
+  enum ActivityType {
+    Initiated,       // First attempt of an operation.
+    RetryRequested,  // A retry of the operation is requested.
+    Retry,           // The retry of the operation is actually starting.
+    Success,         // The operation has completed successfully.
+    Intermediate,    // The activity is an intermediate step of an operation.
+    Failure,         // The operation has failed; no more retries.
+    Error,           // The operation encountered an error.
+  };
+
+  // Contains data that are common to all activity kinds below.
+  struct GCM_EXPORT Activity {
+    Activity();
+    virtual ~Activity();
+    std::string GetTypeString() const;
+
+    base::Time time;
+    ActivityType type;
+    std::string details;  // Any additional detail about this activity.
+  };
+
+  // Contains relevant data of a check-in step.
+  struct GCM_EXPORT CheckinActivity : Activity {
+    CheckinActivity();
+    virtual ~CheckinActivity() OVERRIDE;
+
+    uint64 android_id;
+  };
+
+  // Contains relevant data of a register step.
+  struct GCM_EXPORT RegisterActivity : Activity {
+    RegisterActivity();
+    virtual ~RegisterActivity() OVERRIDE;
+
+    std::string app_id;
+    std::string sender_id;
+  };
+
+  // Contains relevant data of a send-message step.
+  struct GCM_EXPORT SendMessageActivity : Activity {
+    SendMessageActivity();
+    virtual ~SendMessageActivity() OVERRIDE;
+
+    std::string app_id;
+    std::string receiver_id;
+    std::string message_id;
+    uint64 message_len;  // Sent message's length in bytes.
+  };
+
+  // Contains relevant data of a receive-message step.
+  struct GCM_EXPORT ReceiveMessageActivity : Activity {
+    ReceiveMessageActivity();
+    virtual ~ReceiveMessageActivity() OVERRIDE;
+
+    std::string sender_id;
+    std::string app_id;
+    int message_size;  // The number of key-value pairs in received message.
+  };
+
+  GCMStatsRecorder();
+  virtual ~GCMStatsRecorder();
+
+  // Indicates whether the recorder is currently recording activities or not.
+  bool IsRecording() const {

[jianli, 2014/03/18 23:53:23]

nit: this can be exposed as getter: bool is_recording() const

[juyik, 2014/03/20 01:09:53]

Done.

+    return is_recording_;
+  }
+
+  // Turns recording on/off.
+  void SetRecording(bool recording);
+
+  // Clear all recorded activities.
+  void Clear();
+
+  // Record a checkin activity.
+  void RecordCheckin(uint64 android_id, ActivityType type);
+
+  // Record a register activity.
+  void RecordRegister(const std::string& app_id,

[jianli, 2014/03/18 23:53:23]

RecordRegistration?

[juyik, 2014/03/20 01:09:53]

Will do in my future patch. Splitting this part of the code to later patches.

+                      ActivityType type,
+                      const std::vector<std::string>& sender_id);
+  void RecordRegisterWithStatus(const std::string& app_id,

[fgorski, 2014/03/18 21:28:37]

For this and most of the other methods. Consider passing the original status
enums into Recorder, and even decision about failure/success can be moved to the
recorder. The only think that you may want to pass in is the information whether
a retry was attempted.

[juyik, 2014/03/20 01:09:53]

I thought about it a little and I felt that the SoC (Separation of Concerns)
principle is a good design pattern here. Basically a class should be an expert
and owner of its one responsibility; for example the registration class should
own all registration logic, including registration error logic. And the recorder
class should just do the recording and should not know too much about checkin,
registration, send, receive, etc. Hence it should just take a few generic const
string descriptions. This makes it cleaner because domain specific logic will
stay with its owner as close as possible.

+                                ActivityType type,
+                                const std::vector<std::string>& sender_id,
+                                std::string status);

[jianli, 2014/03/18 23:53:23]

It would be better to combine both RecordRegister***. Also use "const
std::string&" for status.

[juyik, 2014/03/20 01:09:53]

Will do in my future patch. Splitting this part of the code to later patches.

+
+  // Record a send activity.
+  void RecordSend(const std::string& app_id,

[jianli, 2014/03/18 23:53:23]

RecordSending?

[juyik, 2014/03/20 01:09:53]

Done.

+                  ActivityType type,
+                  const std::string& receiver_id,
+                  const std::string& message_id,
+                  uint64 msg_len);
+  void RecordSendWithDetails(const std::string& app_id,
+                             ActivityType type,
+                             const std::string& receiver_id,
+                             const std::string& message_id,
+                             uint64 msg_len,
+                             const std::string& details);
+
+  // Record a receive activity.
+  void RecordReceiveDataMessage(const std::string& sender_id,
+                                const std::string& app_id,
+                                int message_size);
+  void RecordReceiveWithDetails(ActivityType type,
+                                const std::string& sender_id,
+                                const std::string& app_id,
+                                const std::string& details);
+
+  const std::deque<CheckinActivity>& CheckinActivities() const {

[jianli, 2014/03/18 23:53:23]

ditto for all these 4 methods

[juyik, 2014/03/20 01:09:53]

Done for sending. Will do the rest in my future patch. Splitting this part of
the code to later patches.

+    return checkin_activities_;
+  }
+  const std::deque<RegisterActivity>& RegisterActivities() const {
+    return register_activities_;
+  }
+  const std::deque<SendMessageActivity>& SendMessageActivities() const {
+    return send_activities_;
+  }
+  const std::deque<ReceiveMessageActivity>& ReceiveMessageActivities() const {
+    return receive_activities_;
+  }
+
+ private:
+  bool is_recording_;
+
+  std::deque<CheckinActivity> checkin_activities_;
+  std::deque<RegisterActivity> register_activities_;
+  std::deque<SendMessageActivity> send_activities_;
+  std::deque<ReceiveMessageActivity> receive_activities_;
+
+  DISALLOW_COPY_AND_ASSIGN(GCMStatsRecorder);
+};
+
+}  // namespace gcm
+
+#endif  // GOOGLE_APIS_GCM_GCM_STATS_RECORDER_H_