From b59b15cc44407f8ecd616c0b4cf68d2a7dbe714b Mon Sep 17 00:00:00 2001 From: Simon Moreno <30335873+simorenoh@users.noreply.github.com> Date: Tue, 19 Mar 2024 12:40:34 -0700 Subject: [PATCH] [Cosmos] add preferred regions logging (#22598) * add new client option * add client config logs * remove new option * nil fix * fix * log client preferred regions on every request * move logging logic to policy * change to 1.21 * Revert "move logging logic to policy" This reverts commit 58be1caea3091d888507caae56cd8d25496a3b69. * move logging logic, add gem to tests * Update go.mod * Update ci.yml * Update ci.yml * Update ci.yml --- sdk/data/azcosmos/ci.yml | 5 ++- sdk/data/azcosmos/cosmos_client.go | 4 +- .../cosmos_client_retry_policy_test.go | 2 - sdk/data/azcosmos/cosmos_client_test.go | 45 ++++++++++++------- sdk/data/azcosmos/cosmos_container_test.go | 27 +++++++---- sdk/data/azcosmos/cosmos_database_test.go | 3 +- .../cosmos_global_endpoint_manager_test.go | 8 +--- 7 files changed, 58 insertions(+), 36 deletions(-) diff --git a/sdk/data/azcosmos/ci.yml b/sdk/data/azcosmos/ci.yml index 54623a90bf1f..4e76d3ef3749 100644 --- a/sdk/data/azcosmos/ci.yml +++ b/sdk/data/azcosmos/ci.yml @@ -25,6 +25,7 @@ extends: parameters: ServiceDirectory: 'data/azcosmos' UsePipelineProxy: false + ExcludeGoNMinus2: true AdditionalStages: - stage: Emulator displayName: 'Cosmos Emulator' @@ -59,7 +60,7 @@ extends: - task: GoTool@0 inputs: - version: '$(go.version)' + version: '1.22.0' displayName: "Select Go Version" - template: /eng/pipelines/templates/steps/create-go-workspace.yml@self @@ -68,6 +69,6 @@ extends: parameters: ServiceDirectory: 'data/azcosmos' Image: $(vm.image) - GoVersion: $(go.version) + GoVersion: '1.22.0' EnvVars: EMULATOR: 'true' diff --git a/sdk/data/azcosmos/cosmos_client.go b/sdk/data/azcosmos/cosmos_client.go index 69356da806a4..ad95aa5cf45d 100644 --- a/sdk/data/azcosmos/cosmos_client.go +++ b/sdk/data/azcosmos/cosmos_client.go @@ -14,9 +14,11 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" + azlog "github.com/Azure/azure-sdk-for-go/sdk/azcore/log" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" azruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" "github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming" + "github.com/Azure/azure-sdk-for-go/sdk/internal/log" ) const ( @@ -70,7 +72,6 @@ func NewClient(endpoint string, cred azcore.TokenCredential, o *ClientOptions) ( if err != nil { return nil, err } - return &Client{endpoint: endpoint, pipeline: newPipeline(newCosmosBearerTokenPolicy(cred, scope, nil), gem, o), gem: gem}, nil } @@ -471,6 +472,7 @@ func (c *Client) attachContent(content interface{}, req *policy.Request) error { } func (c *Client) executeAndEnsureSuccessResponse(request *policy.Request) (*http.Response, error) { + log.Write(azlog.EventResponse, fmt.Sprintf("\n===== Client preferred regions:\n%v\n=====\n", c.gem.preferredLocations)) response, err := c.pipeline.Do(request) if err != nil { return nil, err diff --git a/sdk/data/azcosmos/cosmos_client_retry_policy_test.go b/sdk/data/azcosmos/cosmos_client_retry_policy_test.go index c803905911d9..ad9b5a927660 100644 --- a/sdk/data/azcosmos/cosmos_client_retry_policy_test.go +++ b/sdk/data/azcosmos/cosmos_client_retry_policy_test.go @@ -6,7 +6,6 @@ package azcosmos import ( "context" "encoding/json" - "fmt" "net" "net/url" "testing" @@ -366,7 +365,6 @@ func TestReadServiceUnavailable(t *testing.T) { // Request should retry twice and then succeed (2 preferred regions) assert.NoError(t, err) assert.True(t, retryPolicy.retryCount == 2) - fmt.Println("we here 1") // Setting up responses for retrying and failing srv.AppendResponse( diff --git a/sdk/data/azcosmos/cosmos_client_test.go b/sdk/data/azcosmos/cosmos_client_test.go index a13eb98dcab4..18891646aaf3 100644 --- a/sdk/data/azcosmos/cosmos_client_test.go +++ b/sdk/data/azcosmos/cosmos_client_test.go @@ -90,7 +90,8 @@ func TestEnsureErrorIsGeneratedOnResponse(t *testing.T) { mock.WithStatusCode(404)) pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -128,7 +129,8 @@ func TestEnsureErrorIsNotGeneratedOnResponse(t *testing.T) { mock.WithStatusCode(200)) pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -146,7 +148,8 @@ func TestRequestEnricherIsCalled(t *testing.T) { mock.WithStatusCode(200)) pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -173,7 +176,8 @@ func TestNoOptionsIsCalled(t *testing.T) { mock.WithStatusCode(200)) pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -190,7 +194,8 @@ func TestAttachContent(t *testing.T) { defer close() pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -241,7 +246,8 @@ func TestCreateRequest(t *testing.T) { srv, close := mock.NewTLSServer() defer close() pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -285,7 +291,8 @@ func TestSendDelete(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -308,7 +315,8 @@ func TestSendGet(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -331,7 +339,8 @@ func TestSendPut(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -364,7 +373,8 @@ func TestSendPost(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -397,7 +407,8 @@ func TestSendQuery(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -432,7 +443,8 @@ func TestSendQueryWithParameters(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -474,7 +486,8 @@ func TestSendBatch(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDocument, resourceAddress: "", @@ -518,7 +531,8 @@ func TestSendPatch(t *testing.T) { mock.WithStatusCode(200)) verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} operationContext := pipelineRequestOptions{ resourceType: resourceTypeDatabase, resourceAddress: "", @@ -594,7 +608,8 @@ func TestQueryDatabases(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} receivedIds := []string{} queryPager := client.NewQueryDatabasesPager("select * from c", nil) diff --git a/sdk/data/azcosmos/cosmos_container_test.go b/sdk/data/azcosmos/cosmos_container_test.go index 62a7976bdd6d..4fa92dea66c6 100644 --- a/sdk/data/azcosmos/cosmos_container_test.go +++ b/sdk/data/azcosmos/cosmos_container_test.go @@ -48,7 +48,8 @@ func TestContainerRead(t *testing.T) { mock.WithStatusCode(200)) pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -123,7 +124,8 @@ func TestContainerDeleteItem(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -176,7 +178,8 @@ func TestContainerReadItem(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -233,7 +236,8 @@ func TestContainerReplaceItem(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -294,7 +298,8 @@ func TestContainerUpsertItem(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -359,7 +364,8 @@ func TestContainerCreateItem(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -437,7 +443,8 @@ func TestContainerQueryItems(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -544,7 +551,8 @@ func TestContainerExecuteBatch(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) @@ -607,7 +615,8 @@ func TestContainerPatchItem(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) container, _ := newContainer("containerId", database) diff --git a/sdk/data/azcosmos/cosmos_database_test.go b/sdk/data/azcosmos/cosmos_database_test.go index d4a199306e54..0cdb450c9ed8 100644 --- a/sdk/data/azcosmos/cosmos_database_test.go +++ b/sdk/data/azcosmos/cosmos_database_test.go @@ -37,7 +37,8 @@ func TestDatabaseQueryContainers(t *testing.T) { verifier := pipelineVerifier{} pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{PerCall: []policy.Policy{&verifier}}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} + gem := &globalEndpointManager{preferredLocations: []string{}} + client := &Client{endpoint: srv.URL(), pipeline: pl, gem: gem} database, _ := newDatabase("databaseId", client) diff --git a/sdk/data/azcosmos/cosmos_global_endpoint_manager_test.go b/sdk/data/azcosmos/cosmos_global_endpoint_manager_test.go index 63d9c317180a..a40e9d8b6270 100644 --- a/sdk/data/azcosmos/cosmos_global_endpoint_manager_test.go +++ b/sdk/data/azcosmos/cosmos_global_endpoint_manager_test.go @@ -79,9 +79,7 @@ func TestGlobalEndpointManagerMarkEndpointUnavailableForRead(t *testing.T) { pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} - - endpoint, err := url.Parse(client.endpoint) + endpoint, err := url.Parse(srv.URL()) assert.NoError(t, err) gem, err := newGlobalEndpointManager(srv.URL(), pl, []string{"West US", "Central US"}, 5*time.Minute, true) @@ -101,9 +99,7 @@ func TestGlobalEndpointManagerMarkEndpointUnavailableForWrite(t *testing.T) { pl := azruntime.NewPipeline("azcosmostest", "v1.0.0", azruntime.PipelineOptions{}, &policy.ClientOptions{Transport: srv}) - client := &Client{endpoint: srv.URL(), pipeline: pl} - - endpoint, err := url.Parse(client.endpoint) + endpoint, err := url.Parse(srv.URL()) assert.NoError(t, err) gem, err := newGlobalEndpointManager(srv.URL(), pl, []string{"West US", "Central US"}, 5*time.Minute, true)