[Unified Recorder] Features

[HarshaNalluru]

Background

Bare minimum prototype with storage blob client (underlying core-http) #15826 node test.

Beyond the investigations and issues encountered so far in building a primitive prototype, a lot more needs to be done, following is the list of features that are required before we can attempt migrating the SDKs.

Features / Bugs to fix

• [Unified Recorder] Rebrand recorder-new as @azure-tools/test-recorder 2.0.0 #19743
• [Recorder] Add custom matcher support #20401
• Sanitizers
  • ContinuationSanitizer [Unified Recorder] Fix ContinuationSanitizer #18220
    • Bug
  • OAuthResponseSanitizer [Unified Recorder] Testing OAuthResponseSanitizer  #18221
    • Testing
  • ResetSanitizer [Unified Recorder] Investigate ResetSanitizer #18222
    • Bug
• Add a check for the sanitizers to not be applied when the variable is undefined
• Add logging env var to the docker command
• Recorder(and proxy tool) should allow tweaking the request bodies or paths(customizing outgoing requests) during playback
  • uuids can be randomly generated within the SDK which makes the requests dynamic in playback, so customizing outgoing requests should be supported
• [Unified Recorder] Public API - document it from the perspective of the user #19504
• [Unified Recorder] Access tokens need to be masked automatically before saving the recordings #15954
• [Unified Recorder] Should work with the AAD credential #18782
• @witemple-msft idea of instrumenting mocha so that we don't have to do any of this beforeEach afterEach stuff to set up the recorder at all.
  • What if we could just do something like mocha -r @azure-tools/mocha-recorder and it would "just work" by attaching a recorder to every test context?
• Add transforms provided by the proxy tool [Unified Recorder] Add transforms provided by the proxy tool #18225
• Followups of [Unified Recorder] Call proxy-tool through dev-tool #18322
  • Allow providing docker run options along with the mocha and karma options for the test commands
  • Call mocha programmatically
    • experiment with using dev-tool's own register instead of ts-node/register. That way we can really centralize the concerns in our own config/scripts.
  • Call karma programmatically
  • @witemple-msft's comment
    • All of this is nitpicky, but it's best to be explicit about the mutability of the volume. If we are running in playback mode, we should bind the volume read-only. In record mode, we should bind it read-write. On Linux hosts that have SELInux (Red Hat, Fedora), we'll also need the Z flag to relabel the volume. We can test for SELinux using the selinuxenabled command, and that'll exit 0 if it's enabled on the machine:
  • @scbedd raise a couple of interesting points
    • [Unified Recorder] Only run tests after the proxy tool is active #18712
    • [Unified Recorder] Docker container doesn't stop gracefully on windows #19363
      • Assign a name to the container image and stop the container using the name similar to what the powershell script does.
• Warn users to add replacements/transforms if we detect any secrets being leaked. - (Running credscan on the recordings before generating the reports)
• [Recorder] Add logging - with azure logger #21371
• Scope sanitization to be allowed to remove false positives from cred scan
• I(Will) would still like to see a solution where I can import and use one thing to use DefaultAzureCredential in record/live mode and NoOpCredential in playback mode. Maybe we can put that helper in the test-utils package. I(Harsha) was thinking that, but that means, test-utils can't be a dependency of recorder or recorder-new packages.
• Drafting an idea(suggested by Will) - to extend mocha with recorder describeWithRecorder attempting - doesn't work yet · 133c215 (github.com) - to avoid calling start and stop for each test suite
• @witemple-msft's comments at [Unified Recorder] Support "variables" in recordings with proxy-tool #18585
  • It's clever and the implementation seems very simple. Since the variables are sent as JSON, I wonder why we can't store more than just strings. For example, I might like to store a number or even a nested object.
  • If you're interested in that idea, we can explore how the consumers of this API can strongly type that.
• [Unified Recorder] Proxy tool should understand parts of the URL and do the replacements accordingly #18241
• Add a session-level sanitizer [Unified Recorder] Option to add session level sanitizer #18223
• [Unified Recorder] Parametrize the host and port #18240
• Run the docker command in detached mode and provide the logs
• Figure out and make sure fiddler and Wireshark works to inspect conversations
• Feedback [Recorder] Unified recorder prototyping with storage-queue/data-tables SDKs #15826 (comment)
• Make new recordings smaller by removing unnecessary properties.
• [Template] Windows and Mac tests are not running in the CI #19141
• [Unified Recorder] Add contentType header for sanitizer requests too #18691
  • [Unified Recorder] Allow saving the randomly generated variable in record mode and give it back in playback. #18580
• Migration guide
  • Show the old and new recorder equivalents
  • Dev-tool extension commands migration
  • Mention the docker prerequisite
  • Update the dev-tool readme with the new commands
• Handle [Recorder] Better handling for ephemeral headers #19500
• [Unified Recorder] dotenv - load it by default from recorder-new #19140
• Is it an error to start twice without calling stop?
• Relationship/interaction between recorder and recorder-new - Proposal
  • Eventually, we'll remove recorder-new and put everything in @azure-tools/test-recorder. (mainly because @azure-tools/test-recorder has been already published). So, the users will only need to import @azure-tools/test-recorder.
  • Move the recorder-new code to the @azure-tools/test-recorder package after [Unified Recorder] Add sample unified recorder tests to template #18817 is merged and we lock the design from the user's perspective.
    • Before this, release the recorder to publish the unreleased changes
• [Unified Recorder] Offer delay from recorder to speed up playback #19560
• [Unified Recorder] Allow passing "undefined" keys to the sanitizers #19559
• [Unified Recorder] Come up with a recorder.configureClient() design similar to the perf framework #19364
• Clean up ts-inputs, js-inputs node test commands - support all the test commands that we currently have
• [Unified Recorder] Work on the No-op AAD credential for playback - to be in sync with .NET and Python #18897
• Add matchers provided by the proxy tool [Unified Recorder] Add matchers provided by the proxy tool #18224
• Logs from proxy-tool in the CI Adding Redirection of nohup-ed tool runs so we can dump the log later azure-sdk-tools#2252 (comment)
• Prototype with a package that uses core-http - [ongoing with storage node test [Recorder] Unified recorder prototyping with storage-queue/data-tables SDKs #15826]
• Fix rollup config problem with the recorder v2 [Recorder] Rollup error seen at recorder-v2 with shared rollup config #15931
• Recorder should support the browser tests as well - meaning, running the proxy tool must be handled.
  • [Unified Recorder] Unable to hit the localhost:5000 from a karma test #16356
• [Unified Recorder] Running the test at #15826 in playback mode is also making a live request - fix the bug #16345
• Not all clients that use core-http expose httpClient as an option, need to figure out a way to support such libraries
• [Unified Recorder] Unify RecordingHttpClient for both node and browser #17014
• Unified recorder for core-v2 packages - node & browser #17013
• [Unified Recorder] Unify core-v1 and core-v2 recorders #17019
• [Unified Recorder] Passing the test path to the proxy tool and save recordings #17384
• [Recorder] Proxy tool should support plain string replacements #15953
• Proxy tool should support regex replacements
• Proxy tool should understand connection string format and do the replacements accordingly
• Allow tweaking the recording for the end user programmatically
• Skipping the tests should be allowed (Choosing not to support this, users will have to rely on this.skip() and the if checks)
• Recordings have a lot of headers that can be dynamic, could be better if they are stripped out or updated with defaults
• Generate an application for each OS and let users install the application instead of the docker.
• [Proxy Tool] Connect with https instead of http  #17070
• Understanding connection strings to do the replacements in recordings
• Testing the unified recorder #17750
  • a separate server to run a fake service and return responses
  • to be able to test the proxy-tool's features
  • should work for both node and browser
  • server is supposed to run in parallel to the tests
  • to be able to start the proxy-tool in parallel too
• Right now, the replacements/transforms only support one entry per request, can we do multiple to be sent at once?
  • Proxy-tool - yet to support this directly
• [Unified Recorder] Allow saving the randomly generated variable in record mode and give it back in playback. #18580
• [Unified Recorder] Support "variables" in recordings with proxy-tool #18585
• [Unified Recorder] Recorder should be able to start and stop the proxy tool  #17042
  • Current prototype installs, runs, and stops the docker container manually, we need a script to make it black box and run when the test script is triggered

Resources

• https://github.com/Azure/azure-sdk-tools/tree/feature/http-recording-server/tools/test-proxy/Azure.Sdk.Tools.TestProxy
• https://github.com/Azure/azure-sdk-tools/tree/feature/http-recording-server/tools/test-proxy/docker#build-and-run
• https://github.com/HarshaNalluru/azure-sdk-for-js/tree/harshan/recorder/unified
• https://github.com/nguerrera/azure-sdk-for-js/cf43c0cb8c7b9430283a0f4b207b5341958aebe8

[scbedd]

Hey @HarshaNalluru ! Thanks for the details!

Couple comments!

Current prototype installs, runs and stops the docker container manually, we need a script to make it black box and run when the test script is triggered

Will have this for ya very soon. Messing about with permissions on our ACR to make it truely anonymous access.

Proxy tool should support regex replacements

We absolutely support this for header/URI quite well. I will finish updating the README with a whole bunch of detail and examples and forward it to ya. Should have that for you later today.

Proxy tool should support plain string replacements

Same as above.

uuids can be randomly generated within the SDK which makes the requests dynamic in playback, so customizing outgoing requests should be supported

Which test can I work against to repro? I've got an early version of this already implemented, but don't want to put you through debugging it against a docker container :P

Access tokens need to be masked automatically before saving the recordings

Should already be there, let's chat about where it's not happening!

After a brief discussion, I'll use the branch feature/unified-recorder to repro issues as soon as it's present on the azure-sdk-for-js repo.

For any questions I have that require additional detail, I'll send you a teams meeting request! Thanks!

[HarshaNalluru]

uuids can be randomly generated within the SDK which makes the requests dynamic in playback, so customizing outgoing requests should be supported
Which test can I work against to repro? I've got an early version of this already implemented, but don't want to put you through debugging it against a docker container :P

Assume a test generates a uuid, the saved request has a different uuid from what will be the request during playback. So, this needs customization of the request in playback with a regex of sorts.

Access tokens need to be masked automatically before saving the recordings
Should already be there, let's chat about where it's not happening!

Sure, I just logged this issue with all the details so they are being traked somewhere, I'll strikeoff one-by-one as I test them. :)

After a brief discussion, I'll use the branch feature/unified-recorder to repro issues as soon as it's present on the azure-sdk-for-js repo.

The PR #15826 isn't merged yet, feature branch can be used once that is merged. Until then, the only way would be to use my branch.

[scbedd]

Output of our discussion on 6/21.

Recorder should pass the test path to the proxy tool to be able to save the recording at the desired location

Sitting on this one given the new startup scripts.

Recorder(and proxy tool) should allow tweaking the request bodies or paths(customizing outgoing requests) during playback

Need to think about this more. The test pr has exact context for example.

Access tokens need to be masked automatically before saving the recordings

This is done.

Additional q not in set above. Need to ensure ContinuationSanitizer works appropriately. I will work through an example. Pass me an exact test I can work through. Here is an example recording

Scope sanitization to be allowed to remove false positives from cred scan

The bodies of the responses have a string that's NOT a secret, but still trips credscan. Here's an example. Check for scope argument.. Write a sanitizer to clean it up.

Recordings have a lot of headers that can be dynamic, could be better if they are stripped out or updated with defaults

I will add a RemoveHeader sanitizer. You call it before your test run.

Allow tweaking the recording for the end user programmatically

Use a transform or a sanitizer.

Filing issues for:

• OS Compiled Executable
• Add RemoveHeader Sanitizer
• ScopeURL request body replacement <-- this fits well into BodyRegexSanitizer implementation.
• MSAL Ignores body matching on specific request only. Not on all request/responses within a recording.

[scbedd]

Hey @HarshaNalluru, got some updates for ya.

General Discussion and Advice

To begin, you don't need to install a specific tag of the docker image anymore. Use the latest tag when accessing the docker image. Additionally,
local builds are perfectly functional now.

When you're running in live mode, before any tests invoke, ensure that you hit up the Admin/AddSanitizer to add appropriate sanitization.

I believe there should be a base set or what-have-you that should be present generically, with the ability to add specific items for your service requirements.

In Python, I'd handle this with a session level fixture, with additional customization provided as part of test configuration per specific service.

For all of the below examples, feel free to hit /Info/Available for additional detail. I also have unit tests that exercise these sanitizers
if you need an example of the actual regex as well. I used your scope replacement for one of them.

Random TableNames, other resources that are randomly generated.

I did give some thought to your original question WRT the whole:

Table name is randomly generated, so we retrieve what the last random name from the recording prior to invoking the test.

I don't really think there is a way to do this with a remote server. At least, not one that would be performant. I think the better way to do this is one of two options

1. During record, add a sanitizer that updates stuff like table name (or whatever else is randomized) to a understood "filler". So table1249 changes to targettable. During playback mode, if you activate that same sanitizer, the incoming request will be sanitized to the same table name value and match the recording.
2. During record, use the same sanitizer as in Initial commit for copying files from azure-sdk-for-node #1, and just directly use those filler values when you're running in playback.
3. (As of yet uncoded), we create a generalized matcher that just ignores certain fields when matching your request.

Of course, I'm open to further feedback and discussion here.

Remove-Header Sanitizer

This one is pretty straightforward, pass along a comma separated list of key or keys for headers that you want gone from the saved recording.

POST
url: <proxyURL>/Admin/AddSanitizer
headers: {
    "x-abstraction-identifier": "BodyRegexSanitizer"
}
body: {
    "headersForRemoval": "Location,<your-other-header>",
}

Spaces are trimmed, so you don't need to worry about whitespace around the commas. You also aren't limited to a single one of these, so you could add multiple Remove-Header that each removes one. It's really up to you.

Body-Key Sanitizer

Use this sanitizer if you want to update a specific member w/ a regex or whole value replace. The document string in /Info/Available does link here, but the documentation for available search through your body is documented here

POST
url: <proxyURL>/Admin/AddSanitizer
headers: {
    "x-abstraction-identifier": "BodyKeySanitizer"
}
body: {
    "value": "<your-new-value>",
    "regex": ".*",
    "jsonPath": "$.TableName"
}

Note that the above example is just a "complete" replacement. You can do a scoped replacement just like I have for the BodyRegex example.

Body Regex Sanitizer

Use this sanitizer if you aren't dealing with a JSON body. It does free-text regex replace. Your scope replacement ask is an example of when you'd use this boyo.

POST
url: <proxyURL>/Admin/AddSanitizer
headers: {
    "x-abstraction-identifier": "BodyRegexSanitizer"
}
body: {
    "value": "<your-new-value>",
    "regex": "scope\\=(?<scope>[^&]*)",
    "groupForReplace": "scope"
}

In Summary

I believe the above should meet the vast majority of the needs you have. I have a ContinuationToken written up, but before I ask you to use that, I'd prefer to eat the bullet on debugging the inevitable issues :)

Additionally, I believe that providing an superceding sanitizer that does URI, Body, and HEAD regex replacement with the same regex may be useful, but that's not present currently. LMK what you think about this one.

I expect more issues will arise as this get's coded into real tests, but I'll tackle those when they occur. Easiest way to get through it is to hit the failure.

[github-actions]

Hi @HarshaNalluru, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.