Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change calls to debug log to be verbose #6369

Merged
merged 1 commit into from
Apr 9, 2020
Merged

Change calls to debug log to be verbose #6369

merged 1 commit into from
Apr 9, 2020

Conversation

Breakwell
Copy link
Contributor

Description

The Windows build of osquery does not support a debug build. Change debug logs in the dispatcher to be verbose.

Testing

Run osquery with the verbose flag:

C:\>osquery.exe --D --allow_unsafe --verbose
I0407 12:54:16.810698 16600 init.cpp:345] osquery initialized [version=4.2.0]
I0407 12:54:16.857560 16600 system.cpp:335] Found stale process for osqueryd (29216)
I0407 12:54:16.857560 16600 system.cpp:367] Writing osqueryd pid (21244) to \Program Files\osquery\osqueryd.pidfile
I0407 12:54:16.873204 16600 extensions.cpp:376] Could not autoload extensions: Failed reading: \Program Files\osquery\extensions.load
I0407 12:54:16.873204 16600 dispatcher.cpp:78] Adding new service: WatcherRunner (0000028172CE6B60) to thread: 17544 (0000028172CF3030) in process 21244

@Breakwell Breakwell force-pushed the remove-dlog-calls branch from 1f503b7 to ba63e0a Compare April 8, 2020 15:09
@Breakwell Breakwell force-pushed the remove-dlog-calls branch from ba63e0a to dce8cf4 Compare April 8, 2020 15:21
@Smjert Smjert added the logging label Apr 8, 2020
Smjert
Smjert previously approved these changes Apr 8, 2020
View reviewed changes
Copy link
Member

@Smjert Smjert left a comment

Choose a reason for hiding this comment

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

@Breakwell thanks!
The change definitely makes sense and it also seems the only usage left of DLOG we have.

@theopolis
Copy link
Member

These logs feel more developer-focused than all other INFO logs. I wonder if we can instead use VLOG(2)?

@Smjert
Copy link
Member

Smjert commented Apr 9, 2020

These logs feel more developer-focused than all other INFO logs. I wonder if we can instead use VLOG(2)?

As far as I've ever seen every verbose log is mostly developer focused; many happens to be meaningful to users when troubleshooting is needed it's true, but again I always saw them as debugging logs.

I'm in favor though of separating them more, but we should maybe try to define better what we consider a developer focused log vs something a user could be interested in when troubleshooting.

@theopolis
Copy link
Member

Do you think we should move forward and land this PR as is @Smjert?

@Smjert
Copy link
Member

Smjert commented Apr 9, 2020 •
edited
Loading

Do you think we should move forward and land this PR as is @Smjert?

I think these are indeed developer focused if we want to define them as something that mainly talks about internals of osquery.
So if we want to introduce this as a concept and then also modify other instances, I would start modifying this too as you say.

Other question is, does --verbose enable only v=1? (EDIT: yes).

So yeah lets do it, maybe lets also open an issue to add a flag to enable debug logs?
I know that we can do GLOG_v=2 osqueryd to enable them, but maybe a flag is eventually more handy?

@Smjert Smjert dismissed their stale review April 9, 2020 01:09

I think theopolis is making a good point about the focus of the logs, which needs further thought

@theopolis theopolis mentioned this pull request Apr 9, 2020
Open
@theopolis
Copy link
Member

I opened #6385 so we can discuss this more completely, perhaps during office hours next week. :D

@theopolis theopolis merged commit fb6b514 into osquery:master Apr 9, 2020
aikuchin pushed a commit to aikuchin/osquery that referenced this pull request Jul 11, 2023
@mogrein
Merge pull request osquery#9 in CLOUD/osquery from sync/osquery-4.3.0…
b7720da 
… to master

* commit '8c13dd6bd206f2909a4baea5bcfbc91d5e3f502b': (159 commits)
  release: updating changelog for 4.3.0 release (osquery#6387)
  Build hvci_status table with CMake (osquery#6378)
  Change calls to debug log to verbose (osquery#6369)
  iokit: Fix race when accessing port_ (osquery#6380)
  Check extensions are registered with osquery core (osquery#6374)
  First steps to remove the Buck build system (osquery#6361)
  Return error detaching table, only use primary database (osquery#6373)
  Copy the parent environment when launching worker
  Change process table log errors to info and fix typo (osquery#6370)
  Ensure the extension uuid is never 0 (osquery#6377)
  Remove errors when converting empty numeric rows (osquery#6371)
  Do not force a specific path to install osquery on Windows (osquery#6379)
  Fix readFile API doing blocking I/O with a non-blocking handle (osquery#6368)
  magic: Check return from magic_file (osquery#6363)
  macos: Use -1 for missing ppid in process_events (osquery#6339)
  Update OpenSSL to version 1.1.1f and fix build (osquery#6359)
  Simplify how third party libraries formula work (osquery#6303)
  Add socket_events table for socket auditing in MacOS (osquery#6028)
  Extend the fields of curl_certificate table (osquery#6176)
  add status column to deb_packages table (osquery#6341)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theopolis theopolis theopolis approved these changes

@Smjert Smjert Smjert left review comments

Assignees
No one assigned
Labels
logging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Breakwell @theopolis @Smjert