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.

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

disable token info in traces. #56780

Merged
merged 2 commits into from
Aug 4, 2021
Merged

disable token info in traces. #56780

merged 2 commits into from
Aug 4, 2021

Conversation

mangod9
Copy link
Member

@mangod9 mangod9 commented Aug 3, 2021

Though token+offset is useful for certain scenarios, they might not be usable for majority of scenarios. So disabling them by default. It was enabled by default in #44013

@mangod9
disable token info in traces.
6b015b5 
Though token+offset is useful for certain scenarios, they might not be usable for majority of scenarios. So disabling them by default.
@mangod9 mangod9 mentioned this pull request Aug 3, 2021
Merged
@danmoseley
Copy link
Member

Incidentally, I wanted to see how I'd set this flag without editing the code. I got to here
https://docs.microsoft.com/en-us/dotnet/api/system.appcontext?view=net-5.0#ForConsumers

and it says it's either registry or <runtime> section of app config. I'm guessing this should say that for .NET Core those don't work, and you should use runtimeconfig.json. It can point to https://docs.microsoft.com/en-us/dotnet/core/run-time-config/

@eerhardt is that right?

@eerhardt
Copy link
Member

eerhardt commented Aug 3, 2021

@eerhardt is that right?

That doc looks like it hasn't been updated for .NET Core/5+. Nothing in the runtime respects app.config files, and I'm pretty sure we don't look in the registry anymore either.

noahfalk
noahfalk approved these changes Aug 3, 2021
View reviewed changes
Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @mangod9 👍 LGTM

@mangod9 mangod9 merged commit 55cea64 into dotnet:main Aug 4, 2021
@mangod9 mangod9 deleted the disabletokens branch August 4, 2021 02:01
@mangod9
Copy link
Member Author

mangod9 commented Aug 4, 2021

Assume we need a separate issue to fix the doc issue?

thaystg added a commit to thaystg/runtime that referenced this pull request Aug 4, 2021
@thaystg
Merge remote-tracking branch 'origin/main' into thays_implement_debug…
f56ef84 
…ger_proxy_attribute

* origin/main:
  disable token info in traces. (dotnet#56780)
  [debugger] Fix debugger.break behavior (dotnet#56788)
  [mono][wasm] Allow setting env variables with '=' characters in the test runner. (dotnet#56802)
  Ecma edit for `conv.ovf.<to type>.un`. (dotnet#56450)
  Mark HandleProcessCorruptedStateExceptionsAttribute as obsolete (dotnet#56664)
  Enable SxS install of previews on Mac OS (dotnet#56797)
  CoreCLR runtime tests + Mono on the x64 iOS simulator (dotnet#43954)
  [main] Update dependencies from mono/linker (dotnet#56593)
  STJ: Fix deserialization of UInt16 properties (dotnet#56793)
@noahfalk
Copy link
Member

noahfalk commented Aug 5, 2021

Assume we need a separate issue to fix the doc issue?

Yeah, here is a random example of another issue tracking adding docs if it helps. dotnet/diagnostics#1347

@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danmoseley danmoseley danmoseley approved these changes

@noahfalk noahfalk noahfalk approved these changes

Assignees
No one assigned
Labels
area-ExceptionHandling-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mangod9 @danmoseley @eerhardt @noahfalk