Skip to content

Log SDK environment variable messages only when values differ #12918

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
Merged
baronfel merged 5 commits into from
Jan 5, 2026
Merged

Log SDK environment variable messages only when values differ #12918

baronfel merged 5 commits into from
Jan 5, 2026

Conversation

Copy link
Contributor

Copilot AI commented Dec 11, 2025 •
edited
Loading

Context

Structured log viewer was cluttered with repeated non-actionable messages about SDK environment variables (e.g., DOTNET_HOST_PATH). These appeared for every project evaluation, reporting normal SDK behavior without providing value to users.

Changes Made

Modified ProjectInstance.AddSdkResolvedEnvironmentVariable() to only log messages when an SDK attempts to set an environment variable to a different value than what's already set:

  • When values match (common case): No message is logged, eliminating the clutter shown in the original issue
  • When values differ (conflict): A low-importance message is logged showing both the attempted value and the existing value for diagnostic purposes
  • Code refactoring: Extracted duplicate value comparison and logging logic into a helper method LogIfValueDiffers to improve code maintainability

Updated string resources in Strings.resx and all 13 localized .xlf files:

  • SdkEnvironmentVariableAlreadySet: Now includes both attempted and existing values in the message
  • SdkEnvironmentVariableAlreadySetBySdk: Now includes both attempted and existing values in the message
  • Removed SdkEnvironmentVariableSet (no longer logging successful sets)

The underlying functionality is unchanged—environment variables are still tracked and set correctly, and properties remain visible in the structured log viewer.

Testing

  • Built MSBuild successfully
  • Verified messages no longer appear when values are the same (common case)
  • Messages would appear when values differ (preserves diagnostic value for conflicts)
  • Confirmed existing unit tests pass
  • Verified sample projects build successfully
  • Confirmed properties are still visible in structured log viewer (DOTNET_HOST_PATH, etc.)

Notes

This implementation balances two concerns:

  1. Eliminates noise: Repeated messages about the same value being set multiple times no longer clutter logs
  2. Preserves diagnostics: Actual conflicts (different values) are still logged with full context (both values shown)

The messages log at MessageImportance.Low to provide diagnostic information without being intrusive in normal builds. Code quality was improved by extracting duplicate logic into a reusable helper method.

Original prompt

This section details on the original issue you should resolve

<issue_title>Duplicated messages from SDK/environment resolution</issue_title>
<issue_description>Image

There are a ton of these An SDK attempted to set the environment variable "DOTNET_HOST_PATH" messages that aren't particularly helpful, and they're not attributed to a project so they clutter up the top level of the viewer tool.</issue_description>

Comments on the Issue (you are @copilot in this section)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@YuliiaKovalova
Remove duplicated SDK environment variable log messages
20ba4f9 
These log messages were cluttering the structured log viewer with repeated
messages about DOTNET_HOST_PATH and other SDK environment variables. Since
these messages report normal expected behavior and aren't actionable for
users, they have been removed entirely.

Co-authored-by: YuliiaKovalova <[email protected]>
Copilot AI changed the title [WIP] Fix duplicated messages from SDK/environment resolution Remove duplicate SDK environment variable log messages Dec 11, 2025
Copilot AI requested a review from YuliiaKovalova December 11, 2025 08:46
YuliiaKovalova
YuliiaKovalova reviewed Dec 11, 2025
View reviewed changes
@YuliiaKovalova
Remove unused SDK environment variable string resources
743e2d2 
The string resources SdkEnvironmentVariableAlreadySet,
SdkEnvironmentVariableAlreadySetBySdk, and SdkEnvironmentVariableSet
are no longer used after removing the log messages that referenced them.
This removes them from Strings.resx and all localized .xlf files.

Co-authored-by: YuliiaKovalova <[email protected]>
Copilot AI changed the title Remove duplicate SDK environment variable log messages Remove duplicate SDK environment variable log messages and unused string resources Dec 11, 2025
Copilot AI requested a review from YuliiaKovalova December 11, 2025 10:13
@YuliiaKovalova YuliiaKovalova marked this pull request as ready for review December 11, 2025 16:39
Copilot AI review requested due to automatic review settings December 11, 2025 16:39
@baronfel
Copy link
Member

baronfel commented Dec 11, 2025

This removes the logging entirely, right? How do we know if/how the SdkResolvers had any impact on the overall evaluation - does the viewer still show them as Properties in the eval (I'd expect so)?

Copilot AI reviewed Dec 11, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

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 removes non-actionable log messages about SDK environment variable operations that were cluttering structured log viewers. The changes eliminate three LogComment calls from AddSdkResolvedEnvironmentVariable() and clean up the associated unused string resources from all localization files.

  • Removed logging of SDK environment variable setting operations (already set as env var, already set by another SDK, and successful set)
  • Cleaned up three unused string resources from Strings.resx and all 13 localized .xlf files

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Build/Instance/ProjectInstance.cs Removed three LogComment calls from AddSdkResolvedEnvironmentVariable() method
src/Build/Resources/Strings.resx Removed three unused string resource definitions
src/Build/Resources/xlf/Strings.cs.xlf Removed Czech translations for the three unused string resources
src/Build/Resources/xlf/Strings.de.xlf Removed German translations for the three unused string resources
src/Build/Resources/xlf/Strings.es.xlf Removed Spanish translations for the three unused string resources
src/Build/Resources/xlf/Strings.fr.xlf Removed French translations for the three unused string resources
src/Build/Resources/xlf/Strings.it.xlf Removed Italian translations for the three unused string resources
src/Build/Resources/xlf/Strings.ja.xlf Removed Japanese translations for the three unused string resources
src/Build/Resources/xlf/Strings.ko.xlf Removed Korean translations for the three unused string resources
src/Build/Resources/xlf/Strings.pl.xlf Removed Polish translations for the three unused string resources
src/Build/Resources/xlf/Strings.pt-BR.xlf Removed Brazilian Portuguese translations for the three unused string resources
src/Build/Resources/xlf/Strings.ru.xlf Removed Russian translations for the three unused string resources
src/Build/Resources/xlf/Strings.tr.xlf Removed Turkish translations for the three unused string resources
src/Build/Resources/xlf/Strings.zh-Hans.xlf Removed Simplified Chinese translations for the three unused string resources
src/Build/Resources/xlf/Strings.zh-Hant.xlf Removed Traditional Chinese translations for the three unused string resources

The changes are clean and well-executed. The functionality of tracking and setting SDK environment variables remains intact—only the logging has been removed. All string resources have been properly cleaned up from the main resource file and all 13 localization files consistently. No issues were found during the review.

@YuliiaKovalova
Copy link
Member

YuliiaKovalova commented Dec 11, 2025

This removes the logging entirely, right? How do we know if/how the SdkResolvers had any impact on the overall evaluation - does the viewer still show them as Properties in the eval (I'd expect so)?

this pr removes only sdk-resolution related messages, the defined property remains here
{977DDD06-0AFE-4DFF-A20E-88583125FAC6}

rainersigwald
rainersigwald reviewed Dec 18, 2025
View reviewed changes
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

I think I might favor keeping these messages only if the values differ, instead of dropping them entirely.

@YuliiaKovalova
Copy link
Member

YuliiaKovalova commented Dec 29, 2025

I think I might favor keeping these messages only if the values differ, instead of dropping them entirely.

@copilot please take a look

@YuliiaKovalova
Only log SDK environment variable messages when values differ
6571644 
Per feedback from @rainersigwald, changed the logging behavior to only
emit messages when an SDK attempts to set an environment variable to a
different value than what's already set. This reduces noise while still
alerting users to potential conflicts.

Changes:
- Modified AddSdkResolvedEnvironmentVariable to check if values differ
  before logging
- Updated string resources to include both the attempted and existing
  values in the message
- Restored string resources to Strings.resx and all xlf files with
  updated messages that show both values

This addresses the original issue of duplicate messages cluttering logs
while preserving diagnostic information about actual conflicts.

Co-authored-by: YuliiaKovalova <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 29, 2025

I think I might favor keeping these messages only if the values differ, instead of dropping them entirely.

@copilot please take a look

I've updated the implementation in commit 6571644 to only log messages when values differ. The new behavior:

  • No messages when the same value is set multiple times (eliminates the clutter shown in the original issue)
  • Log message when an SDK attempts to set a variable to a different value than what's already set, showing both values for clarity

This preserves diagnostic information about actual conflicts while eliminating noise from normal operations.

Copilot AI changed the title Remove duplicate SDK environment variable log messages and unused string resources Log SDK environment variable messages only when values differ Dec 29, 2025
Copilot AI requested a review from YuliiaKovalova December 29, 2025 09:43
YuliiaKovalova
YuliiaKovalova reviewed Dec 29, 2025
View reviewed changes
@YuliiaKovalova
Refactor duplicate code into helper method
82fe9b2 
Extracted the duplicated value comparison and logging logic from
AddSdkResolvedEnvironmentVariable into a new helper method
LogIfValueDiffers. This eliminates code duplication while maintaining
the same functionality.

Co-authored-by: YuliiaKovalova <[email protected]>
Copilot AI requested a review from YuliiaKovalova December 29, 2025 09:56
YuliiaKovalova
YuliiaKovalova reviewed Dec 29, 2025
View reviewed changes
Copilot AI requested a review from YuliiaKovalova December 29, 2025 10:29
@baronfel baronfel merged commit 9ba8f3e into main Jan 5, 2026
12 checks passed
@baronfel baronfel deleted the copilot/fix-duplicated-sdk-messages branch January 5, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rainersigwald rainersigwald rainersigwald left review comments

Copilot code review Copilot Copilot left review comments

@baronfel baronfel baronfel approved these changes

+1 more reviewer

@YuliiaKovalova YuliiaKovalova YuliiaKovalova approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@YuliiaKovalova YuliiaKovalova

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Duplicated messages from SDK/environment resolution

4 participants

@baronfel @YuliiaKovalova @rainersigwald