Clean up file hashing logic and PlatformFile - #9039
Open
zwass wants to merge 5 commits into
Open
Conversation
This method is only used in the file hashing logic. As pointed out in osquery#8939, that usage was incorrect. Because it is unused besides this, removing it entirely. Other places in the codebase already use std::is_regular_file.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors PlatformFile and file-reading logic, primarily aiming to (1) fix Windows file size reporting for files larger than 4GB and (2) simplify/remove the PlatformFile::isSpecialFile() API that was being used incorrectly.
Changes:
- Windows: switch
PlatformFile::size()toGetFileSizeEx()to correctly report sizes >4GB. - Remove
PlatformFile::isSpecialFile()from both Windows and POSIX implementations and drop related assertions from tests. - Simplify
readFile()loops by removing the special-file gating logic (but this currently reintroduces “special file” hazards).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| osquery/filesystem/windows/fileops.cpp | Uses GetFileSizeEx() for 64-bit file size reporting on Windows. |
| osquery/filesystem/posix/fileops.cpp | Removes the POSIX isSpecialFile() implementation. |
| osquery/filesystem/filesystem.cpp | Removes special-file handling from readFile() loops (needs rework to avoid FIFO/pipe hazards). |
| osquery/filesystem/fileops.h | Removes the isSpecialFile() API from PlatformFile. |
| osquery/filesystem/tests/fileops.cpp | Updates tests to stop referencing isSpecialFile(). |
Suppressed comments (2)
osquery/filesystem/filesystem.cpp:215
- This overload now retries on
hasPendingIo()without checking whether the target is a regular file. For FIFOs/sockets on POSIX (EAGAIN) or named pipes on Windows (incomplete overlapped I/O), that can busy-loop indefinitely.
After introducing an is_regular_file boolean earlier in this function (see comment near the read-size initialization), gate the pending-IO retry to regular files only.
}
}
} while (read_size > 0 && (res > 0 || file_handle.hasPendingIo()));
osquery/filesystem/filesystem.cpp:152
- The loop condition now retries on
hasPendingIo()unconditionally. On POSIX,PlatformFile::read()setshas_pending_io_onEAGAINfor non-blocking FIFOs/sockets; on Windows, it can be set for incomplete overlapped I/O on named pipes. Retrying on pending I/O for these special file types can create a tight infinite loop.
After introducing an is_regular_file boolean earlier in this function (see comment on the loop setup above), gate the retry-on-pending path to regular files only, restoring the behavior documented in filesystem.h (stop reading special files when data is not immediately available).
predicate({buffer, static_cast<std::size_t>(res)});
}
} while (res > 0 || file_handle.hasPendingIo());
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
177
to
179
| /* We read in blocks only if we don't know the file size; | ||
| otherwise use the file size for efficiency */ | ||
| std::size_t read_size = 0; |
Comment on lines
129
to
131
| ssize_t res = 0; | ||
| std::size_t total_bytes = 0; | ||
| char buffer[kBlockSize]; |
Comment on lines
220
to
224
| { | ||
| std::vector<char> buf(expected_read_len); | ||
| PlatformFile fd(path, PF_OPEN_EXISTING | PF_READ); | ||
| ASSERT_TRUE(fd.isValid()); | ||
| ASSERT_FALSE(fd.isSpecialFile()); | ||
| EXPECT_EQ(expected_read_len, fd.read(buf.data(), expected_read_len)); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Suppressed comments (10)
osquery/tables/system/darwin/gpu_metrics.mm:304
- This per-channel diagnostic log runs during normal table execution and can quickly flood logs on systems with many IOReport channels. Prefer VLOG (or guard behind a verbose flag) for this high-volume output.
LOG(INFO) << "IOReport(all) GPU power candidate group='" << group_string
<< "' name='" << name_string << "' value=" << value
<< " unit='" << unit_string << "' watts=" << channel_power_w;
++logged;
osquery/tables/system/darwin/gpu_metrics.mm:316
- These LOG(INFO) lines will run during normal table execution and can be noisy. Consider lowering to VLOG so production logs aren't flooded by periodic table queries.
LOG(INFO) << "Total GPU power from all-channel scan: " << total_power_w
<< " W";
power_data.push_back(total_power_w);
} else {
LOG(INFO) << "All-channel GPU power scan found no matching candidates";
}
osquery/tables/system/windows/gpu_metrics.cpp:12
- is required for std::isdigit/isdigit usage; relying on transitive includes can break builds depending on header ordering.
#include <algorithm>
#include <map>
osquery/tables/system/windows/gpu_metrics.cpp:94
- name_it is dereferenced unconditionally, but only type_it is validated. If queryKey ever returns a row without a "name" field, this will be undefined behavior. Guard name_it before using it.
const auto type_it = row.find("type");
const auto name_it = row.find("name");
if (type_it == row.end() || type_it->second != "subkey") {
continue;
osquery/tables/system/windows/gpu_metrics.cpp:104
- Prefer std::isdigit (and avoid the global isdigit macro) for correct overload resolution and to match C++ style; this also prevents subtle issues on some toolchains.
for (char c : subkeyName) {
if (!isdigit(static_cast<unsigned char>(c))) {
numeric = false;
break;
osquery/tables/system/linux/gpu_metrics.cpp:13
- This file uses std::vector but does not include ; relying on indirect includes can break builds depending on platform/header ordering.
#include <fstream>
#include <memory>
#include <optional>
#include <string>
osquery/tables/system/linux/gpu_metrics.cpp:313
- gpu_index needs to be incremented after emitting a GPU row; otherwise every row will report 0.
results.emplace_back(std::move(r));
}
osquery/tables/system/darwin/gpu_metrics.mm:220
- Avoid emitting LOG(INFO) on every table query when IOReport channels are missing; this is expected on some systems and can spam logs. Prefer VLOG for diagnostic output.
This issue also appears in the following locations of the same file:
- line 301
- line 311
LOG(INFO) << "No IOReport channels available";
osquery/tables/system/darwin/gpu_metrics.mm:217
- collectGPUPowerData() unconditionally sleeps for 0.5s and scans all IOReport channels, which makes every gpu_metrics query take at least 500ms even when power_draw_watts is ultimately unused (e.g., multiple GPUs). Consider gating this behind a smaller sample interval, caching, or only collecting power when it will be reported.
constexpr useconds_t kSampleIntervalUs = 500U * 1000U;
constexpr double kSampleIntervalSeconds = 0.5;
specs/gpu_metrics.table:2
- The PR description/title focus on file hashing and PlatformFile behavior, but this PR also introduces a brand new cross-platform gpu_metrics table (specs + implementations + integration test). If the intent is only the hashing/PlatformFile fix, consider splitting gpu_metrics into a separate PR to keep scope and review risk contained.
table_name("gpu_metrics")
description("Information about GPU devices on the system, including inventory and runtime telemetry where supported.")
Comment on lines
1267
to
1273
| size_t PlatformFile::size() const { | ||
| return ::GetFileSize(handle_, nullptr); | ||
| LARGE_INTEGER file_size{}; | ||
| if (!::GetFileSizeEx(handle_, &file_size)) { | ||
| return 0; | ||
| } | ||
| return static_cast<size_t>(file_size.QuadPart); | ||
| } |
Comment on lines
+3
to
+7
| schema([ | ||
| Column("vendor_name", TEXT, "The vendor name of the GPU."), | ||
| Column("device_name", TEXT, "The model/device name of the GPU."), | ||
| Column("driver_version", TEXT, "The installed driver version for the GPU.", collate="version"), | ||
| Column("vram_total_bytes", BIGINT, "Total video RAM in bytes."), |
Comment on lines
+149
to
+152
| int gpu_index = 0; | ||
| for (const auto& item : wmiReq->results()) { | ||
| Row r; | ||
|
|
Comment on lines
+208
to
+212
| struct udev_list_entry *device_entries, *entry; | ||
| device_entries = udev_enumerate_get_list_entry(enumerate.get()); | ||
|
|
||
| udev_list_entry_foreach(entry, device_entries) { | ||
| const char* path = udev_list_entry_get_name(entry); |
Comment on lines
+227
to
+229
| Row r; | ||
| r["pci_bus"] = UdevEventPublisher::getValue(device.get(), kGpuPCIKeySlot); | ||
|
|
Comment on lines
+526
to
+528
| int gpu_index = 0; | ||
| for (NSDictionary* item in items) { | ||
| Row r; |
Comment on lines
174
to
179
| return status; | ||
| } | ||
|
|
||
| const bool isSpecialFile = file_handle.isSpecialFile(); | ||
|
|
||
| /* If the file is a regular file on disk and has no data, | ||
| do not attempt to read */ | ||
| if (!isSpecialFile && file_size == 0) { | ||
| return Status::success(); | ||
| } | ||
|
|
||
| /* We read in blocks only if we don't know the file size; | ||
| otherwise use the file size for efficiency */ | ||
| std::size_t read_size = 0; |
Comment on lines
126
to
131
| return status; | ||
| } | ||
|
|
||
| const bool isSpecialFile = file_handle.isSpecialFile(); | ||
|
|
||
| /* If the file is a regular file on disk and has no data, | ||
| do not attempt to read */ | ||
| if (!isSpecialFile && file_size == 0) { | ||
| return Status::success(); | ||
| } | ||
|
|
||
| ssize_t res = 0; | ||
| std::size_t total_bytes = 0; | ||
| char buffer[kBlockSize]; |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (15)
specs/gpu_metrics.table:7
- The integration test expects a
gpu_indexcolumn, but the table spec does not define it and none of the implementations populate it. This will cause the new integration test to fail and makes row correlation harder.
schema([
Column("vendor_name", TEXT, "The vendor name of the GPU."),
Column("device_name", TEXT, "The model/device name of the GPU."),
Column("driver_version", TEXT, "The installed driver version for the GPU.", collate="version"),
Column("vram_total_bytes", BIGINT, "Total video RAM in bytes."),
osquery/tables/system/windows/gpu_metrics.cpp:153
- The table spec/integration test expects
gpu_index, but the Windows implementation never sets it in the output row.
int gpu_index = 0;
for (const auto& item : wmiReq->results()) {
Row r;
item.GetString("AdapterCompatibility", r["vendor_name"]);
osquery/tables/system/linux/gpu_metrics.cpp:212
- The table spec/integration test expects
gpu_index, but the Linux implementation does not currently track or emit a stable per-row index.
struct udev_list_entry *device_entries, *entry;
device_entries = udev_enumerate_get_list_entry(enumerate.get());
udev_list_entry_foreach(entry, device_entries) {
const char* path = udev_list_entry_get_name(entry);
osquery/tables/system/linux/gpu_metrics.cpp:229
- Emit
gpu_indexfor each returned row so callers (and tests) can correlate rows across queries and platforms.
Row r;
r["pci_bus"] = UdevEventPublisher::getValue(device.get(), kGpuPCIKeySlot);
osquery/tables/system/linux/gpu_metrics.cpp:313
- Increment the
gpu_indexcounter after emitting a row so each GPU gets a unique index.
results.emplace_back(std::move(r));
}
osquery/tables/system/darwin/gpu_metrics.mm:529
- The table spec/integration test expects
gpu_index, but the Darwin implementation does not include it in the output row.
int gpu_index = 0;
for (NSDictionary* item in items) {
Row r;
osquery/tables/system/darwin/gpu_metrics.mm:501
- To avoid the 0.5s sampling delay on multi-GPU systems, only call
collectGPUPowerData()whenresult.size() == 1(the only case where the value is used).
// Power data is a system-wide total; only meaningful when one GPU is present.
if (result.size() == 1 && !gpu_power_data.empty()) {
result[0].power_draw_watts = gpu_power_data[0];
}
osquery/filesystem/filesystem.cpp:125
readFile()is used by the hashing code path; with PF_NONBLOCK it can destructively consume FIFO data (see #8939). Also, with the updatedres > 0 || hasPendingIo()loop, non-regular files that return EAGAIN can cause a busy-loop. Add a regular-file guard before opening/reading.
const auto file_size_opt = file_handle.size();
if (!file_size_opt) {
return Status::failure("Cannot determine size of: " + path.string());
}
const std::uint64_t file_size = *file_size_opt;
osquery/filesystem/filesystem.cpp:176
- Same issue as the predicate overload: this variant should refuse to open/read non-regular files (FIFOs, devices). Otherwise hashing via readFile() can consume FIFO contents and the hasPendingIo loop can spin on EAGAIN.
const auto file_size_opt = file_handle.size();
if (!file_size_opt) {
return Status::failure("Cannot determine size of: " + path.string());
}
const std::uint64_t file_size = *file_size_opt;
osquery/tables/system/windows/gpu_metrics.cpp:12
- This file uses
isdigit(...)but does not include . Relying on transitive includes is brittle and may fail to compile on some toolchains.
#include <algorithm>
#include <map>
osquery/tables/system/windows/gpu_metrics.cpp:97
collectVramSizes()dereferencesname_it->secondwithout checkingname_it != row.end(), which can cause undefined behavior if the registry query returns a row missing the "name" field.
const auto type_it = row.find("type");
const auto name_it = row.find("name");
if (type_it == row.end() || type_it->second != "subkey") {
continue;
}
osquery/tables/system/darwin/gpu_metrics.mm:233
IOReportCreateSubscriptionreturns a subscription reference that should be released; otherwise repeated queries will leak. Add a scope guard to CFRelease the subscription when leaving the autoreleasepool scope.
CFMutableDictionaryRef subbed_channels = nullptr;
IOReportSubscriptionRef sub = IOReportCreateSubscription(
nullptr, all_channels, &subbed_channels, 0, nullptr);
const auto subbed_channels_guard = scope_guard::CFRelease(subbed_channels);
osquery/tables/system/darwin/gpu_metrics.mm:316
- Logging per-channel GPU power candidates at INFO (up to 20 lines per query) is very noisy for production deployments. This should be downgraded to VLOG/DEBUG so normal table queries don’t spam logs.
LOG(INFO) << "IOReport(all) GPU power candidate group='" << group_string
<< "' name='" << name_string << "' value=" << value
<< " unit='" << unit_string << "' watts=" << channel_power_w;
++logged;
}
osquery/tables/system/darwin/gpu_metrics.mm:429
collectGPUPowerData()samples via IOReport and sleeps for 0.5s; calling it unconditionally means everygpu_metricsquery pays that cost even on multi-GPU systems where the result is discarded. Defer the sampling until you know it will be used.
This issue also appears on line 498 of the same file.
// Collect GPU power data via private framework (once for all GPUs)
const auto gpu_power_data = collectGPUPowerData();
specs/gpu_metrics.table:2
- This PR’s title/description focuses on file hashing and PlatformFile, but it also introduces a new
gpu_metricstable (spec + 3 platform implementations + integration test). Consider either updating the PR description/title to reflect the new table, or splitting the GPU work into a separate PR for easier review/revert.
table_name("gpu_metrics")
description("Information about GPU devices on the system, including inventory and runtime telemetry where supported.")
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
osquery/filesystem/filesystem.cpp:223
- Same issue as the streaming readFile overload: for non-regular files (FIFOs/sockets/devices) opened nonblocking, an EAGAIN read sets hasPendingIo() and the loop condition can spin indefinitely. Refusing to read non-regular files avoids both the busy loop and destructive FIFO reads (see #8939).
PlatformFile file_handle(path, PF_OPEN_EXISTING | PF_READ | PF_NONBLOCK);
if (!file_handle.isValid()) {
return Status::failure("Cannot open file for reading: " +
file_handle.getFilePath().string());
}
const auto file_size_opt = file_handle.size();
if (!file_size_opt) {
return Status::failure("Cannot determine size of: " + path.string());
}
const std::uint64_t file_size = *file_size_opt;
// Fail to read if the file is bigger than the configured limit
auto status = checkFileReadLimit(file_size, path, shouldLog);
if (!status.ok()) {
return status;
}
/* We read in blocks only if we don't know the file size;
otherwise use the file size for efficiency */
std::size_t read_size = 0;
if (file_size > 0) {
read_size = file_size;
content.resize(file_size);
} else {
read_size = kBlockSize;
content.resize(kBlockSize);
}
std::size_t offset = 0;
ssize_t res = 0;
do {
res = file_handle.read(&content[offset], read_size);
// EOF
if (res == 0) {
break;
}
if (res > 0) {
offset += res;
auto status = checkFileReadLimit(offset, path, shouldLog);
if (!status.ok()) {
content.clear();
return status;
}
if (file_size > 0) {
read_size = file_size - offset;
} else {
content.resize(content.size() + kBlockSize);
}
}
} while (read_size > 0 && (res > 0 || file_handle.hasPendingIo()));
tests/integration/tables/gpu_metrics.cpp:41
gpu_indexis validated by the integration test, but the gpu_metrics table spec does not define agpu_indexcolumn (and the current implementations also don’t populate it). This will makeselect * from gpu_metricsfail validation. Either addgpu_indexto the table schema (and set it in each platform implementation) or drop it from the test expectations.
ValidationMap row_map = {
{"gpu_index", NonNegativeInt},
{"vendor_name", NormalType},
{"device_name", NormalType},
{"driver_version", NormalType},
{"vram_total_bytes", IntOrEmpty},
{"gpu_utilization_pct", NormalType},
};
specs/gpu_metrics.table:13
pci_busis documented as a PCI bus address, but the Darwin implementation populates this field with a bus type (e.g., "PCIe", "Built-In"). The column description should be relaxed to match the cross-platform meaning to avoid misleading API consumers.
extended_schema(POSIX, [
Column("pci_bus", TEXT, "The PCI bus address of the GPU."),
Column("power_draw_watts", DOUBLE, "Current GPU power draw in Watts."),
osquery/tables/system/windows/gpu_metrics.cpp:98
collectVramSizes()dereferencesname_itwithout checking it exists. IfqueryKey()ever returns a row missing the "name" field, this will be undefined behavior/crash. Add aname_it == row.end()guard alongside the existingtype_itcheck.
const auto type_it = row.find("type");
const auto name_it = row.find("name");
if (type_it == row.end() || type_it->second != "subkey") {
continue;
}
// Skip non-numeric subkeys (e.g. "Properties").
const std::string& subkeyName = name_it->second;
bool numeric = !subkeyName.empty();
specs/gpu_metrics.table:3
- The PR title/description focus on hashing + PlatformFile, but this change set also introduces a new gpu_metrics table (spec + 3 platform implementations + integration test). To keep review/traceability clear, consider updating the PR description/title to mention the new table (or splitting into a separate PR).
table_name("gpu_metrics")
description("Information about GPU devices on the system, including inventory and runtime telemetry where supported.")
schema([
osquery/tables/system/darwin/gpu_metrics.mm:315
- The table currently logs GPU power scan details at INFO level. Since this table may be queried frequently, these INFO logs can become noisy in production; consider using VLOG (or removing) for the success-path diagnostic messages.
LOG(INFO) << "Total GPU power from all-channel scan: " << total_power_w
<< " W";
power_data.push_back(total_power_w);
} else {
LOG(INFO) << "All-channel GPU power scan found no matching candidates";
Comment on lines
114
to
+155
| PlatformFile file_handle(path, PF_OPEN_EXISTING | PF_READ | PF_NONBLOCK); | ||
|
|
||
| if (!file_handle.isValid()) { | ||
| return Status::failure("Cannot open file for reading: " + | ||
| file_handle.getFilePath().string()); | ||
| } | ||
|
|
||
| const std::uint64_t file_size = file_handle.size(); | ||
| const auto file_size_opt = file_handle.size(); | ||
| if (!file_size_opt) { | ||
| return Status::failure("Cannot determine size of: " + path.string()); | ||
| } | ||
| const std::uint64_t file_size = *file_size_opt; | ||
|
|
||
| // Fail to read if the file is bigger than the configured limit. | ||
| auto status = checkFileReadLimit(file_size, path, shouldLog); | ||
| if (!status.ok()) { | ||
| return status; | ||
| } | ||
|
|
||
| const bool isSpecialFile = file_handle.isSpecialFile(); | ||
|
|
||
| /* If the file is a regular file on disk and has no data, | ||
| do not attempt to read */ | ||
| if (!isSpecialFile && file_size == 0) { | ||
| return Status::success(); | ||
| } | ||
|
|
||
| ssize_t res = 0; | ||
| std::size_t total_bytes = 0; | ||
| char buffer[kBlockSize]; | ||
|
|
||
| do { | ||
| res = file_handle.read(buffer, kBlockSize); | ||
|
|
||
| // EOF | ||
| if (res == 0) { | ||
| break; | ||
| } | ||
|
|
||
| if (res > 0) { | ||
| total_bytes += res; | ||
| status = checkFileReadLimit(total_bytes, path, shouldLog); | ||
|
|
||
| if (!status.ok()) { | ||
| return status; | ||
| } | ||
|
|
||
| predicate({buffer, static_cast<std::size_t>(res)}); | ||
| } | ||
| } while (res > 0 || (!isSpecialFile && file_handle.hasPendingIo())); | ||
| } while (res > 0 || file_handle.hasPendingIo()); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Suppressed comments (11)
specs/gpu_metrics.table:5
- The table schema is missing a gpu_index column, but the integration test and implementations use/expect a stable per-row GPU index. Add gpu_index to the base schema so it is always present across platforms.
schema([
Column("vendor_name", TEXT, "The vendor name of the GPU."),
Column("device_name", TEXT, "The model/device name of the GPU."),
osquery/tables/system/windows/gpu_metrics.cpp:152
- Windows implementation does not populate the gpu_index column (required by the spec/test). Populate it from the enumeration index before filling other fields.
for (const auto& item : wmiReq->results()) {
Row r;
osquery/tables/system/linux/gpu_metrics.cpp:211
- Linux implementation needs a stable gpu_index column for each returned GPU row. Introduce a counter outside the enumeration loop so each emitted Row can include gpu_index.
struct udev_list_entry *device_entries, *entry;
device_entries = udev_enumerate_get_list_entry(enumerate.get());
udev_list_entry_foreach(entry, device_entries) {
osquery/tables/system/linux/gpu_metrics.cpp:228
- Populate the gpu_index column for each Linux gpu_metrics row (required by the spec/test).
Row r;
r["pci_bus"] = UdevEventPublisher::getValue(device.get(), kGpuPCIKeySlot);
osquery/tables/system/linux/gpu_metrics.cpp:313
- Increment gpu_index after emitting each gpu_metrics row so subsequent rows get unique indices.
results.emplace_back(std::move(r));
}
osquery/tables/system/darwin/gpu_metrics.mm:529
- macOS implementation does not populate the gpu_index column (required by the spec/test). Populate it from the loop index before filling other fields.
for (NSDictionary* item in items) {
Row r;
osquery/filesystem/filesystem.cpp:176
- Same issue as the predicate overload: without a regular-file check, hashing/readFile can consume FIFO contents (destructive) because FIFOs report size 0 but are still readable. Refuse non-regular files up front.
const auto file_size_opt = file_handle.size();
if (!file_size_opt) {
return Status::failure("Cannot determine size of: " + path.string());
}
const std::uint64_t file_size = *file_size_opt;
osquery/filesystem/filesystem.cpp:125
- This readFile() path is used by hashing (hashMultiFromFile) and still allows reading from FIFOs/special files on POSIX (size() for FIFOs returns 0, so hashing proceeds and consumes FIFO contents; see #8939). Add an explicit regular-file check before opening/reading so we refuse to read non-regular files.
const auto file_size_opt = file_handle.size();
if (!file_size_opt) {
return Status::failure("Cannot determine size of: " + path.string());
}
const std::uint64_t file_size = *file_size_opt;
specs/gpu_metrics.table:2
- The PR title/description focuses on file hashing and PlatformFile, but this change set also introduces a new gpu_metrics table with new platform implementations and integration tests. Consider splitting this into a separate PR or updating the PR description to reflect the added table surface area.
table_name("gpu_metrics")
description("Information about GPU devices on the system, including inventory and runtime telemetry where supported.")
osquery/tables/system/darwin/gpu_metrics.mm:216
- On macOS, collectGPUPowerData() sleeps for 0.5s on every gpu_metrics query (usleep(500ms)), which adds a noticeable fixed latency to table execution. Consider reducing the sampling interval or making it configurable to avoid slowing down normal queries.
constexpr useconds_t kSampleIntervalUs = 500U * 1000U;
constexpr double kSampleIntervalSeconds = 0.5;
osquery/tables/system/darwin/gpu_metrics.mm:316
- This table logs a lot of IOReport channel-level information at LOG(INFO) on every query, which can be very noisy in production. Prefer VLOG() for diagnostic detail, and keep INFO-level logs to a minimum.
LOG(INFO) << "IOReport(all) GPU power candidate group='" << group_string
<< "' name='" << name_string << "' value=" << value
<< " unit='" << unit_string << "' watts=" << channel_power_w;
++logged;
}
| Column("power_limit_watts", DOUBLE, "Configured GPU power limit in Watts."), | ||
| Column("fan_speed_pct", DOUBLE, "GPU fan speed as a percentage of maximum (0-100)."), | ||
| ]) | ||
| implementation("gpu_metrics@genGpuMetrics") |
Comment on lines
215
to
+219
| if (!stat_success) { | ||
| // Final fallback with defaults | ||
| LOG(WARNING) << "Could not stat file: " << f.string() | ||
| << " to preserve metadata in archive"; | ||
| archive_entry_set_size(entry, pFile.size()); | ||
| const auto fallback_size = pFile.size().value_or(0); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix file_size on Windows to support larger than 4GB files.
Remove the isSpecialFile method that is only used in one place, and incorrectly (as pointed out in #8939). Other places in the codebase already use std::is_regular_file.