cameraservice: Add logId to correlate CameraActionEvents
Cameraservice currently triggers logging of CameraActionEvents (through
CameraServiceProxy) which are logged as independent event. For data
analysis purposes, it would be helpful to have one set of
OPEN-n*SESSION-CLOSE events correlate with each other.
To help with that, an mLogId field was added in CameraSessionStats which
will hold a string identifier. This identifier will be randomly
generated for OPEN events and reused for corresponding
ACTIVE, IDLE, and CLOSE events.
Bug: 271297241
Test: Manually tested that statsd gets the expected logId for camera
sessions.
Change-Id: Ia26f2cd5d75bdeb15737e85d7df1a46b75993712
diff --git a/camera/CameraSessionStats.cpp b/camera/CameraSessionStats.cpp
index 0706ac1..26c612a 100644
--- a/camera/CameraSessionStats.cpp
+++ b/camera/CameraSessionStats.cpp
@@ -271,6 +271,7 @@
mApiLevel(0),
mIsNdk(false),
mLatencyMs(-1),
+ mLogId(0),
mMaxPreviewFps(0),
mSessionType(0),
mInternalReconfigure(0),
@@ -281,7 +282,7 @@
CameraSessionStats::CameraSessionStats(const String16& cameraId,
int facing, int newCameraState, const String16& clientName,
- int apiLevel, bool isNdk, int32_t latencyMs) :
+ int apiLevel, bool isNdk, int32_t latencyMs, int64_t logId) :
mCameraId(cameraId),
mFacing(facing),
mNewCameraState(newCameraState),
@@ -289,6 +290,7 @@
mApiLevel(apiLevel),
mIsNdk(isNdk),
mLatencyMs(latencyMs),
+ mLogId(logId),
mMaxPreviewFps(0),
mSessionType(0),
mInternalReconfigure(0),
@@ -347,6 +349,12 @@
return err;
}
+ int64_t logId;
+ if ((err = parcel->readInt64(&logId)) != OK) {
+ ALOGE("%s: Failed to read log ID from parcel", __FUNCTION__);
+ return err;
+ }
+
float maxPreviewFps;
if ((err = parcel->readFloat(&maxPreviewFps)) != OK) {
ALOGE("%s: Failed to read maxPreviewFps from parcel", __FUNCTION__);
@@ -408,6 +416,7 @@
mApiLevel = apiLevel;
mIsNdk = isNdk;
mLatencyMs = latencyMs;
+ mLogId = logId;
mMaxPreviewFps = maxPreviewFps;
mSessionType = sessionType;
mInternalReconfigure = internalReconfigure;
@@ -464,6 +473,11 @@
return err;
}
+ if ((err = parcel->writeInt64(mLogId)) != OK) {
+ ALOGE("%s: Failed to write log ID!", __FUNCTION__);
+ return err;
+ }
+
if ((err = parcel->writeFloat(mMaxPreviewFps)) != OK) {
ALOGE("%s: Failed to write maxPreviewFps!", __FUNCTION__);
return err;
@@ -508,6 +522,7 @@
ALOGE("%s: Failed to write video stabilization mode!", __FUNCTION__);
return err;
}
+
return OK;
}
diff --git a/camera/include/camera/CameraSessionStats.h b/camera/include/camera/CameraSessionStats.h
index 90ee924..091a7ff 100644
--- a/camera/include/camera/CameraSessionStats.h
+++ b/camera/include/camera/CameraSessionStats.h
@@ -128,6 +128,22 @@
bool mIsNdk;
// latency in ms for camera open, close, or session creation.
int mLatencyMs;
+
+ /*
+ * A randomly generated identifier to map the open/active/idle/close stats to each other after
+ * being logged. Every 'open' event will have a newly generated id which will be logged with
+ * active/idle/closed that correspond to the particular 'open' event.
+ *
+ * This ID is not meant to be globally unique forever. Probabilistically, this ID can be
+ * safely considered unique across all logs from one android build for 48 to 72 hours from
+ * its generation. Chances of identifier collisions are significant past a week or two.
+ *
+ * NOTE: There are no guarantees that the identifiers will be unique. The probability of
+ * collision within a short timeframe is low, but any system consuming these identifiers at
+ * scale should handle identifier collisions, potentially even from the same device.
+ */
+ int64_t mLogId;
+
float mMaxPreviewFps;
// Session info and statistics
@@ -146,7 +162,8 @@
// Constructors
CameraSessionStats();
CameraSessionStats(const String16& cameraId, int facing, int newCameraState,
- const String16& clientName, int apiLevel, bool isNdk, int32_t latencyMs);
+ const String16& clientName, int apiLevel, bool isNdk, int32_t latencyMs,
+ int64_t logId);
virtual status_t readFromParcel(const android::Parcel* parcel) override;
virtual status_t writeToParcel(android::Parcel* parcel) const override;
diff --git a/camera/tests/fuzzer/camera_SessionStats_fuzzer.cpp b/camera/tests/fuzzer/camera_SessionStats_fuzzer.cpp
index 5866aaf..2f2ad77 100644
--- a/camera/tests/fuzzer/camera_SessionStats_fuzzer.cpp
+++ b/camera/tests/fuzzer/camera_SessionStats_fuzzer.cpp
@@ -131,8 +131,13 @@
parcelCamSessionStats.writeInt32(latencyMs);
}
+ int64_t logId = fdp.ConsumeIntegral<int64_t>();
+ if (fdp.ConsumeBool()) {
+ parcelCamSessionStats.writeInt64(logId);
+ }
+
cameraSessionStats = new CameraSessionStats(cameraId, facing, newCameraState, clientName,
- apiLevel, isNdk, latencyMs);
+ apiLevel, isNdk, latencyMs, logId);
}
if (fdp.ConsumeBool()) {
diff --git a/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.cpp b/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.cpp
index 7aaf6b2..1b80de2 100644
--- a/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.cpp
+++ b/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.cpp
@@ -248,9 +248,12 @@
apiLevel = CameraSessionStats::CAMERA_API_LEVEL_2;
}
- sessionStats = std::make_shared<CameraSessionStatsWrapper>(String16(id), facing,
- CameraSessionStats::CAMERA_STATE_OPEN, clientPackageName,
- apiLevel, isNdk, latencyMs);
+ // Generate a new log ID for open events
+ int64_t logId = generateLogId(mRandomDevice);
+
+ sessionStats = std::make_shared<CameraSessionStatsWrapper>(
+ String16(id), facing, CameraSessionStats::CAMERA_STATE_OPEN, clientPackageName,
+ apiLevel, isNdk, latencyMs, logId);
mSessionStatsMap.emplace(id, sessionStats);
ALOGV("%s: Adding id %s", __FUNCTION__, id.c_str());
}
@@ -300,4 +303,16 @@
return ret;
}
-}; // namespace android
+int64_t CameraServiceProxyWrapper::generateLogId(std::random_device& randomDevice) {
+ int64_t ret = 0;
+ do {
+ // std::random_device generates 32 bits per call, so we call it twice
+ ret = randomDevice();
+ ret = ret << 32;
+ ret = ret | randomDevice();
+ } while (ret == 0); // 0 is not a valid identifier
+
+ return ret;
+}
+
+} // namespace android
diff --git a/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.h b/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.h
index f90a841..2868e26 100644
--- a/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.h
+++ b/services/camera/libcameraservice/utils/CameraServiceProxyWrapper.h
@@ -24,6 +24,7 @@
#include <utils/String16.h>
#include <utils/StrongPointer.h>
#include <utils/Timers.h>
+#include <random>
#include <camera/CameraSessionStats.h>
@@ -37,7 +38,7 @@
sp<hardware::ICameraServiceProxy> mCameraServiceProxy;
class CameraSessionStatsWrapper {
- private:
+ private:
hardware::CameraSessionStats mSessionStats;
Mutex mLock; // lock for per camera session stats
@@ -47,11 +48,12 @@
*/
void updateProxyDeviceState(sp<hardware::ICameraServiceProxy>& proxyBinder);
- public:
+ public:
CameraSessionStatsWrapper(const String16& cameraId, int facing, int newCameraState,
- const String16& clientName, int apiLevel, bool isNdk, int32_t latencyMs) :
- mSessionStats(cameraId, facing, newCameraState, clientName, apiLevel, isNdk, latencyMs)
- { }
+ const String16& clientName, int apiLevel, bool isNdk,
+ int32_t latencyMs, int64_t logId)
+ : mSessionStats(cameraId, facing, newCameraState, clientName, apiLevel, isNdk,
+ latencyMs, logId) {}
void onOpen(sp<hardware::ICameraServiceProxy>& proxyBinder);
void onClose(sp<hardware::ICameraServiceProxy>& proxyBinder, int32_t latencyMs,
@@ -69,8 +71,15 @@
// Map from camera id to the camera's session statistics
std::map<String8, std::shared_ptr<CameraSessionStatsWrapper>> mSessionStatsMap;
+ std::random_device mRandomDevice; // pulls 32-bit random numbers from /dev/urandom
+
sp<hardware::ICameraServiceProxy> getCameraServiceProxy();
+ // Returns a randomly generated ID that is suitable for logging the event. A new identifier
+ // should only be generated for an open event. All other events for the cameraId should use the
+ // ID generated for the open event associated with them.
+ static int64_t generateLogId(std::random_device& randomDevice);
+
public:
CameraServiceProxyWrapper(sp<hardware::ICameraServiceProxy> serviceProxy = nullptr) :
mCameraServiceProxy(serviceProxy)