-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Check DOTNET_HOST_* for host (COREHOST_*) tracing environment variables #117894
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
Conversation
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov |
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.
Pull Request Overview
This PR updates the host tracing environment variables from the legacy COREHOST_*
prefix to the newer DOTNET_HOST_*
prefix for better consistency with other .NET environment variables. The changes maintain backward compatibility by checking both prefixes with DOTNET_HOST_*
taking precedence.
Key changes:
- Implements fallback logic to check both
DOTNET_HOST_*
andCOREHOST_*
environment variables - Updates error messages and documentation to reference the new variable names
- Updates test constants and adds backward compatibility test coverage
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/native/corehost/hostpolicy/hostpolicy_context.cpp |
Updates error message to reference DOTNET_HOST_TRACE instead of COREHOST_TRACE |
src/native/corehost/hostmisc/trace.cpp |
Implements fallback logic for environment variable checking and updates comments |
src/installer/tests/TestUtils/Constants.cs |
Updates test constants to use new DOTNET_HOST_* variable names |
src/installer/tests/HostActivation.Tests/Tracing.cs |
Refactors tests to be simpler and adds backward compatibility test |
/ba-g timeouts - for the one leg that had host tests, it timed out waiting for the Ubuntu run - the console logs show the tests all passed. |
@am11 had a good suggestion of using the
DOTNET_
prefix forCOREHOST_
environment variables as a move toward greater consistency for .NET environment variables: #117797 (comment)This updates the host to also check
DOTNET_HOST_*
for existingCOREHOST_*
variables used for tracing.cc @dotnet/appmodel @AaronRobinsonMSFT @richlander