X Tutup
Skip to content

Redirect ETW logging to Syslog on Linux #5144

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

Merged
daxian-dbw merged 21 commits into from
Nov 6, 2017
Merged

Redirect ETW logging to Syslog on Linux #5144

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
479559d
script to generate resx file and event id resource lookup class from …
dantraMSFT Oct 13, 2017
ebfbdcf
Include the name of the manifest file in the resx and C# header comment
dantraMSFT Oct 13, 2017
536eb44
Initial version of the ETW manifest
dantraMSFT Oct 13, 2017
36c25c8
Initial draft of LogProvider for SysLog
dantraMSFT Oct 13, 2017
4e23d84
Minor updates to code gen and exclude gen\EventResource.cs from build
dantraMSFT Oct 14, 2017
e8f138e
logging to syslog enabled - Remove duplicate PSEtwlog class and use …
dantraMSFT Oct 14, 2017
aff9e61
Implement level, channel, and keyword filtering
dantraMSFT Oct 15, 2017
b529cba
Enable PowerShellProperties.json support for linux logging
dantraMSFT Oct 16, 2017
ffb7a88
Fix syslog ident paramter - was using hard-coded string instead of co…
dantraMSFT Oct 17, 2017
31f20e4
Fix build break
dantraMSFT Oct 17, 2017
3db6101
Fix build break
dantraMSFT Oct 17, 2017
1dfb014
address PR feedback
dantraMSFT Oct 20, 2017
731c5e1
Fix stale property reference
dantraMSFT Oct 20, 2017
c8372e8
PR feedback
dantraMSFT Oct 24, 2017
9e4465a
Generate resx with ASCII encoding.
dantraMSFT Oct 24, 2017
c875f8d
PR feedback.
dantraMSFT Oct 25, 2017
99cf86d
Use the right OrdinalIgnoreCase instance.
dantraMSFT Oct 25, 2017
89782f3
Update comments for GitCommitId and use explicit private for MissingE…
dantraMSFT Oct 26, 2017
e20ca59
Regenerate to include explicit private scoping of MissingEventIdResou…
dantraMSFT Oct 26, 2017
e7901fe
Remove inadverntant change.
dantraMSFT Oct 26, 2017
4e04118
Remove GitCommitId hash code logic and log the id raw with each log e…
dantraMSFT Oct 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
PR feedback.
  • Loading branch information
@dantraMSFT
dantraMSFT committed Nov 3, 2017
commit c875f8dfcd804e0a2d3ee49ed483fbe641d581db
37 changes: 16 additions & 21 deletions src/System.Management.Automation/engine/PropertyAccessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,19 +363,16 @@ internal override void SetDefaultSourcePath(string defaultPath)
/// <returns>
/// The string identity to use for writing to syslog.
/// <para>
/// The default value is 'powershell'
/// The default value is 'powershell'.
/// </para>
/// </returns>
internal override string GetSysLogIdentity()
{
string fileName = Path.Combine(psHomeConfigDirectory, configFileName);
Copy link
Contributor

@PaulHigin PaulHigin Oct 19, 2017

Choose a reason for hiding this comment

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

It looks like this same path creation is done repeatedly in other methods so it can be done just once in the constructor and assigned to a readonly field. Also psHomeConfigDirectory should be readonly since it is set just once in the constructor.

Copy link
Contributor Author

@dantraMSFT dantraMSFT Oct 19, 2017

Choose a reason for hiding this comment

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

The entire class uses this pattern and; while i was tempted to change it; I think it's beyond the scope of this PR. I will mark the two fields as read-only, though.

Copy link
Contributor

@PaulHigin PaulHigin Oct 20, 2017

Choose a reason for hiding this comment

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

Ok.

Copy link
Contributor Author

@dantraMSFT dantraMSFT Oct 20, 2017

Choose a reason for hiding this comment

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

I created issue #5179 to track this.

string identity = ReadValueFromFile<string>(fileName, "LogIdentity");
Copy link
Member

@daxian-dbw daxian-dbw Oct 23, 2017

Choose a reason for hiding this comment

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

Just curious, why don't we want the identity name to always be "powershell" or "pwsh"?

Copy link
Contributor Author

@dantraMSFT dantraMSFT Oct 23, 2017

Choose a reason for hiding this comment

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

Overriding the name allows side by side instances to be distinguished from each other in the log.

Copy link
Member

@daxian-dbw daxian-dbw Oct 24, 2017

Choose a reason for hiding this comment

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

Make sense. Thanks!

if
(
string.IsNullOrEmpty(identity)
||
StringComparer.OrdinalIgnoreCase.Compare(LogDefaultValueName, identity) == 0
)

if (string.IsNullOrEmpty(identity) ||
identity.Equals(LogDefaultValue, StringComparer.OrdinalIgnoreCase))
{
identity = "powershell";
Copy link
Member

@daxian-dbw daxian-dbw Oct 23, 2017

Choose a reason for hiding this comment

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

Shall we use pwsh? We've changed the name.

Copy link
Collaborator

@rkeithhill rkeithhill Oct 23, 2017

Choose a reason for hiding this comment

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

You just changed the name of the binary. The product identity seems like it should be powershell_core IMO.

Copy link
Member

@daxian-dbw daxian-dbw Oct 23, 2017

Choose a reason for hiding this comment

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

That makes sense.

Copy link
Contributor Author

@dantraMSFT dantraMSFT Oct 23, 2017

Choose a reason for hiding this comment

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

When discussing this early on, the agreement was to stay with powershell. Appending _core doesn't add any value on non-windows systems and, on windows, the provider is identified via it's provider guid.

Copy link
Member

@daxian-dbw daxian-dbw Oct 24, 2017

Choose a reason for hiding this comment

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

But say if I capture a lot of events on a windows system, will it be easy for me to differentiate the powershell core ones from windows powershell ones?

Copy link
Member

@daxian-dbw daxian-dbw Oct 24, 2017

Choose a reason for hiding this comment

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

Never mind, it's for non-windows logging, so it won't be a problem :)

Copy link
Contributor Author

@dantraMSFT dantraMSFT Oct 24, 2017

Choose a reason for hiding this comment

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

The next PR I submit will clarify that. The provider guid is changed and the location in the event viewer also changes.

}
Expand All @@ -387,20 +384,18 @@ internal override string GetSysLogIdentity()
/// </summary>
/// <returns>One of the PSLevel values indicating the level to log.
/// <para>
/// The default value is PSLevel.Informational
/// The default value is PSLevel.Informational.
/// </para>
/// </returns>
internal override PSLevel GetLogLevel()
{
string fileName = Path.Combine(psHomeConfigDirectory, configFileName);
string levelName = ReadValueFromFile<string>(fileName, "LogLevel");
PSLevel level;
if
(
StringComparer.OrdinalIgnoreCase.Compare(LogDefaultValueName, levelName) == 0
||
!Enum.TryParse<PSLevel>(levelName, true, out level )
)

if (string.IsNullOrEmpty(levelName) ||
levelName.Equals(LogDefaultValue, StringComparer.OrdinalIgnoreCase) ||
!Enum.TryParse<PSLevel>(levelName, true, out level))
{
level = PSLevel.Informational;
}
Expand All @@ -415,17 +410,17 @@ internal override PSLevel GetLogLevel()
/// <summary>
/// Provides a string name to indicate the default for a configuration setting.
/// </summary>
const string LogDefaultValueName = "default";
const string LogDefaultValue = "default";

const PSChannel DefaultChannels = PSChannel.Operational;

/// <summary>
/// Gets the bitmask of the PSChannel values to log.
/// </summary>
/// <returns>
/// A bitmask of PSChannel.Operational and/or PSChannel.Analytic
/// A bitmask of PSChannel.Operational and/or PSChannel.Analytic.
/// <para>
/// The default value is PSChannel.Operational
/// The default value is PSChannel.Operational.
/// </para>
/// </returns>
internal override PSChannel GetLogChannels()
Expand All @@ -440,7 +435,7 @@ internal override PSChannel GetLogChannels()

foreach (string name in names)
{
if (StringComparer.OrdinalIgnoreCase.Compare(LogDefaultValueName, name) == 0)
if (name.Equals(LogDefaultValue, StringComparer.OrdinalIgnoreCase.Compare))
{
result = 0;
break;
Expand All @@ -462,7 +457,7 @@ internal override PSChannel GetLogChannels()
return result;
}

// by default, do not include analytic events
// by default, do not include analytic events.
const PSKeyword DefaultKeywords = (PSKeyword) (0xFFFFFFFFFFFFFFFF & ~(ulong)PSKeyword.UseAlwaysAnalytic);

/// <summary>
Expand All @@ -471,7 +466,7 @@ internal override PSChannel GetLogChannels()
/// <returns>
/// A bitmask of PSKeyword values.
/// <para>
/// The default value is all keywords other than UseAlwaysAnalytic
/// The default value is all keywords other than UseAlwaysAnalytic.
/// </para>
/// </returns>
internal override PSKeyword GetLogKeywords()
Expand All @@ -486,7 +481,7 @@ internal override PSKeyword GetLogKeywords()

foreach (string name in names)
{
if (StringComparer.OrdinalIgnoreCase.Compare(LogDefaultValueName, name) == 0)
if (name.Equals(LogDefaultValue, StringComparer.OrdinalIgnoreCase))
{
result = 0;
break;
Expand Down
24 changes: 12 additions & 12 deletions src/System.Management.Automation/logging/LogProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ private static class Strings
internal static readonly string LogContextShellId = EtwLoggingStrings.LogContextShellId;
}

/// <summary>
/// Gets PSLogUserData from execution context
/// <summary>
/// Gets PSLogUserData from execution context.
/// </summary>
/// <param name="context"></param>
/// <returns></returns>
Expand All @@ -170,10 +170,10 @@ protected static string GetPSLogUserData(ExecutionContext context)
}

/// <summary>
/// Appends exception information
/// Appends exception information.
/// </summary>
/// <param name="sb">string builder</param>
/// <param name="except">exception</param>
/// <param name="sb">string builder.</param>
/// <param name="except">exception.</param>
protected static void AppendException(StringBuilder sb, Exception except)
{
sb.AppendLine(StringUtil.Format(EtwLoggingStrings.ErrorRecordMessage, except.Message));
Expand All @@ -199,10 +199,10 @@ protected static void AppendException(StringBuilder sb, Exception except)
}

/// <summary>
/// Appends additional information
/// Appends additional information.
/// </summary>
/// <param name="sb">string builder</param>
/// <param name="additionalInfo">additional information</param>
/// <param name="sb">string builder.</param>
/// <param name="additionalInfo">additional information.</param>
protected static void AppendAdditionalInfo(StringBuilder sb, Dictionary<String, String> additionalInfo)
{
if (additionalInfo != null)
Expand All @@ -215,10 +215,10 @@ protected static void AppendAdditionalInfo(StringBuilder sb, Dictionary<String,
}

/// <summary>
/// Gets PSLevel from severity
/// Gets PSLevel from severity.
/// </summary>
/// <param name="severity">error severity</param>
/// <returns>PS log level</returns>
/// <param name="severity">error severity.</param>
/// <returns>PS log level.</returns>
protected static PSLevel GetPSLevelFromSeverity(string severity)
{
switch (severity)
Expand All @@ -236,7 +236,7 @@ protected static PSLevel GetPSLevelFromSeverity(string severity)
// Estimate an approximate size to use for the StringBuilder in LogContextToString
// Estimated length of all Strings.* values
// Rough estimate of values
// max path for
// max path for Command path
const int LogContextInitialSize = 30 * 16 + 13 * 20 + 255;

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions src/System.Management.Automation/utils/tracing/PSSysLogProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace System.Management.Automation.Tracing
{
/// <summary>
/// SysLog LogProvider implementation
/// SysLog LogProvider implementation.
/// </summary>
internal class PSSysLogProvider : LogProvider
{
Expand All @@ -20,7 +20,7 @@ internal class PSSysLogProvider : LogProvider
internal const PSKeyword DefaultKeywords = (PSKeyword) (0xFFFFFFFFFFFFFFFF & ~(ulong)PSKeyword.UseAlwaysAnalytic);
Copy link
Contributor

@PaulHigin PaulHigin Oct 19, 2017

Choose a reason for hiding this comment

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

This is also defined in PropertyAccessor.cs. We should only define it once.

Copy link
Contributor Author

@dantraMSFT dantraMSFT Oct 19, 2017

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor Author

@dantraMSFT dantraMSFT Oct 20, 2017

Choose a reason for hiding this comment

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

Fixed - now only defined in PropertyAccessor


/// <summary>
/// Class constructor
/// Class constructor.
/// </summary>
static PSSysLogProvider()
{
Expand All @@ -46,7 +46,7 @@ private static StringBuilder PayloadBuilder
{
if (_payloadBuilder == null)
{
// First time initialization for this thread.
// NOTE: Thread static fields must be explicitly initialized for each thread.
_payloadBuilder = new StringBuilder(200);
}
return _payloadBuilder;
Expand Down
Loading
X Tutup