Skip to content

Commit 9cf1548

Browse files
gzadica1zma2
gzadica1
and
zma2
authored
[UNITRACE] Fix temp file regression with -k --result-dir (#1041)
* [UNITRACE] Fix temp file regression with -k --result-dir Temporary directory for data files is generated only after logger factory already instantiated in case --result-dir is used. Sample the data dir environemnt when a data file needs to be created and not only during logger factory construction. * Exit with error if data directory did not defined at a stage we must have it. * Update logger_factory.h --------- Co-authored-by: Zhiqiang Ma <zhiqiang.ma@intel.com>
1 parent a18d3d3 commit 9cf1548

2 files changed

Lines changed: 25 additions & 17 deletions

File tree

‎tools/unitrace/src/utils/logger_factory.cc‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ LoggerFactory::LoggerFactory(uint32_t app_id)
2121
: app_id_(app_id),
2222
dir_path_(""),
2323
app_name_(ComputeAppName()),
24-
rank_((utils::GetEnv("PMI_RANK").empty()) ? utils::GetEnv("PMIX_RANK") : utils::GetEnv("PMI_RANK")),
25-
data_dir_path_(utils::GetEnv("UNITRACE_DataDir"))
24+
rank_((utils::GetEnv("PMI_RANK").empty()) ? utils::GetEnv("PMIX_RANK") : utils::GetEnv("PMI_RANK"))
2625
{}
2726

2827
// LoggerFactory directory creation helper
@@ -239,15 +238,15 @@ std::string LegacyLoggerFactory::GenerateLogFileName(LoggerType type, int32_t de
239238
case LOGGER_TYPE_METRICS_SAMPLING:
240239
return GetMetricsFileName(log_file_, app_id_);
241240
case LOGGER_TYPE_KPROPS:
242-
return data_dir_path_ + "/.kprops." + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
241+
return GetDataDirPath() + "/.kprops." + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
243242
case LOGGER_TYPE_KTIME:
244-
return data_dir_path_ + "/.ktime." + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
243+
return GetDataDirPath() + "/.ktime." + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
245244
case LOGGER_TYPE_METRICS_QUERY_TEMP:
246-
return data_dir_path_ + "/.metrics." + std::to_string(app_id_) + ".q";
245+
return GetDataDirPath() + "/.metrics." + std::to_string(app_id_) + ".q";
247246
case LOGGER_TYPE_METRICS_SAMPLING_TEMP:
248247
{
249248
std::string metric_group = utils::GetEnv("UNITRACE_MetricGroup");
250-
return data_dir_path_ + "/." + std::to_string(device_id) + "." + metric_group + "." + std::to_string(app_id_) + ".t";
249+
return GetDataDirPath() + "/." + std::to_string(device_id) + "." + metric_group + "." + std::to_string(app_id_) + ".t";
251250
}
252251
case LOGGER_TYPE_KMD_TRACE:
253252
return GetLogFileName("oskmd.json", app_id_);
@@ -263,7 +262,7 @@ std::string LegacyLoggerFactory::GenerateLogFileName(LoggerType type, int32_t de
263262
std::pair<std::string, std::string> LegacyLoggerFactory::SearchRawMetricFiles(uint32_t pid) const {
264263
std::pair<std::string, std::string> result;
265264

266-
std::string temp_file = data_dir_path_ + "/.metrics." + std::to_string(pid) + ".q";
265+
std::string temp_file = GetDataDirPath() + "/.metrics." + std::to_string(pid) + ".q";
267266
std::string final_file = GetMetricsFileName(log_file_, pid);
268267
result = std::make_pair(temp_file, final_file);
269268
return result;
@@ -287,8 +286,9 @@ std::vector<std::string> LegacyLoggerFactory::SearchFilesByType(LoggerType type,
287286
return result;
288287
}
289288

290-
if (!data_dir_path_.empty() && CXX_STD_FILESYSTEM_NAMESPACE::exists(data_dir_path_) && CXX_STD_FILESYSTEM_NAMESPACE::is_directory(data_dir_path_)) {
291-
for (const auto& e : CXX_STD_FILESYSTEM_NAMESPACE::directory_iterator(CXX_STD_FILESYSTEM_NAMESPACE::path(data_dir_path_))) {
289+
std::string data_dir_path = GetDataDirPath(false);
290+
if (!data_dir_path.empty() && CXX_STD_FILESYSTEM_NAMESPACE::exists(data_dir_path) && CXX_STD_FILESYSTEM_NAMESPACE::is_directory(data_dir_path)) {
291+
for (const auto& e : CXX_STD_FILESYSTEM_NAMESPACE::directory_iterator(CXX_STD_FILESYSTEM_NAMESPACE::path(data_dir_path))) {
292292
if (e.path().filename().string().find(prefix + std::to_string(device_id)) == 0) {
293293
result.push_back(e.path().string());
294294
}
@@ -378,15 +378,15 @@ std::string ResultDirLoggerFactory::GenerateLogFileName(LoggerType type, int32_t
378378
CreateDirectory(metrics_dir_path_);
379379
return metrics_dir_path_ + "/metrics_" + std::to_string(device_id) + ".csv";
380380
case LOGGER_TYPE_KPROPS:
381-
return data_dir_path_ + "/kprops_" + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
381+
return GetDataDirPath() + "/kprops_" + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
382382
case LOGGER_TYPE_KTIME:
383-
return data_dir_path_ + "/ktime_" + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
383+
return GetDataDirPath() + "/ktime_" + std::to_string(device_id) + "." + std::to_string(app_id_) + ".txt";
384384
case LOGGER_TYPE_METRICS_QUERY_TEMP:
385-
return data_dir_path_ + "/metrics." + std::to_string(app_id_) + ".q";
385+
return GetDataDirPath() + "/metrics." + std::to_string(app_id_) + ".q";
386386
case LOGGER_TYPE_METRICS_SAMPLING_TEMP:
387387
{
388388
std::string metric_group = utils::GetEnv("UNITRACE_MetricGroup");
389-
return data_dir_path_ + "/" + std::to_string(device_id) + "." + metric_group + "." + std::to_string(app_id_) + ".t";
389+
return GetDataDirPath() + "/" + std::to_string(device_id) + "." + metric_group + "." + std::to_string(app_id_) + ".t";
390390
}
391391
default:
392392
PTI_ASSERT(false);
@@ -398,7 +398,7 @@ std::string ResultDirLoggerFactory::GenerateLogFileName(LoggerType type, int32_t
398398
// for windows - father process need to find .q file that was created by child process and prepare final metrics files
399399
std::pair<std::string, std::string> ResultDirLoggerFactory::SearchRawMetricFiles(uint32_t pid) const {
400400
std::pair<std::string, std::string> result;
401-
std::string temp_file = data_dir_path_ + "/metrics." + std::to_string(pid) + ".q";
401+
std::string temp_file = GetDataDirPath() + "/metrics." + std::to_string(pid) + ".q";
402402
if (result_dir_.empty() || !CXX_STD_FILESYSTEM_NAMESPACE::exists(result_dir_) || !CXX_STD_FILESYSTEM_NAMESPACE::is_directory(result_dir_)) {
403403
return result;
404404
}
@@ -436,8 +436,9 @@ std::vector<std::string> ResultDirLoggerFactory::SearchFilesByType(LoggerType ty
436436
return result;
437437
}
438438

439-
if (!data_dir_path_.empty() && CXX_STD_FILESYSTEM_NAMESPACE::exists(data_dir_path_) && CXX_STD_FILESYSTEM_NAMESPACE::is_directory(data_dir_path_)) {
440-
for (const auto& e : CXX_STD_FILESYSTEM_NAMESPACE::directory_iterator(CXX_STD_FILESYSTEM_NAMESPACE::path(data_dir_path_))) {
439+
std::string data_dir_path = GetDataDirPath(false);
440+
if (!data_dir_path.empty() && CXX_STD_FILESYSTEM_NAMESPACE::exists(data_dir_path) && CXX_STD_FILESYSTEM_NAMESPACE::is_directory(data_dir_path)) {
441+
for (const auto& e : CXX_STD_FILESYSTEM_NAMESPACE::directory_iterator(CXX_STD_FILESYSTEM_NAMESPACE::path(data_dir_path))) {
441442
if (e.path().filename().string().find(prefix + std::to_string(device_id)) == 0) {
442443
result.push_back(e.path().string());
443444
}

‎tools/unitrace/src/utils/logger_factory.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,19 @@ class LoggerFactory {
6868
void CreateDirectory(const std::string& dir) const;
6969
std::shared_ptr<Logger> GetLoggerImpl(LoggerType type, int32_t device_id, bool lazy_flush, bool lock_free) const;
7070
void SetAppId(uint32_t app_id) {app_id_ = app_id;}
71+
std::string GetDataDirPath(bool warn = true) const {
72+
std::string data_dir = utils::GetEnv("UNITRACE_DataDir");
73+
if (data_dir.empty() && warn) {
74+
std::cerr << "[ERROR] Data directory is missing or not specified." << std::endl;
75+
exit(-1); // Bail out if data directory is not specified at this point.
76+
}
77+
return data_dir;
78+
}
7179

7280
uint32_t app_id_;
7381
const std::string app_name_;
7482
const std::string rank_;
7583
std::string dir_path_; // Directory path for output, empty by default
76-
std::string data_dir_path_; // path for temporary files
7784
mutable std::mutex mutex_;
7885
mutable std::map<std::pair<LoggerType, int32_t>, std::shared_ptr<Logger>> loggers_;
7986
};

0 commit comments

Comments
 (0)