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.

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

Apply Sanitizers Immediately #3265

Open
scbedd opened this issue May 2, 2022 · 0 comments
Open

Apply Sanitizers Immediately #3265

scbedd opened this issue May 2, 2022 · 0 comments
Assignees
Labels
Central-EngSys This issue is owned by the Engineering System team. Test-Proxy Anything relating to test-proxy requests or issues.

Comments

@scbedd
Copy link
Member

scbedd commented May 2, 2022

@timovv was investigating Azure/azure-sdk-for-js#18222

Essentially, sanitizers apply during record when we stop the recording. During playback, sanitizers are applied to each request prior to matching against the recordings.

There are complicating factors here. We have sanitizers that rely on more than just an individual request/response pair. See ContinuationSanitizer. We should properly apply those sanitizers that do operate on individual entries during the recording, then apply all session-level sanitizers when saving to disk.

This makes sense because to not apply individual sanitizers to each request means that if a sanitizer is added while a recording is in progress than it should only apply to the requests after it was added. Example Timeline below

  • Admin/AddSanitizer to add sanitizer A
  • Record/Start -> returns recordingid
  • Record Request 1
  • Admin/AddSanitizer to add sanitizer B
  • Record Request 2

Request 1 should be sanitized by A only. Request 2 should be sanitized by A AND B.

With the way the proxy currently works, A and B will apply to both regardless.

@scbedd scbedd added the Central-EngSys This issue is owned by the Engineering System team. label May 2, 2022
@scbedd scbedd self-assigned this May 2, 2022
@scbedd scbedd added the Test-Proxy Anything relating to test-proxy requests or issues. label May 11, 2022
@scbedd scbedd changed the title [Test-Proxy] Apply Sanitizers Immediately Apply Sanitizers Immediately May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team. Test-Proxy Anything relating to test-proxy requests or issues.
Projects
Status: 📋 Backlog
Development

No branches or pull requests

1 participant