Adding match as command line arguement for strict request matching #556
Conversation
|
|
|
Hi @karishma-kasliwal, I don't fully understand the purpose of the proposed changes. Could you help me understand what you're trying to achieve here? Reading the changeset, you're adding a command line argument to be able to change the minimum number of differences needed to consider a tape as matching. The default value for this is Passing a value of 0 to this argument would mean that proxay would only return tapes with a 100% match (0 differences). Am I correct that, in practice, no value other than |
karishma-kasliwal
changed the title
I have made changes as requested. |
package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "proxay", | |||
| "version": "1.6.1", | |||
| "version": "1.6.2", | |||
There was a problem hiding this comment.
Issue: Please remove the version bump from this PR. We will do a release later on that contains this changeset as part of the release.
src/cli.ts
Outdated
| @@ -57,6 +57,7 @@ async function main(argv: string[]) { | |||
| .option("--default-tape <tape-name>", "Name of the default tape", "default") | |||
| .option("-h, --host <host>", "Host to proxy (not required in replay mode)") | |||
| .option("-p, --port <port>", "Local port to serve on", "3000") | |||
| .option("--match <match>", "Strict request matching") | |||
There was a problem hiding this comment.
Suggestion: The CLI argument here could be a lot more self-descriptive:
| .option("--match <match>", "Strict request matching") | |
| .option("--exact-request-matching", "Perform exact request matching instead of best-effort request matching during replay.") |
Flag CLI arguments don't need a value, it seems: https://github.com/tj/commander.js?tab=readme-ov-file#common-option-types-boolean-and-value
| case "x-datadog-trace-id": | ||
| case "x-datadog-parent-id": | ||
| case "traceparent": |
There was a problem hiding this comment.
Non-blocking comment: Thanks for these sensible additions 👍
src/cli.ts
Outdated
| @@ -105,6 +106,7 @@ async function main(argv: string[]) { | |||
| const unframeGrpcWebJsonRequestsHostnames: string[] = | |||
| options.unframeGrpcWebJsonRequestsHostname; | |||
| const rewriteBeforeDiffRules: RewriteRules = options.rewriteBeforeDiff; | |||
| const match: boolean = options.match === undefined ? false : options.match; | |||
There was a problem hiding this comment.
Suggestion: Rename match to exactRequestMatching throughout this changeset to make the intent of the flag more self-descriptive.
|
@karishma-kasliwal feel free to merge this PR whenever you're ready. We'll do a release for you once it's merged. |
@timdawborn I dont see an option to merge this PR. It could be possible that I dont have the required permissions. |
|
@karishma-kasliwal This functionality has been included in the newly published Thanks again for contributing! |
Adding --match where we can pass boolean value for strict request matching while running proxay in 'Replay' mode.
Skipping --match will consider strict matching false, and proxay will behave as before.
Passing --match as true would enable strict request matching, that is zero differences between current request and recorded one.