Skip to content

Commit 5aec284

Browse files
committed
adddressed review comments:
- remove collection of username,cwd,lldb_path (that can be collected in vendor-specific impl) - use DebuggerID as key rather than the debugger pointer - renaming
1 parent 11efc09 commit 5aec284

File tree

2 files changed

+44
-55
lines changed

2 files changed

+44
-55
lines changed

lldb/include/lldb/Core/Telemetry.h

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,24 +66,15 @@ struct ExitDescription {
6666
std::string description;
6767
};
6868

69-
struct DebuggerTelemetryInfo : public LLDBBaseTelemetryInfo {
70-
std::string username;
71-
std::string lldb_git_sha;
72-
std::string lldb_path;
73-
std::string cwd;
69+
struct DebuggerInfo : public LLDBBaseTelemetryInfo {
70+
std::string lldb_version;
7471
std::optional<ExitDescription> exit_desc;
7572

76-
DebuggerTelemetryInfo() = default;
73+
std::string lldb_path;
74+
std::string cwd;
75+
std::string username;
7776

78-
// Provide a copy ctor because we may need to make a copy before
79-
// sanitizing the data.
80-
// (The sanitization might differ between different Destination classes).
81-
DebuggerTelemetryInfo(const DebuggerTelemetryInfo &other) {
82-
username = other.username;
83-
lldb_git_sha = other.lldb_git_sha;
84-
lldb_path = other.lldb_path;
85-
cwd = other.cwd;
86-
};
77+
DebuggerInfo() = default;
8778

8879
llvm::telemetry::KindType getKind() const override {
8980
return LLDBEntryKind::DebuggerInfo;
@@ -134,8 +125,8 @@ class TelemetryManager : public llvm::telemetry::Manager {
134125

135126
const llvm::telemetry::Config *getConfig();
136127

137-
void atDebuggerStartup(DebuggerTelemetryInfo *entry);
138-
void atDebuggerExit(DebuggerTelemetryInfo *entry);
128+
virtual void AtDebuggerStartup(DebuggerInfo *entry);
129+
virtual void AtDebuggerExit(DebuggerInfo *entry);
139130

140131
virtual llvm::StringRef GetInstanceName() const = 0;
141132
static TelemetryManager *getInstance();
@@ -147,10 +138,13 @@ class TelemetryManager : public llvm::telemetry::Manager {
147138

148139
private:
149140
std::unique_ptr<llvm::telemetry::Config> m_config;
150-
// Each debugger is assigned a unique ID (session_id).
141+
// Each instance of a TelemetryManager is assigned a unique ID.
142+
const std::string m_id;
143+
144+
// Map of debugger's ID to a unique session_id string.
151145
// All TelemetryInfo entries emitted for the same debugger instance
152146
// will get the same session_id.
153-
llvm::DenseMap<Debugger *, std::string> session_ids;
147+
llvm::DenseMap<lldb::user_id_t, std::string> session_ids;
154148
static std::unique_ptr<TelemetryManager> g_instance;
155149
};
156150

lldb/source/Core/Telemetry.cpp

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,32 @@ static uint64_t ToNanosec(const SteadyTimePoint Point) {
4141
return std::chrono::nanoseconds(Point.time_since_epoch()).count();
4242
}
4343

44-
static std::string MakeUUID(Debugger *debugger) {
44+
// Generate a unique string. This should be unique across different runs.
45+
// We build such string by combining three parts:
46+
// <16 random bytes>_<timestamp>_<hash of username>
47+
// This reduces the chances of getting the same UUID, even when the same
48+
// user runs the two copies of binary at the same time.
49+
static std::string MakeUUID() {
4550
uint8_t random_bytes[16];
51+
std::string randomString = "_";
4652
if (auto ec = llvm::getRandomBytes(random_bytes, 16)) {
4753
LLDB_LOG(GetLog(LLDBLog::Object),
4854
"Failed to generate random bytes for UUID: {0}", ec.message());
49-
// Fallback to using timestamp + debugger ID.
50-
return llvm::formatv(
51-
"{0}_{1}", std::chrono::steady_clock::now().time_since_epoch().count(),
52-
debugger->GetID());
55+
} else {
56+
randomString = UUID(random_bytes).GetAsString();
5357
}
54-
return UUID(random_bytes).GetAsString();
58+
59+
std::string username = "_";
60+
UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
61+
std::optional<llvm::StringRef> opt_username =
62+
resolver.GetUserName(lldb_private::HostInfo::GetUserID());
63+
if (opt_username)
64+
username = *opt_username;
65+
66+
return llvm::formatv(
67+
"{0}_{1}_{2}", randomString,
68+
std::chrono::steady_clock::now().time_since_epoch().count(),
69+
llvm::MD5Hash(username));
5570
}
5671

5772
void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
@@ -62,7 +77,7 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
6277
serializer.write("end_time", ToNanosec(end_time.value()));
6378
}
6479

65-
void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
80+
void DebuggerInfo::serialize(Serializer &serializer) const {
6681
LLDBBaseTelemetryInfo::serialize(serializer);
6782

6883
serializer.write("username", username);
@@ -85,18 +100,21 @@ void MiscTelemetryInfo::serialize(Serializer &serializer) const {
85100
}
86101

87102
TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
88-
: m_config(std::move(config)) {}
103+
: m_config(std::move(config)), m_id(MakeUUID) {}
89104

90105
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
106+
// Look up the session_id to assign to this entry or make one
107+
// if none had been computed for this debugger.
91108
LLDBBaseTelemetryInfo *lldb_entry =
92109
llvm::dyn_cast<LLDBBaseTelemetryInfo>(entry);
93-
std::string session_id = "";
110+
std::string session_id = m_id;
94111
if (Debugger *debugger = lldb_entry->debugger) {
95-
auto session_id_pos = session_ids.find(debugger);
112+
auto session_id_pos = session_ids.find(debugger->getID());
96113
if (session_id_pos != session_ids.end())
97114
session_id = session_id_pos->second;
98115
else
99-
session_id_pos->second = session_id = MakeUUID(debugger);
116+
session_id_pos->second = session_id =
117+
llvm::formatv("{0}_{1}", m_id, debugger->getID());
100118
}
101119
lldb_entry->SessionId = session_id;
102120

@@ -105,36 +123,13 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
105123

106124
const Config *getConfig() { return m_config.get(); }
107125

108-
void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
109-
UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
110-
std::optional<llvm::StringRef> opt_username =
111-
resolver.GetUserName(lldb_private::HostInfo::GetUserID());
112-
if (opt_username)
113-
entry->username = *opt_username;
114-
115-
entry->lldb_git_sha =
116-
lldb_private::GetVersion(); // TODO: find the real git sha?
117-
118-
entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath();
119-
120-
llvm::SmallString<64> cwd;
121-
if (!llvm::sys::fs::current_path(cwd)) {
122-
entry->cwd = cwd.c_str();
123-
} else {
124-
MiscTelemetryInfo misc_info;
125-
misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
126-
if (llvm::Error er = dispatch(&misc_info)) {
127-
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
128-
"Failed to dispatch misc-info at startup: {0}");
129-
}
130-
}
131-
126+
void TelemetryManager::AtDebuggerStartup(DebuggerInfo *entry) {
132127
if (auto er = dispatch(entry)) {
133128
LLDB_LOG(GetLog(LLDBLog::Object), "Failed to dispatch entry at startup");
134129
}
135130
}
136131

137-
void TelemetryManager::atDebuggerExit(DebuggerTelemetryInfo *entry) {
132+
void TelemetryManager::AtDebuggerExit(DebuggerInfo *entry) {
138133
// There must be a reference to the debugger at this point.
139134
assert(entry->debugger != nullptr);
140135

0 commit comments

Comments
 (0)