Visitar URL original
Replace ProcessInfoLayoutRenderer with ProcessStartLayoutRenderer by snakefoot · Pull Request #6224 · NLog/NLog · GitHub
Skip to content

Replace ProcessInfoLayoutRenderer with ProcessStartLayoutRenderer - #6224

Closed
snakefoot wants to merge 1 commit into
NLog:devfrom
snakefoot:ProcessInfoTrim
Closed

snakefoot wants to merge 1 commit into
NLog:devfrom
snakefoot:ProcessInfoTrim

Conversation

@snakefoot

@snakefoot snakefoot commented Jul 6, 2026 •

Copy link
Copy Markdown
Contributor

Replaced by #6231

NET11 reduces the AOT-filesize when not using exotic features from the Process-object.

Changed ${processinfo} to only support Id + Name, and everything else are either hardcoded values or similar values.

Instead introduced ${processstart} with the following options:

  • Format
  • UniversalTime
  • Culture

${processstart} outputs the same value during the entire AppDomain-lifetime, which allows NLog to convert into literal for optimal performance.

And there is also the existing ${processid} + ${processname} + ${processtime} + ${gc}. And have now introduced support for ${gc:workingset}

@snakefoot snakefoot added this to the 6.2 milestone Jul 6, 2026
@snakefoot snakefoot added enhancement Improvement on existing feature breaking behavior change Same API, different result labels Jul 6, 2026
@coderabbitai

coderabbitai Bot commented Jul 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 82b289bc-2c0d-4936-bcb5-ff20be1171f6

📥 Commits

Reviewing files that changed from the base of the PR and between c606c08 and 7975462.

📒 Files selected for processing (6)
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessInfoLayoutRendererTests.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessInfoLayoutRendererTests.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs

Walkthrough

ProcessInfoLayoutRenderer and ProcessInfoProperty are marked obsolete, ProcessInfoLayoutRenderer now uses IAppEnvironment instead of reflective process access, and a new processstart layout renderer is added and registered.

Changes

Process layout renderer changes

Layer / File(s) Summary
ProcessInfoProperty updates
src/NLog/LayoutRenderers/ProcessInfoProperty.cs
Enum is marked [Obsolete], StartTime is documented as local time, and StartTimeUtc is added.
ProcessInfoLayoutRenderer rework
src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs, tests/NLog.UnitTests/LayoutRenderers/ProcessInfoLayoutRendererTests.cs
Renderer is marked [Obsolete], switches from reflective Process access to IAppEnvironment, rewires construction, renders properties through a switch, and is covered by a test that enumerates the enum values.
ProcessStartLayoutRenderer addition
src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs, src/NLog/Config/AssemblyExtensionTypes.cs, tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
A new ProcessStartLayoutRenderer renders ZeroDate, is registered as processstart, and is validated by a unit test.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Logger
  participant ProcessInfoLayoutRenderer
  participant IAppEnvironment

  Logger->>ProcessInfoLayoutRenderer: Append(logEvent)
  ProcessInfoLayoutRenderer->>ProcessInfoLayoutRenderer: switch(Property)
  ProcessInfoLayoutRenderer->>IAppEnvironment: read process values
  ProcessInfoLayoutRenderer-->>Logger: append formatted value
Loading
sequenceDiagram
  participant LayoutConfiguration
  participant ProcessStartLayoutRenderer
  participant LogEventInfo

  LayoutConfiguration->>ProcessStartLayoutRenderer: create ${processstart}
  ProcessStartLayoutRenderer->>LogEventInfo: read ZeroDate
  ProcessStartLayoutRenderer->>ProcessStartLayoutRenderer: format value
  ProcessStartLayoutRenderer-->>LayoutConfiguration: append timestamp
Loading

Poem

A rabbit hops where old bits fade,
And process ticks in cleaner shade.
${processstart} now leaps out bright,
While obsolete hops out of sight. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the primary change: replacing ProcessInfoLayoutRenderer with ProcessStartLayoutRenderer.
Description check ✅ Passed The description describes the renderer replacement, the new processstart options, and related processinfo changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs (1)

44-53: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low value

Obsolete guidance is inconsistent between doc and attribute.

The XML summary (Line 47) lists ${processtime} as an alternative, but the [Obsolete] attribute (Line 53) omits it. Align both messages. The ${processstarttime} reference is also flagged in ProcessInfoProperty.cs Line 41.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs` around lines 44 - 53,
The obsolete guidance for ProcessInfoLayoutRenderer is inconsistent between the
XML summary and the [Obsolete] attribute. Update the ProcessInfoLayoutRenderer
documentation and attribute message so they list the same alternatives, and make
sure the Obsolete text matches the summary wording around ${processid},
${processname}, ${processstarttime}, ${processtime}, and ${gc}. Also review the
related ProcessInfoProperty reference so the ${processstarttime} mention is
consistent there as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs`:
- Around line 173-174: The managed memory value in
ProcessInfoLayoutRenderer.ProcessInfoLayoutRenderer is being truncated by the
unchecked `(int)GC.GetTotalMemory(false)` cast, which can overflow for heaps
above 2 GB. Update the logic that computes and formats `currentMemorySize` to
clamp the `GC.GetTotalMemory(false)` result instead of directly casting it to
`int`, so `AppendFormattedValue` receives a safe IntPtr-sized value without
negative overflow.
- Around line 150-164: The 64-bit branches in
ProcessInfoLayoutRenderer.ProcessInfoLayoutRenderer for
ProcessInfoProperty.MaxWorkingSet/NonPagedSystemMemorySize/PagedSystemMemorySize
and the *64 variants are using unchecked((int)long.MaxValue), which truncates to
-1. Update the first group’s 64-bit path to emit int.MaxValue as the sentinel,
and update the PagedSystemMemorySize64/NonPagedSystemMemorySize64 64-bit path to
emit long.MaxValue while keeping the 32-bit path as (long)int.MaxValue.

In `@src/NLog/LayoutRenderers/ProcessInfoProperty.cs`:
- Line 41: The obsolete message on ProcessInfoProperty should point users to the
actual supported alias. Update the Obsolete attribute text in
ProcessInfoProperty so it references ${processstart} instead of
${processstarttime}, keeping the rest of the guidance aligned with the existing
processid, processname, and gc aliases.

In `@src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs`:
- Around line 88-91: `ProcessStartLayoutRenderer.GetValue()` is still returning
the `LogEventInfo.ZeroDate` sentinel, so `${processstart}` never reflects the
actual process launch time. Update the `ProcessStartLayoutRenderer`
implementation to read and return a real process-start timestamp from the
runtime/process API instead of `ZeroDate`, and keep the existing `UniversalTime`
handling by converting that real start time to UTC or local as needed.

---

Duplicate comments:
In `@src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs`:
- Around line 44-53: The obsolete guidance for ProcessInfoLayoutRenderer is
inconsistent between the XML summary and the [Obsolete] attribute. Update the
ProcessInfoLayoutRenderer documentation and attribute message so they list the
same alternatives, and make sure the Obsolete text matches the summary wording
around ${processid}, ${processname}, ${processstarttime}, ${processtime}, and
${gc}. Also review the related ProcessInfoProperty reference so the
${processstarttime} mention is consistent there as well.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9e7648bd-8d35-4eaa-b488-6a3a2253f2d8

📥 Commits

Reviewing files that changed from the base of the PR and between ca25a82 and 326e323.

📒 Files selected for processing (5)
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs`:
- Around line 47-53: Update the obsolete guidance in ProcessInfoLayoutRenderer
so the summary, remarks, and [Obsolete] message point users to ${processstart}
instead of ${processstarttime}; keep the existing references to ${processid},
${processname}, ${processtime}, and ${gc}, and ensure the wording matches the
new renderer name introduced by the PR.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 14e448a3-33bd-48af-b606-eff3feecb267

📥 Commits

Reviewing files that changed from the base of the PR and between 326e323 and defd469.

📒 Files selected for processing (5)
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs

@snakefoot
snakefoot force-pushed the ProcessInfoTrim branch 2 times, most recently from cd741ba to d5322e5 Compare July 6, 2026 19:46
@snakefoot snakefoot changed the title Replacing ProcessInfoLayoutRenderer with ProcessStartLayoutRenderer Replace ProcessInfoLayoutRenderer with ProcessStartLayoutRenderer Jul 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/NLog/LayoutRenderers/GarbageCollectorInfoLayoutRenderer.cs (1)

61-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicates existing invariant-append pattern.

This mirrors the Int64/UInt64 branches in StringBuilderExt.AppendNumericInvariant (src/NLog/Internal/StringBuilderExt.cs:451-485). Consider calling builder.AppendNumericInvariant(value, TypeCode.Int64) (or a similar shared helper) instead of duplicating the framework-conditional cast logic here, to keep the two implementations in sync going forward.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/NLog/LayoutRenderers/GarbageCollectorInfoLayoutRenderer.cs` around lines
61 - 65, The garbage collector info renderer duplicates the same invariant
numeric append logic that already exists in
StringBuilderExt.AppendNumericInvariant. Update
GarbageCollectorInfoLayoutRenderer to use the shared helper for the value
formatting path instead of keeping the framework-conditional cast and
AppendInvariant logic inline, so the Int64/UInt64 handling stays aligned with
the existing implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/NLog/LayoutRenderers/GarbageCollectorInfoLayoutRenderer.cs`:
- Around line 61-65: The garbage collector info renderer duplicates the same
invariant numeric append logic that already exists in
StringBuilderExt.AppendNumericInvariant. Update
GarbageCollectorInfoLayoutRenderer to use the shared helper for the value
formatting path instead of keeping the framework-conditional cast and
AppendInvariant logic inline, so the Int64/UInt64 handling stays aligned with
the existing implementation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6c1cc854-4ec4-4bf6-b258-bcbd9d832a0b

📥 Commits

Reviewing files that changed from the base of the PR and between defd469 and d5322e5.

📒 Files selected for processing (7)
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/GarbageCollectorInfoLayoutRenderer.cs
  • src/NLog/LayoutRenderers/GarbageCollectorProperty.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
✅ Files skipped from review due to trivial changes (2)
  • src/NLog/LayoutRenderers/GarbageCollectorProperty.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/NLog/LayoutRenderers/GarbageCollectorInfoLayoutRenderer.cs`:
- Around line 61-66: The GC info formatter currently only uses invariant
formatting under NETFRAMEWORK, while the non-Framework long path in
GarbageCollectorInfoLayoutRenderer.Append still falls back to culture-sensitive
formatting. Update the Append logic so the long-value path also uses
builder.AppendInvariant(value) (or equivalent invariant handling) instead of
builder.Append(value), keeping the formatting consistent across all targets and
locales.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 43faf0e4-9673-4cde-a44b-99b93b18531d

📥 Commits

Reviewing files that changed from the base of the PR and between d5322e5 and 4409870.

📒 Files selected for processing (7)
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/GarbageCollectorInfoLayoutRenderer.cs
  • src/NLog/LayoutRenderers/GarbageCollectorProperty.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
✅ Files skipped from review due to trivial changes (1)
  • src/NLog/LayoutRenderers/GarbageCollectorProperty.cs
🚧 Files skipped from review as they are similar to previous changes (5)
  • tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs
  • src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs
  • src/NLog/Config/AssemblyExtensionTypes.cs
  • src/NLog/LayoutRenderers/ProcessInfoProperty.cs
  • src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new ${processstart} layout renderer and deprecates the existing ${processinfo} surface to reduce AOT footprint, while also expanding the ${gc} renderer and adding unit tests for the new/updated renderers.

Changes:

  • Added ${processstart} renderer with Format, UniversalTime, and Culture options and registered it for configuration.
  • Marked ${processinfo} and ProcessInfoProperty as obsolete and reworked ${processinfo} to rely on IAppEnvironment + placeholders instead of System.Diagnostics.Process.
  • Extended ${gc} with a WorkingSet property and added/updated unit tests for ${processstart}, ${processinfo}, and ${gc}.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/NLog.UnitTests/LayoutRenderers/ProcessStartLayoutRendererTests.cs Adds coverage for the new ${processstart} renderer formatting/options.
tests/NLog.UnitTests/LayoutRenderers/ProcessInfoLayoutRendererTests.cs Adds coverage for ${processinfo} properties and marks tests obsolete with migration guidance.
tests/NLog.UnitTests/LayoutRenderers/GarbageCollectorInfoLayoutRendererTests.cs Adds coverage for ${gc} properties including the new WorkingSet.
src/NLog/LayoutRenderers/ProcessStartLayoutRenderer.cs Implements new ${processstart} layout renderer + raw value support.
src/NLog/LayoutRenderers/ProcessInfoProperty.cs Marks enum obsolete and adds StartTimeUtc.
src/NLog/LayoutRenderers/ProcessInfoLayoutRenderer.cs Refactors ${processinfo} away from Process to IAppEnvironment + placeholders; marks obsolete.
src/NLog/LayoutRenderers/GarbageCollectorProperty.cs Adds WorkingSet enum value (with XML docs).
src/NLog/LayoutRenderers/GarbageCollectorInfoLayoutRenderer.cs Implements WorkingSet retrieval and adds IRawValue + [ThreadAgnostic].
src/NLog/Config/AssemblyExtensionTypes.cs Registers the new ${processstart} renderer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@snakefoot
snakefoot force-pushed the ProcessInfoTrim branch 4 times, most recently from 66ff4bb to c606c08 Compare July 9, 2026 19:13
@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

@snakefoot snakefoot closed this Jul 10, 2026
@snakefoot

Copy link
Copy Markdown
Contributor Author

Something strange happened, so created #6231

@snakefoot snakefoot removed this from the 6.2 milestone Aug 13, 2026
@snakefoot snakefoot added breaking behavior change Same API, different result and removed enhancement Improvement on existing feature needs documentation on wiki breaking behavior change Same API, different result labels Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking behavior change Same API, different result size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants