From 25043410705f2a7a275e69ec513c8889465d4ba3 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Fri, 26 Jan 2024 19:17:38 -0800 Subject: [PATCH] bootlog: remove timestamp from filename (#31187) * bootlog: remove timestamp from filename * revert route * test * revert that * fix loggerd test --- common/params.cc | 1 + system/loggerd/bootlog.cc | 6 +++--- system/loggerd/logger.cc | 27 +++++++++++++++++++++++++++ system/loggerd/logger.h | 1 + system/loggerd/tests/test_loggerd.py | 13 +++++++++---- tools/lib/bootlog.py | 19 +++++++------------ tools/lib/helpers.py | 3 ++- 7 files changed, 50 insertions(+), 20 deletions(-) diff --git a/common/params.cc b/common/params.cc index 386813efdd..3ce2505243 100644 --- a/common/params.cc +++ b/common/params.cc @@ -95,6 +95,7 @@ std::unordered_map keys = { {"AthenadPid", PERSISTENT}, {"AthenadUploadQueue", PERSISTENT}, {"AthenadRecentlyViewedRoutes", PERSISTENT}, + {"BootCount", PERSISTENT}, {"CalibrationParams", PERSISTENT}, {"CameraDebugExpGain", CLEAR_ON_MANAGER_START}, {"CameraDebugExpTime", CLEAR_ON_MANAGER_START}, diff --git a/system/loggerd/bootlog.cc b/system/loggerd/bootlog.cc index 771594d20c..b8257b6d69 100644 --- a/system/loggerd/bootlog.cc +++ b/system/loggerd/bootlog.cc @@ -49,8 +49,8 @@ static kj::Array build_boot_log() { } int main(int argc, char** argv) { - const std::string timestr = logger_get_route_name(); - const std::string path = Path::log_root() + "/boot/" + timestr; + const std::string id = logger_get_identifier("BootCount"); + const std::string path = Path::log_root() + "/boot/" + id; LOGW("bootlog to %s", path.c_str()); // Open bootlog @@ -64,7 +64,7 @@ int main(int argc, char** argv) { file.write(build_boot_log().asBytes()); // Write out bootlog param to match routes with bootlog - Params().put("CurrentBootlog", timestr.c_str()); + Params().put("CurrentBootlog", id.c_str()); return 0; } diff --git a/system/loggerd/logger.cc b/system/loggerd/logger.cc index bc842f4a9a..bb829df6ed 100644 --- a/system/loggerd/logger.cc +++ b/system/loggerd/logger.cc @@ -3,6 +3,9 @@ #include #include #include +#include +#include +#include #include "common/params.h" #include "common/swaglog.h" @@ -94,6 +97,30 @@ std::string logger_get_route_name() { return route_name; } +std::string logger_get_identifier(std::string key) { + // a log identifier is a 32 bit counter, plus a 10 character unique ID. + // e.g. 000001a3--c20ba54385 + + Params params; + uint32_t cnt; + try { + cnt = std::stol(params.get(key)); + } catch (std::exception &e) { + cnt = 0; + } + params.put(key, std::to_string(cnt + 1)); + + std::stringstream ss; + std::random_device rd; + std::mt19937 mt(rd()); + std::uniform_int_distribution dist(0, 15); + for (int i = 0; i < 10; ++i) { + ss << std::hex << dist(mt); + } + + return util::string_format("%08x--%s", cnt, ss.str().c_str()); +} + static void log_sentinel(LoggerState *log, SentinelType type, int eixt_signal = 0) { MessageBuilder msg; auto sen = msg.initEvent().initSentinel(); diff --git a/system/loggerd/logger.h b/system/loggerd/logger.h index 76a12b9e87..dd3bee150c 100644 --- a/system/loggerd/logger.h +++ b/system/loggerd/logger.h @@ -53,3 +53,4 @@ protected: kj::Array logger_build_init_data(); std::string logger_get_route_name(); +std::string logger_get_identifier(std::string key); diff --git a/system/loggerd/tests/test_loggerd.py b/system/loggerd/tests/test_loggerd.py index 625fa22f6d..0cd8548809 100755 --- a/system/loggerd/tests/test_loggerd.py +++ b/system/loggerd/tests/test_loggerd.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 import numpy as np import os +import re import random import string import subprocess @@ -21,6 +22,7 @@ from openpilot.system.loggerd.xattr_cache import getxattr from openpilot.system.loggerd.deleter import PRESERVE_ATTR_NAME, PRESERVE_ATTR_VALUE from openpilot.selfdrive.manager.process_config import managed_processes from openpilot.system.version import get_version +from openpilot.tools.lib.helpers import RE from openpilot.tools.lib.logreader import LogReader from cereal.visionipc import VisionIpcServer, VisionStreamType from openpilot.common.transformations.camera import tici_f_frame_size, tici_d_frame_size, tici_e_frame_size @@ -109,7 +111,6 @@ class TestLoggerd: ("GitRemote", "gitRemote", "remote"), ] params = Params() - params.clear_all() for k, _, v in fake_params: params.put(k, v) params.put("AccessToken", "abc") @@ -129,15 +130,13 @@ class TestLoggerd: # check params logged_params = {entry.key: entry.value for entry in initData.params.entries} - expected_params = {k for k, _, __ in fake_params} | {'AccessToken'} + expected_params = {k for k, _, __ in fake_params} | {'AccessToken', 'BootCount'} assert set(logged_params.keys()) == expected_params, set(logged_params.keys()) ^ expected_params assert logged_params['AccessToken'] == b'', f"DONT_LOG param value was logged: {repr(logged_params['AccessToken'])}" for param_key, initData_key, v in fake_params: assert getattr(initData, initData_key) == v assert logged_params[param_key].decode() == v - params.put("AccessToken", "") - @flaky(max_runs=3) def test_rotation(self): os.environ["LOGGERD_TEST"] = "1" @@ -216,6 +215,12 @@ class TestLoggerd: bootlog_val = [e.value for e in boot.pstore.entries if e.key == fn][0] assert expected_val == bootlog_val + # next one should increment by one + bl1 = re.match(RE.LOG_ID_V2, bootlog_path.name) + bl2 = re.match(RE.LOG_ID_V2, self._gen_bootlog().name) + assert bl1.group('uid') != bl2.group('uid') + assert int(bl1.group('count')) == 0 and int(bl2.group('count')) == 1 + def test_qlog(self): qlog_services = [s for s in CEREAL_SERVICES if SERVICE_LIST[s].decimation is not None] no_qlog_services = [s for s in CEREAL_SERVICES if SERVICE_LIST[s].decimation is None] diff --git a/tools/lib/bootlog.py b/tools/lib/bootlog.py index 01756bb5e9..827ef1eefc 100644 --- a/tools/lib/bootlog.py +++ b/tools/lib/bootlog.py @@ -1,11 +1,10 @@ -import datetime import functools import re from typing import List, Optional from openpilot.tools.lib.auth_config import get_token from openpilot.tools.lib.api import CommaApi -from openpilot.tools.lib.helpers import RE, timestamp_to_datetime +from openpilot.tools.lib.helpers import RE @functools.total_ordering @@ -17,8 +16,8 @@ class Bootlog: if not r: raise Exception(f"Unable to parse: {url}") + self._id = r.group('log_id') self._dongle_id = r.group('dongle_id') - self._timestamp = r.group('timestamp') @property def url(self) -> str: @@ -29,25 +28,21 @@ class Bootlog: return self._dongle_id @property - def timestamp(self) -> str: - return self._timestamp - - @property - def datetime(self) -> datetime.datetime: - return timestamp_to_datetime(self._timestamp) + def id(self) -> str: + return self._id def __str__(self): - return f"{self._dongle_id}|{self._timestamp}" + return f"{self._dongle_id}/{self._id}" def __eq__(self, b) -> bool: if not isinstance(b, Bootlog): return False - return self.datetime == b.datetime + return self.id == b.id def __lt__(self, b) -> bool: if not isinstance(b, Bootlog): return False - return self.datetime < b.datetime + return self.id < b.id def get_bootlog_from_id(bootlog_id: str) -> Optional[Bootlog]: # TODO: implement an API endpoint for this diff --git a/tools/lib/helpers.py b/tools/lib/helpers.py index 029ecb7966..37bbaf0c4a 100644 --- a/tools/lib/helpers.py +++ b/tools/lib/helpers.py @@ -7,7 +7,8 @@ TIME_FMT = "%Y-%m-%d--%H-%M-%S" class RE: DONGLE_ID = r'(?P[a-z0-9]{16})' TIMESTAMP = r'(?P[0-9]{4}-[0-9]{2}-[0-9]{2}--[0-9]{2}-[0-9]{2}-[0-9]{2})' - LOG_ID = r"(?P{})".format(TIMESTAMP) + LOG_ID_V2 = r'(?P[a-z0-9]{8})--(?P[a-z0-9]{10})' + LOG_ID = r'(?P(?:{}|{}))'.format(TIMESTAMP, LOG_ID_V2) ROUTE_NAME = r'(?P{}[|_/]{})'.format(DONGLE_ID, LOG_ID) SEGMENT_NAME = r'{}(?:--|/)(?P[0-9]+)'.format(ROUTE_NAME)