Test proxy logs - sanitizer info #9164
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d; update commentsAdded a ToString method to RecordEntry for string representation.Introduced logging in RecordSession to track sanitizer changes.Added SanitizerId to RecordedTestSanitizer for unique identification.Updated RegisteredSanitizer constructor to set SanitizerId.Corrected copyright comment in RequestOrResponse.cs.Added necessary using directives to RecordEntry.cs.
HarshaNalluru
requested review from
scbedd,
mikeharder and
benbp
as code owners
|
I like the idea, but we are adding a ton of workload to every single request. Every time we sanitize we write all the entries out. We have to put this behind a debug flag somehow. I'm not certain how big of an impact there will be, so my thoughts is once you get these tests resolved, I will help you get this upgrade to a branch off of Easiest way to check performance of a proxy upgrade is to simply try updating the target_version.txt and see how all the repos handle it. |
scbedd
requested changes
Oct 15, 2024
There was a problem hiding this comment.
Thank you Harsha!
We spoke offline about fixing tests, figuring out how to put the heavier consistent tostringing behind a flag, and checking against existing libraries.
Push this to an upstream branch at your convenience so we can easily publish the results as proxy versions.
scbedd
reviewed
Oct 15, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 15, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
xirzec
reviewed
Oct 15, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
xirzec
reviewed
Oct 15, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 15, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 15, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
…n.cs Co-authored-by: Scott Beddall <[email protected]>
…n.cs Co-authored-by: Scott Beddall <[email protected]>
scbedd
reviewed
Oct 22, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordEntry.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 23, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RequestOrResponse.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 23, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 23, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordEntry.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 23, 2024
scbedd
reviewed
Oct 23, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 23, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
scbedd
requested changes
Oct 23, 2024
scbedd
reviewed
Oct 23, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordEntry.cs
Outdated
Show resolved
Hide resolved
scbedd
reviewed
Oct 23, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordEntry.cs
Outdated
Show resolved
Hide resolved
HarshaNalluru
commented
Oct 24, 2024
tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/RecordSession.cs
Outdated
Show resolved
Hide resolved
scbedd
requested changes
Oct 25, 2024
There was a problem hiding this comment.
Please address this comment and then we're g2g aside from your ToString() todo.
Feel free to use TryGetContentAsText if you'd prefer that to relying on ToString().
…time of sanitization
HarshaNalluru
added a commit
to Azure/azure-sdk-for-js
that referenced
this pull request
Nov 25, 2024
## What's going on? - Invocation of `test-proxy` in the `child_process` will now support environment variables - injects the current environment variables into the child's environment - Added a new flag `"test-proxy-debug"` to all the test commands, which enables debug logging for the test-proxy - sets the environment variable `Logging__LogLevel__Default` to `"Debug"` - Simply setting the environment variable `Logging__LogLevel__Default` to `"Debug"` will have the same effect as passing `test-proxy-debug` flag to the test commands. Related to Azure/azure-sdk-tools#9164
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test Proxy Logs - Impacted recordings and sanitizers
Introducing improvements to the test proxy logging in terms of which sanitizers are applied and how.
Key changes below:
Main changes
RecordEntry.cs
Clonemethod to create a copy of theRecordEntry, which is used primarily for sanitization logging.RecordSession.cs
RecordEntryand log modifications, enhancing traceability and debugging capabilities.LoggingTests.cs
AssertLogsmethod to include atestTypeparameter, adjusting the logic to handle different test types.PlaybackLogsSanitizedRequestandRecordingHandlerLogsSanitizedRequeststests to use the newAssertLogsmethod.Impact
These changes enhance the transparency and traceability of the sanitizers in test proxy by adding detailed logging for request and response modifications. This will aid in debugging and understanding the effects of sanitizers during test sessions.
Fixes #8474
Example Output
Output 1
Output 2