Skip to content

Conversation

LarryOsterman
Copy link
Member

Fixes #3099

@Copilot Copilot AI review requested due to automatic review settings October 16, 2025 20:46
@github-actions github-actions bot added Azure.Core The azure_core crate Cosmos The azure_cosmos crate Event Hubs KeyVault Key Vault Storage Storage Service (Queues, Blobs, Files) labels Oct 16, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds test proxy support to the codebase by updating the instrument method signature to accept an optional RecordingOptions parameter. The changes enable control over test proxy behavior, particularly for performance tests, while maintaining backward compatibility by allowing None for most test cases.

Key Changes:

  • Updated Recording::instrument method to accept an Option<RecordingOptions> parameter
  • Added RemoveRecording header type to control whether recordings are removed during playback
  • Modified pipeline construction to move LoggingPolicy after per-try policies for proper test proxy integration
  • Updated all existing call sites to pass None for recording options, maintaining current behavior

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sdk/core/azure_core_test/src/recording.rs Added RecordingOptions parameter to instrument method and implemented RemoveRecording header
sdk/core/azure_core_test/src/proxy/policy.rs Extended RecordingOptions struct with remove_recording field and updated header conversion
sdk/core/typespec_client_core/src/http/pipeline.rs Moved LoggingPolicy after per-try policies in pipeline construction
sdk/keyvault/azure_security_keyvault_secrets/perf/get_secret.rs Updated to use new RecordingOptions with RemoveRecording(false) for performance tests
Multiple test files Updated all instrument calls to pass None for recording options
sdk/keyvault/assets.json Updated test assets tag reference
sdk/core/azure_core_test/src/perf/README.md Added documentation about performance tests being recorded tests

Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

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

Just a few suggestions, but this LGTM and is much cleaner!

Copy link
Member

@j7nw4r j7nw4r left a comment

Choose a reason for hiding this comment

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

LGTM

Commenting so that it is known I'm reviewing, though I want someone with a little more time to be the actual reviewer.

Copy link

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure_core

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core The azure_core crate Cosmos The azure_cosmos crate Event Hubs KeyVault Key Vault Storage Storage Service (Queues, Blobs, Files)

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

Added recorded test support to support the "test proxy" perf test feature.

3 participants