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.

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

Add property BetterStackLogsTarget.IncludeScopeContext #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

menuet
Copy link

@menuet menuet commented Mar 6, 2024

Hi
Currently, the target BetterStackLogsTarget uploads only the GlobalDiagnosticContext's properties.
It is useful for us to uplaod the ScopeContext's properties too.
So we duplicated and hacked your class in our code base for this purpose.
We made this PR in case you are interested to make this patch available to everyone (and for getting rid of our duplicate if yours does the job).
Regards

…se) in order to be able upload the ScopeContext's properties
@snakefoot
Copy link
Contributor

snakefoot commented Oct 7, 2024

Notice that you inherit from TargetWithContext, which correctly handles ScopeContext. Where you can call

{
   IDictionary<string,object> logProperties = this.GetAllProperties(logEvent);
}

See also: https://github.com/NLog/NLog/wiki/How-to-write-a-custom-target-for-structured-logging

Maybe this will be better:

        protected override void Write(LogEventInfo logEvent)
        {
            var properties = GetAllProperties(logEvent); // Assign IncludeEventProperties = true in constructor

            var contextDictionary = new Dictionary<string, object> {
                ["logger"] = logEvent.LoggerName,
                ["properties"] = properties,
                ["runtime"] = new Dictionary<string, object> {
                    ["class"] = logEvent.CallerClassName,
                    ["member"] = logEvent.CallerMemberName,
                    ["file"] = string.IsNullOrEmpty(logEvent.CallerFilePath) ? null : logEvent.CallerFilePath,
                    ["line"] = string.IsNullOrEmpty(logEvent.CallerFilePath) ? null : logEvent.CallerLineNumber as int?,
                },
            };

scopedProperties[prop.Key] = prop.Value;
}
if (scopedProperties != null)
contextDictionary["sc"] = scopedProperties;
Copy link
Contributor

Choose a reason for hiding this comment

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

Acronyms like "sc" are not easy for others to understand

/// <summary>
/// Include Include ScopeContext's properties in logs.
/// </summary>
public bool IncludeScopeContext { get; set; } = false;
Copy link
Contributor

@snakefoot snakefoot Oct 7, 2024

Choose a reason for hiding this comment

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

NLog TargetWithContext already provides IncludeScopeProperties that works together with GetAllProperties()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants