GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Use os_log APIs to log PowerShell events on OSX by dantraMSFT · Pull Request #5310 · PowerShell/PowerShell

Conversation

@dantraMSFT

@dantraMSFT Dan Travison (dantraMSFT) commented Nov 2, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This change replaces calls to syslog with os_log on OSX. syslog calls on 10.12 was not producing log output and the functions do not report errors.

See #3766.

NOTE: PR #5144 depends on this PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@dantraMSFT
Dan Travison (dantraMSFT) requested review from James Truher (JamesWTruher), Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) November 2, 2017 18:04
This was referenced Nov 2, 2017
Create tests for os_log usage in nativesyslog.cpp #5311
Closed
Redirect ETW logging to Syslog on Linux #5144
Merged
dantraMSFT @dantraMSFT
Use native os_log APIs on OSX
411c521
daxian-dbw
Dongbo Wang (daxian-dbw) reviewed Nov 2, 2017
View reviewed changes
Comment thread src/libpsl-native/src/nativesyslog.cpp
break;

case LOG_DEBUG:
os_log_debug(_log, MESSAGE_FORMAT, message);

@daxian-dbw Dongbo Wang (daxian-dbw) Nov 2, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

For those different log types, do we need to check whether the type is enabled before calling the log method? What would happen in case the target type is not enabled?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@dantraMSFT Dan Travison (dantraMSFT) Nov 2, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The check occurs in SysLogProvider.cs. The filters are passed into its constructor by PSSysLogProvider. Both Linux and Mac use the PowerShellProperties.json to define the filters (see JsonConfigFileAccessor in PropertyAccessor.cs)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Nov 2, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't see the API os_log_type_enabled exposed in this .cpp file, how does SysLogProvider.cs check if a log type is enabled? Does it enable all log types when creating the log object when calling os_log_create?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@dantraMSFT Dan Travison (dantraMSFT) Nov 3, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It's not needed. Operational events are logged at the default level; which is enabled by default so we'll see the same output we would see in the event log.

https://developer.apple.com/documentation/os/os_log

I'd prefer to filter without interop calls at the risk that someone will change the configuration for powershell at the native layer and we make unneeded interop calls. We can address this later, if it becomes an issue.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Nov 3, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for the clarification. #Closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@daxian-dbw Dongbo Wang (daxian-dbw) assigned Aditya Patwardhan (adityapatwardhan) Nov 2, 2017
@dantraMSFT

Dan Travison (dantraMSFT) commented Nov 3, 2017

Copy link
Copy Markdown
Contributor Author

Aditya Patwardhan (@adityapatwardhan) Would you mind taking a look at this? My syslog PR is blocked until this gets into main and the nuget package is published.

Thanks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Nov 3, 2017
View reviewed changes
@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit ed499ce into PowerShell:master Nov 3, 2017
@dantraMSFT
Dan Travison (dantraMSFT) deleted the libpsl-native/syslog branch November 3, 2017 18:12
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@dantraMSFT @daxian-dbw
Use native os_log APIs on OSX for PowerShell logging (PowerShell#5310)
2dd7780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@adityapatwardhan Aditya Patwardhan (adityapatwardhan)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@dantraMSFT @daxian-dbw @adityapatwardhan
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.