Skip to content

feat: add application launch event reporting#354

Merged
ComixHe merged 1 commit into
linuxdeepin:masterfrom
Ivy233:feat/dde-am-eventlog1
May 9, 2026
Merged

feat: add application launch event reporting#354
ComixHe merged 1 commit into
linuxdeepin:masterfrom
Ivy233:feat/dde-am-eventlog1

Conversation

@Ivy233
Copy link
Copy Markdown
Contributor

@Ivy233 Ivy233 commented May 8, 2026

Add event reporting for application launch lifecycle, including launch, launch failure, launch duration, and abnormal exit events via DDEAPI EventLogger.

添加应用启动事件上报功能,通过 DDEAPI EventLogger 上报启动、启动失败、
启动耗时及异常退出等事件。

  • Add EventReporter module for reporting app launch, launch failure, launch duration and abnormal exit events.

  • Pass launch type through dde-am CLI (--launch-type) and track it in InstanceService as a D-Bus property.

  • Monitor systemd unit result to detect abnormal exits (failed, canceled, timeout, signal, core-dump, exit-code) and report with journalctl logs.

  • Handle ll-cli applications by extracting actual appId for event reporting.

  • Add LaunchType property (0: other, 1: launcher, 2: taskbar, 3: desktop icon, 4: autostart) to Instance D-Bus interface.

  • Add dde-api-dev build dependency.

  • 新增 EventReporter 模块,用于上报应用启动、启动失败、启动耗时及异常退出事件。

  • 通过 dde-am CLI (--launch-type) 传递启动来源类型,并在 InstanceService 中 作为 D-Bus 属性记录。

  • 监听 systemd unit 的 Result 属性变化,检测异常退出(failed、canceled、 timeout、signal、core-dump、exit-code)并上报 journalctl 日志。

  • 对 ll-cli 应用解析出实际 appId 用于事件上报。

  • 在 Instance D-Bus 接口新增 LaunchType 属性(0: 其他,1: 启动器, 2: 任务栏,3: 桌面图标,4: 自启动)。

  • 添加 dde-api-dev 打包依赖。

PMS: TASK-388657

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Ivy233, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@Ivy233 Ivy233 requested a review from BLumia May 8, 2026 04:53
@Ivy233 Ivy233 force-pushed the feat/dde-am-eventlog1 branch from 89bf3d6 to b924948 Compare May 8, 2026 04:58
Copy link
Copy Markdown
Member

@BLumia BLumia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/dbus/applicationservice.cpp Outdated
Comment thread src/eventreporter.cpp Outdated
Comment thread src/eventreporter.cpp Outdated
Comment thread src/eventreporter.cpp Outdated
Comment thread src/eventreporter.cpp Outdated
Comment thread apps/dde-am/src/main.cpp Outdated
Comment thread api/dbus/org.desktopspec.ApplicationManager1.Instance.xml Outdated
Comment thread apps/dde-am/src/launcher.cpp Outdated
@BLumia BLumia requested a review from ComixHe May 8, 2026 05:23
@Ivy233 Ivy233 force-pushed the feat/dde-am-eventlog1 branch from b924948 to 185ec30 Compare May 8, 2026 08:04
@Ivy233 Ivy233 requested a review from BLumia May 8, 2026 08:13
Comment thread src/CMakeLists.txt Outdated
Comment thread api/dbus/org.desktopspec.ApplicationManager1.Application.xml
Add event reporting for application launch lifecycle, including launch,
launch failure, launch duration, and abnormal exit events via DDEAPI
EventLogger.

添加应用启动事件上报功能,通过 DDEAPI EventLogger 上报启动、启动失败、
启动耗时及异常退出等事件。

- Add EventReporter module for reporting app launch, launch failure,
  launch duration and abnormal exit events.
- Pass launch type through dde-am CLI (--launch-type) and track it
  in InstanceService as a D-Bus property.
- Monitor systemd unit result to detect abnormal exits (failed,
  canceled, timeout, signal, core-dump, exit-code) and report with
  journalctl logs.
- Handle ll-cli applications by extracting actual appId for event
  reporting.
- Add LaunchType property (0: other, 1: launcher, 2: taskbar,
  3: desktop icon, 4: autostart) to Instance D-Bus interface.
- Add dde-api-dev build dependency.

- 新增 EventReporter 模块,用于上报应用启动、启动失败、启动耗时及异常退出事件。
- 通过 dde-am CLI (--launch-type) 传递启动来源类型,并在 InstanceService 中
  作为 D-Bus 属性记录。
- 监听 systemd unit 的 Result 属性变化,检测异常退出(failed、canceled、
  timeout、signal、core-dump、exit-code)并上报 journalctl 日志。
- 对 ll-cli 应用解析出实际 appId 用于事件上报。
- 在 Instance D-Bus 接口新增 LaunchType 属性(0: 其他,1: 启动器,
  2: 任务栏,3: 桌面图标,4: 自启动)。
- 添加 dde-api-dev 打包依赖。

PMS: TASK-388657
@Ivy233 Ivy233 force-pushed the feat/dde-am-eventlog1 branch from 185ec30 to 880836d Compare May 8, 2026 08:50
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

Git Diff 代码审查报告

整体概述

这段代码主要实现了应用程序启动事件的记录和报告功能,包括:

  1. 添加了启动类型(launch_type)的追踪
  2. 实现了事件报告系统(EventReporter)
  3. 增加了应用程序异常退出时的日志记录
  4. 添加了对dde-api库的可选依赖支持

语法与逻辑审查

1. CMakeLists.txt 修改

find_package(DDEAPI QUIET)
if(DDEAPI_FOUND)
    set(HAVE_DDE_API_EVENTLOGGER ON)
    message(STATUS "Found DDEAPI: EventLogger available")
else()
    message(STATUS "DDEAPI not found, event logging will be disabled")
endif()

优点

  • 使用QUIET选项避免找不到包时产生错误
  • 提供了清晰的构建状态消息

建议

  • 考虑在debian/control中添加对dde-api-dev的版本要求,确保兼容性

2. DBus接口修改

<property name="LaunchType" type="s" access="read">

优点

  • 添加了清晰的文档说明
  • 使用下划线前缀标记内部选项

建议

  • 考虑为LaunchType添加枚举值限制,确保只接受预定义的启动类型

3. EventReporter 实现

void EventReporter::reportAppLaunch(const QString &appName, const QString &appVersion, qint64 timeMs, const QString &launchType, const QString &uniqueID)
{
#ifdef HAVE_DDE_API_EVENTLOGGER
    DDE_EventLogger::EventLogger::instance().writeEventLog({...});
#else
    Q_UNUSED(appName)
    Q_UNUSED(appVersion)
    Q_UNUSED(timeMs)
    Q_UNUSED(launchType)
    Q_UNUSED(uniqueID)
#endif
}

优点

  • 使用条件编译确保在没有dde-api时也能正常编译
  • 提供了完整的日志记录功能

问题

  • 当HAVE_DDE_API_EVENTLOGGER未定义时,函数完全没有任何输出,调试困难
  • 日志事件ID(如1000610001)是硬编码的,缺乏文档说明

建议

  • 在没有dde-api时,至少添加qCDebug输出
  • 为事件ID添加常量定义和注释说明

4. 版本查询实现

QString EventReporter::getAppVersion(const QString &appId)
{
    // ...
    QProcess proc;
    proc.start("dpkg-query", {"-W", "-f=${Version}", appId});
    proc.waitForFinished(3000);
    // ...
    if (version.isEmpty() && appId.startsWith("org.")) {
        proc.start("ll-cli", {"info", appId});
        proc.waitForFinished(5000);
        // ...
    }
    // ...
}

问题

  • 同步等待进程完成(3000ms和5000ms)可能阻塞主线程
  • 没有对进程启动失败的处理
  • 对ll-cli的假设可能不适用于所有环境

建议

  • 考虑使用异步方式查询版本
  • 添加对进程启动失败的处理
  • 添加对ll-cli可用性的检查

5. 异常退出处理

if (result == u"failed" || result == u"canceled" || result == u"timeout"
    || result == u"signal" || result == u"core-dump" || result == u"exit-code") {
    QStringList logArgs{QStringLiteral("--unit=%1").arg(unitName),
                        "-n", "20", "-o", "cat", "-o", "with-unit", "--no-pager"};
    QProcess logProc;
    logProc.start("journalctl", logArgs);
    logProc.waitForFinished(3000);
    QString logInfo = QString::fromUtf8(logProc.readAllStandardOutput());
    EventReporter::reportAppAbnormalExit(app->eventAppId(), (*instanceIt)->launchType(), unitName, logInfo, (*instanceIt)->launchUniqueId());
}

问题

  • 同步等待journalctl完成(3000ms)可能阻塞
  • 没有对日志输出大小的限制,可能导致内存问题
  • 日志信息可能包含敏感数据

建议

  • 限制日志输出大小
  • 考虑异步处理
  • 添加对敏感信息的过滤

代码质量审查

优点

  1. 代码结构清晰,职责分离良好
  2. 使用了现代C++特性(如QStringLiteral, u""_s)
  3. 添加了适当的日志输出
  4. 使用了条件编译处理可选依赖

改进建议

  1. 错误处理:多处缺少对QProcess启动失败的处理
  2. 文档:缺少对新增API的详细文档说明
  3. 测试:没有看到相应的单元测试代码
  4. 魔法数字:事件ID和超时时间等魔法数字应定义为常量

代码性能审查

  1. 同步操作

    • 多处使用waitForFinished()阻塞主线程
    • 版本查询可能在启动时被频繁调用
  2. 缓存

    • 版本查询使用了缓存,这是好的实践
    • 但缓存没有失效机制,可能导致版本信息过时
  3. 建议

    • 考虑使用异步API处理进程
    • 为版本缓存添加失效机制
    • 考虑在后台线程中处理日志收集

代码安全审查

  1. 命令注入风险

    proc.start("journalctl", logArgs);

    虽然使用了参数列表而非字符串拼接,但仍需确保unitName等参数不会被注入恶意内容

  2. 敏感信息

    • 日志中可能包含敏感信息
    • 没有对日志内容进行过滤
  3. 资源限制

    • 没有限制日志输出大小
    • 可能导致内存耗尽
  4. 建议

    • 对所有外部输入进行验证和清理
    • 实现日志内容过滤
    • 添加资源限制(如最大日志大小)
    • 考虑对敏感信息进行脱敏处理

其他建议

  1. 事件ID管理

    // 建议添加
    namespace EventIds {
        constexpr quint64 AppLaunch = 1000610001;
        constexpr quint64 AppLaunchFailed = 1000610002;
        constexpr quint64 AppLaunchDuration = 1000610003;
        constexpr quint64 AppAbnormalExit = 1000600012;
    }
  2. 异步处理

    // 建议使用QtConcurrent或QFuture处理耗时操作
    auto future = QtConcurrent::run([appId]() {
        return EventReporter::getAppVersion(appId);
    });
  3. 错误处理改进

    if (!proc.startDetached("journalctl", logArgs)) {
        qCWarning(amEventReporter) << "Failed to start journalctl";
        return;
    }
  4. 日志大小限制

    constexpr int MAX_LOG_SIZE = 10240; // 10KB
    QString logInfo = QString::fromUtf8(logProc.readAllStandardOutput());
    if (logInfo.size() > MAX_LOG_SIZE) {
        logInfo = logInfo.left(MAX_LOG_SIZE) + "... (truncated)";
    }

总结

这段代码实现了应用程序启动事件的记录功能,整体设计合理,但存在以下主要问题需要改进:

  1. 同步操作可能影响性能
  2. 错误处理不够完善
  3. 缺少对敏感信息的保护
  4. 魔法数字应定义为常量
  5. 缺少单元测试

建议在合并前重点解决同步操作和安全性问题,并添加适当的测试用例。

@ComixHe ComixHe removed their request for review May 9, 2026 02:07
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, ComixHe, Ivy233

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ComixHe ComixHe merged commit c45e3f2 into linuxdeepin:master May 9, 2026
18 checks passed
@Ivy233 Ivy233 deleted the feat/dde-am-eventlog1 branch May 9, 2026 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants