Skip to content

[LLDB][Telemetry]Define TargetInfo for collecting data about a target #127834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
0d6a36d
[LLDB][Telemetry]Define TargetInfo for collecting data about a target
oontvoo Feb 19, 2025
19c6181
Merge branch 'llvm:main' into target
oontvoo Feb 24, 2025
f152b3b
introduce helper
oontvoo Feb 24, 2025
4f8555a
undo change
oontvoo Mar 3, 2025
a5bd22e
Merge branch 'main' into target
oontvoo Mar 3, 2025
bb22874
update interface
oontvoo Mar 3, 2025
f49c3e2
formatting
oontvoo Mar 3, 2025
3bd8897
remove unused
oontvoo Mar 3, 2025
d3cb8d8
formatting
oontvoo Mar 3, 2025
267345a
qual
oontvoo Mar 4, 2025
32d949b
use UUID for target_uuid
oontvoo Mar 5, 2025
5c6c83c
Merge branch 'llvm:main' into target
oontvoo Mar 5, 2025
39c39cc
Merge branch 'main' into target
oontvoo Mar 6, 2025
1fdf120
revert dup
oontvoo Mar 8, 2025
1ed73e7
Merge branch 'target' of github.com:oontvoo/llvm-project into target
oontvoo Mar 8, 2025
92e06fa
Merge branch 'main' into target
oontvoo Mar 8, 2025
aab5002
unused include
oontvoo Mar 8, 2025
31c63e0
Update Telemetry.h
oontvoo Mar 8, 2025
20c8f59
Update Telemetry.cpp
oontvoo Mar 8, 2025
9954083
reconcile with main
oontvoo Mar 8, 2025
55aa4ee
Merge branch 'target' of github.com:oontvoo/llvm-project into target
oontvoo Mar 8, 2025
c8ca5b8
format
oontvoo Mar 8, 2025
4783b57
Update lldb/include/lldb/Core/Telemetry.h
oontvoo Mar 11, 2025
ae73ebe
reformatting
oontvoo Mar 12, 2025
0a2505a
Addressed review comments:
oontvoo Mar 12, 2025
4d3d26e
...
oontvoo Mar 12, 2025
b1b7a50
addressed review comment
oontvoo Mar 13, 2025
f7be9c9
remove unused import
oontvoo Mar 13, 2025
41f0696
Merge branch 'llvm:main' into target
oontvoo Mar 18, 2025
5e99cc7
addressed review comments
oontvoo Mar 18, 2025
a709ac3
Update Telemetry.h
oontvoo Mar 18, 2025
220554a
Update Telemetry.cpp
oontvoo Mar 18, 2025
1ecbe48
Update CommandInterpreter.cpp
oontvoo Mar 18, 2025
1231bc2
Update Target.cpp
oontvoo Mar 18, 2025
5757b41
Merge branch 'llvm:main' into target
oontvoo Mar 18, 2025
a936e2e
Merge branch 'target' of github.com:oontvoo/llvm-project into target
oontvoo Mar 18, 2025
142097a
more fixing
oontvoo Mar 18, 2025
d297813
formating
oontvoo Mar 18, 2025
885675e
Merge branch 'main' into target
oontvoo Mar 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 66 additions & 4 deletions lldb/include/lldb/Core/Telemetry.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@
#include <atomic>
#include <chrono>
#include <ctime>
#include <functional>
#include <memory>
#include <optional>
#include <string>
#include <type_traits>
#include <utility>

namespace lldb_private {
namespace telemetry {
Expand All @@ -46,12 +49,18 @@ struct LLDBConfig : public ::llvm::telemetry::Config {
// Specifically:
// - Length: 8 bits
// - First two bits (MSB) must be 11 - the common prefix
// - Last two bits (LSB) are reserved for grand-children of LLDBTelemetryInfo
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have to say I'm liking this bitfield system less and less every time I see it.

// If any of the subclass has descendents, those descendents
// must have their LLDBEntryKind in the similar form (ie., share common prefix)
// must have their LLDBEntryKind in the similar form (ie., share common prefix
// and differ by the last two bits)
struct LLDBEntryKind : public ::llvm::telemetry::EntryKind {
static const llvm::telemetry::KindType BaseInfo = 0b11000000;
static const llvm::telemetry::KindType CommandInfo = 0b11010000;
static const llvm::telemetry::KindType DebuggerInfo = 0b11000100;
// clang-format off
static const llvm::telemetry::KindType BaseInfo = 0b11000000;
static const llvm::telemetry::KindType CommandInfo = 0b11010000;
static const llvm::telemetry::KindType DebuggerInfo = 0b11001000;
static const llvm::telemetry::KindType ExecModuleInfo = 0b11000100;
static const llvm::telemetry::KindType ProcessExitInfo = 0b11001100;
// clang-format on
};

/// Defines a convenient type for timestamp of various events.
Expand Down Expand Up @@ -146,6 +155,59 @@ struct DebuggerInfo : public LLDBBaseTelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};

struct ExecModuleInfo : public LLDBBaseTelemetryInfo {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
struct ExecModuleInfo : public LLDBBaseTelemetryInfo {
struct ExecutableModuleInfo : public LLDBBaseTelemetryInfo {

lldb::ModuleSP exec_mod;
Copy link
Member

Choose a reason for hiding this comment

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

Let's drop the exec_ prefix as this is relatively obvious from the class it belongs to and we don't consistently prefix all the members.

Suggested change
lldb::ModuleSP exec_mod;
lldb::ModuleSP exec_mod;

Copy link
Member Author

Choose a reason for hiding this comment

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

done


/// The same as the executable-module's UUID.
UUID exec_uuid;
/// PID of the process owned by this target.
lldb::pid_t pid;
/// The triple of this executable module.
std::string triple;

/// If true, this entry was emitted at the beginning of an event (eg., before
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// If true, this entry was emitted at the beginning of an event (eg., before
/// If true, this entry was emitted at the beginning of an event (e.g. before

/// the executable is set). Otherwise, it was emitted at the end of an
/// event (eg., after the module and any dependency were loaded.)
bool is_start_entry;

ExecModuleInfo() = default;

llvm::telemetry::KindType getKind() const override {
return LLDBEntryKind::ExecModuleInfo;
}

static bool classof(const TelemetryInfo *T) {
// Subclasses of this is also acceptable
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Subclasses of this is also acceptable
// Subclasses of this is also acceptable.

return (T->getKind() & LLDBEntryKind::ExecModuleInfo) ==
LLDBEntryKind::ExecModuleInfo;
}
void serialize(llvm::telemetry::Serializer &serializer) const override;
};

/// Describes an exit status.
struct ExitDescription {
int exit_code;
Copy link
Member

Choose a reason for hiding this comment

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

Contrary to my other comments I think we can keep the exit_ prefix here as "exit code" is a fairly well understood term.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

std::string description;
};

struct ProcessExitInfo : public LLDBBaseTelemetryInfo {
UUID exec_uuid;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
UUID exec_uuid;
UUID uuid;

Copy link
Member Author

Choose a reason for hiding this comment

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

done

lldb::pid_t pid;
bool is_start_entry;
std::optional<ExitDescription> exit_desc;

llvm::telemetry::KindType getKind() const override {
return LLDBEntryKind::ProcessExitInfo;
}

static bool classof(const TelemetryInfo *T) {
// Subclasses of this is also acceptable
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Subclasses of this is also acceptable
// Subclasses of this is also acceptable.

return (T->getKind() & LLDBEntryKind::ProcessExitInfo) ==
LLDBEntryKind::ProcessExitInfo;
}
void serialize(llvm::telemetry::Serializer &serializer) const override;
};

/// The base Telemetry manager instance in LLDB.
/// This class declares additional instrumentation points
/// applicable to LLDB.
Expand Down
25 changes: 23 additions & 2 deletions lldb/source/Core/Telemetry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,36 @@ void CommandInfo::serialize(Serializer &serializer) const {
serializer.write("error_data", error_data.value());
}

std::atomic<uint64_t> CommandInfo::g_command_id_seed = 0;
uint64_t CommandInfo::GetNextId() { return g_command_id_seed.fetch_add(1); }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint64_t CommandInfo::GetNextId() { return g_command_id_seed.fetch_add(1); }
uint64_t CommandInfo::GetNextID() { return g_command_id_seed.fetch_add(1); }

Copy link
Member Author

Choose a reason for hiding this comment

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

done


void DebuggerInfo::serialize(Serializer &serializer) const {
LLDBBaseTelemetryInfo::serialize(serializer);

serializer.write("lldb_version", lldb_version);
serializer.write("is_exit_entry", is_exit_entry);
}

std::atomic<uint64_t> CommandInfo::g_command_id_seed = 0;
uint64_t CommandInfo::GetNextId() { return g_command_id_seed.fetch_add(1); }
void ExecModuleInfo::serialize(Serializer &serializer) const {
LLDBBaseTelemetryInfo::serialize(serializer);

serializer.write("exec_uuid", exec_uuid.GetAsString());
serializer.write("pid", pid);
serializer.write("triple", triple);
serializer.write("is_start_entry", is_start_entry);
}

void ProcessExitInfo::serialize(Serializer &serializer) const {
LLDBBaseTelemetryInfo::serialize(serializer);

serializer.write("exec_uuid", exec_uuid.GetAsString());
serializer.write("pid", pid);
serializer.write("is_start_entry", is_start_entry);
if (exit_desc.has_value()) {
serializer.write("exit_code", exit_desc->exit_code);
serializer.write("exit_desc", exit_desc->description);
}
}

TelemetryManager::TelemetryManager(std::unique_ptr<LLDBConfig> config)
: m_config(std::move(config)), m_id(MakeUUID()) {}
Expand Down
21 changes: 21 additions & 0 deletions lldb/source/Target/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Progress.h"
#include "lldb/Core/Telemetry.h"
#include "lldb/Expression/DiagnosticManager.h"
#include "lldb/Expression/DynamicCheckerFunctions.h"
#include "lldb/Expression/UserExpression.h"
Expand Down Expand Up @@ -1066,6 +1067,26 @@ const char *Process::GetExitDescription() {
bool Process::SetExitStatus(int status, llvm::StringRef exit_string) {
// Use a mutex to protect setting the exit status.
std::lock_guard<std::mutex> guard(m_exit_status_mutex);
telemetry::ScopedDispatcher<telemetry::ProcessExitInfo> helper;

// Find the executable-module's UUID, if available.
Target &target = GetTarget();
helper.SetDebugger(&(target.GetDebugger()));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
helper.SetDebugger(&(target.GetDebugger()));
helper.SetDebugger(&target.GetDebugger());

Copy link
Member Author

Choose a reason for hiding this comment

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

done

UUID exec_uuid;
if (ModuleSP exec_mod = target.GetExecutableModule())
exec_uuid = exec_mod->GetUUID();

helper.DispatchNow([&](telemetry::ProcessExitInfo *info) {
info->exec_uuid = exec_uuid;
info->pid = m_pid;
info->is_start_entry = true;
info->exit_desc = {status, exit_string.str()};
});

helper.DispatchOnExit([&](telemetry::ProcessExitInfo *info) {
info->exec_uuid = exec_uuid;
info->pid = m_pid;
});

Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
LLDB_LOG(log, "(plugin = {0} status = {1} ({1:x8}), description=\"{2}\")",
Expand Down
20 changes: 20 additions & 0 deletions lldb/source/Target/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "lldb/Core/Section.h"
#include "lldb/Core/SourceManager.h"
#include "lldb/Core/StructuredDataImpl.h"
#include "lldb/Core/Telemetry.h"
#include "lldb/DataFormatters/FormatterSection.h"
#include "lldb/Expression/DiagnosticManager.h"
#include "lldb/Expression/ExpressionVariable.h"
Expand Down Expand Up @@ -1559,10 +1560,29 @@ void Target::DidExec() {

void Target::SetExecutableModule(ModuleSP &executable_sp,
LoadDependentFiles load_dependent_files) {
telemetry::ScopedDispatcher<telemetry::ExecModuleInfo> helper(&m_debugger);
Log *log = GetLog(LLDBLog::Target);
ClearModules(false);

if (executable_sp) {
lldb::pid_t pid;
if (ProcessSP proc = GetProcessSP())
pid = proc->GetID();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
lldb::pid_t pid;
if (ProcessSP proc = GetProcessSP())
pid = proc->GetID();
lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
if (ProcessSP proc = GetProcessSP())
pid = proc->GetID();

Copy link
Member Author

Choose a reason for hiding this comment

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

done


helper.DispatchNow([&](telemetry::ExecModuleInfo *info) {
info->exec_mod = executable_sp;
info->exec_uuid = executable_sp->GetUUID();
info->pid = pid;
info->triple = executable_sp->GetArchitecture().GetTriple().getTriple();
info->is_start_entry = true;
});

helper.DispatchOnExit([&](telemetry::ExecModuleInfo *info) {
info->exec_mod = executable_sp;
info->exec_uuid = executable_sp->GetUUID();
info->pid = pid;
});

ElapsedTime elapsed(m_stats.GetCreateTime());
LLDB_SCOPED_TIMERF("Target::SetExecutableModule (executable = '%s')",
executable_sp->GetFileSpec().GetPath().c_str());
Expand Down