Change ErrorInfo Format (#19240)

* start

* changed results error

* update tests

* new ErrorInfo format

* tidy

* fix readme

* add to tests/readme

* fix link

Co-authored-by: gracewilcox <[email protected]>

sdk/monitor/azquery/CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Features Added
 
 ### Breaking Changes
+* Changed format of `ErrorInfo` to custom error type
 
 ### Bugs Fixed
 

sdk/monitor/azquery/README.md

@@ -22,8 +22,8 @@ go get github.com/Azure/azure-sdk-for-go/sdk/azidentity
 
 * An [Azure subscription][azure_sub]
 * A supported Go version (the Azure SDK supports the two most recent Go releases)
-* For log queries, a Log Analytics workspace. 
-* For metric queries, a Resource URI.
+* For log queries, an [Azure Log Analytics workspace][log_analytics_workspace_create] ID. 
+* For metric queries, the Resource URI of any Azure resource (Storage Account, Key Vault, CosmosDB, etc).
 
 ### Authentication
 
@@ -77,7 +77,7 @@ For examples of Logs and Metrics queries, see the [Examples](#examples) section
 
 The Log Analytics service applies throttling when the request rate is too high. Limits, such as the maximum number of rows returned, are also applied on the Kusto queries. For more information, see [Query API](https://docs.microsoft.com/azure/azure-monitor/service-limits#la-query-api).
 
-If you're executing a batch logs query, a throttled request will return a `LogsQueryError` object. That object's `code` value will be `ThrottledError`.
+If you're executing a batch logs query, a throttled request will return a `ErrorInfo` object. That object's `code` value will be `ThrottledError`.
 
 ### Metrics data structure
 
@@ -136,8 +136,8 @@ full example: [link][example_query_workspace]
 ```
 Body
 |---Query *string // Kusto Query
-|---Timespan *string // ISO8601 Standard Timespan- refer to timespan section for more info
-|---Workspaces []*string //Optional- additional workspaces to query
+|---Timespan *string // ISO8601 Standard Timespan
+|---Workspaces []*string // Optional- additional workspaces to query
 ```
 
 #### Logs query result structure
@@ -150,6 +150,7 @@ Results
 	|---Name *string
 	|---Rows [][]interface{}
 |---Error *ErrorInfo
+	|---Code *string // custom error type
 |---Render interface{}
 |---Statistics interface{}
 ```
@@ -193,7 +194,8 @@ BatchRequest
 BatchResponse
 |---Responses []*BatchQueryResponse
 	|---Body *BatchQueryResults
-		|---Error *ErrorInfo
+		|---Error *ErrorInfo // custom error type
+			|---Code *string
 		|---Render interface{}
 		|---Statistics interface{}
 		|---Tables []*Table
@@ -301,6 +303,12 @@ Response
 
 ## Troubleshooting
 
+### Error Handling
+
+All methods which send HTTP requests return `*azcore.ResponseError` when these requests fail. `ResponseError` has error details and the raw response from Monitor Query.
+
+For Logs, an error may also be returned in the response's `ErrorInfo` struct, usually to indicate a partial error from the service.
+
 ### Logging
 
 This module uses the logging implementation in `azcore`. To turn on logging for all Azure SDK modules, set `AZURE_SDK_GO_LOGGING` to `all`. By default the logger writes to stderr. Use the `azcore/log` package to control log output. For example, logging only HTTP request and response events, and printing them to stdout:
@@ -341,10 +349,12 @@ comments.
 [azure_monitor_create_using_portal]: https://docs.microsoft.com/azure/azure-monitor/logs/quick-create-workspace
 [azure_monitor_overview]: https://docs.microsoft.com/azure/azure-monitor/overview
 [context]: https://pkg.go.dev/context
+[default_cred_ref]: https://github.com/Azure/azure-sdk-for-go/tree/main/sdk/azidentity#defaultazurecredential
 [example_batch]: https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/monitor/azquery#example-LogsClient.Batch
 [example_query_workspace]: https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/monitor/azquery#example-LogsClient.QueryWorkspace
 [kusto_query_language]: https://learn.microsoft.com/azure/data-explorer/kusto/query/
 [log_analytics_workspace]: https://learn.microsoft.com/azure/azure-monitor/logs/log-analytics-workspace-overview
+[log_analytics_workspace_create]: https://learn.microsoft.com/azure/azure-monitor/logs/quick-create-workspace?tabs=azure-portal
 [time_go]: https://pkg.go.dev/time
 [time_intervals]: https://en.wikipedia.org/wiki/ISO_8601#Time_intervals
 

sdk/monitor/azquery/autorest.md

@@ -97,6 +97,12 @@ directive:
   - from: models_serde.go
     where: $
     transform: return $.replace(/(?:\/\/.*\s)+func \(\w \*?(?:ErrorResponse|ErrorResponseAutoGenerated)\).*\{\s(?:.+\s)+\}\s/g, "");
+  - from: models.go
+    where: $
+    transform: return $.replace(/(?:\/\/.*\s)+type (?:ErrorInfo|ErrorDetail).+\{(?:\s.+\s)+\}\s/g, "");
+  - from: models_serde.go
+    where: $
+    transform: return $.replace(/(?:\/\/.*\s)+func \(\w \*?(?:ErrorInfo|ErrorDetail)\).*\{\s(?:.+\s)+\}\s/g, "");
 
   # delete generated constructor
   - from: logs_client.go

sdk/monitor/azquery/custom_client.go

@@ -9,6 +9,9 @@ package azquery
 // this file contains handwritten additions to the generated code
 
 import (
+	"encoding/json"
+	"fmt"
+
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
@@ -45,3 +48,29 @@ func NewMetricsClient(credential azcore.TokenCredential, options *MetricsClientO
 }
 
 const metricsHost string = "https://management.azure.com"
+
+// ErrorInfo - The code and message for an error.
+type ErrorInfo struct {
+	// REQUIRED; A machine readable error code.
+	Code string
+
+	// full error message detailing why the operation failed.
+	data []byte
+}
+
+// UnmarshalJSON implements the json.Unmarshaller interface for type ErrorInfo.
+func (e *ErrorInfo) UnmarshalJSON(data []byte) error {
+	e.data = data
+	ei := struct{ Code string }{}
+	if err := json.Unmarshal(data, &ei); err != nil {
+		return fmt.Errorf("unmarshalling type %T: %v", e, err)
+	}
+	e.Code = ei.Code
+
+	return nil
+}
+
+// Error implements a custom error for type ErrorInfo.
+func (e *ErrorInfo) Error() string {
+	return string(e.data)
+}

sdk/monitor/azquery/logs_client_test.go

@@ -8,8 +8,11 @@ package azquery_test
 
 import (
 	"context"
+	"errors"
+	"strings"
 	"testing"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
 	"github.com/Azure/azure-sdk-for-go/sdk/monitor/azquery"
 	"github.com/stretchr/testify/require"
@@ -31,58 +34,72 @@ func TestQueryWorkspace_BasicQuerySuccess(t *testing.T) {
 		t.Fatalf("error with query, %s", err.Error())
 	}
 
-	if res.Results.Error != nil {
+	if res.Error != nil {
 		t.Fatal("expended Error to be nil")
 	}
-	if res.Results.Render != nil {
+	if res.Render != nil {
 		t.Fatal("expended Render to be nil")
 	}
-	if res.Results.Statistics != nil {
+	if res.Statistics != nil {
 		t.Fatal("expended Statistics to be nil")
 	}
-	if len(res.Results.Tables) != 1 {
+	if len(res.Tables) != 1 {
 		t.Fatal("expected one table")
 	}
-	if len(res.Results.Tables[0].Rows) != 100 {
+	if len(res.Tables[0].Rows) != 100 {
 		t.Fatal("expected 100 rows")
 	}
 
-	testSerde(t, &res.Results)
+	testSerde(t, &res)
 }
 
 func TestQueryWorkspace_BasicQueryFailure(t *testing.T) {
 	client := startLogsTest(t)
-	query := "not a valid query"
-	body := azquery.Body{
-		Query: &query,
-	}
 
-	res, err := client.QueryWorkspace(context.Background(), workspaceID, body, nil)
+	res, err := client.QueryWorkspace(context.Background(), workspaceID, azquery.Body{Query: to.Ptr("not a valid query")}, nil)
 	if err == nil {
-		t.Fatalf("expected BadArgumentError")
+		t.Fatalf("expected an error")
+	}
+	if res.Error != nil {
+		t.Fatal("expected no error code")
 	}
-	if res.Results.Tables != nil {
+	if res.Tables != nil {
 		t.Fatalf("expected no results")
 	}
-	testSerde(t, &res.Results)
+
+	var httpErr *azcore.ResponseError
+	if !errors.As(err, &httpErr) {
+		t.Fatal("expected an azcore.ResponseError")
+	}
+	if httpErr.ErrorCode != "BadArgumentError" {
+		t.Fatal("expected a BadArgumentError")
+	}
+	if httpErr.StatusCode != 400 {
+		t.Fatal("expected a 400 error")
+	}
+
+	testSerde(t, &res)
 }
 
 func TestQueryWorkspace_PartialError(t *testing.T) {
 	client := startLogsTest(t)
 	query := "let Weight = 92233720368547758; range x from 1 to 3 step 1 | summarize percentilesw(x, Weight * 100, 50)"
-	body := azquery.Body{
-		Query: &query,
-	}
 
-	res, err := client.QueryWorkspace(context.Background(), workspaceID, body, nil)
+	res, err := client.QueryWorkspace(context.Background(), workspaceID, azquery.Body{Query: &query}, nil)
 	if err != nil {
 		t.Fatal("error with query")
 	}
-	if *res.Results.Error.Code != "PartialError" {
+	if res.Error == nil {
+		t.Fatal("expected an error")
+	}
+	if res.Error.Code != "PartialError" {
 		t.Fatal("expected a partial error")
 	}
+	if !strings.Contains(res.Error.Error(), "PartialError") {
+		t.Fatal("expected error message to contain PartialError")
+	}
 
-	testSerde(t, &res.Results)
+	testSerde(t, &res)
 }
 
 // tests for special options: timeout, statistics, visualization
@@ -99,16 +116,16 @@ func TestQueryWorkspace_AdvancedQuerySuccess(t *testing.T) {
 	if err != nil {
 		t.Fatalf("error with query, %s", err.Error())
 	}
-	if res.Results.Tables == nil {
+	if res.Tables == nil {
 		t.Fatal("expected Tables results")
 	}
-	if res.Results.Error != nil {
+	if res.Error != nil {
 		t.Fatal("expended Error to be nil")
 	}
-	if res.Results.Render == nil {
+	if res.Render == nil {
 		t.Fatal("expended Render results")
 	}
-	if res.Results.Statistics == nil {
+	if res.Statistics == nil {
 		t.Fatal("expended Statistics results")
 	}
 }
@@ -126,10 +143,10 @@ func TestQueryWorkspace_MultipleWorkspaces(t *testing.T) {
 	if err != nil {
 		t.Fatalf("error with query, %s", err.Error())
 	}
-	if res.Results.Error != nil {
+	if res.Error != nil {
 		t.Fatal("result error should be nil")
 	}
-	if len(res.Results.Tables[0].Rows) != 100 {
+	if len(res.Tables[0].Rows) != 100 {
 		t.Fatalf("expected 100 results, received")
 	}
 }
@@ -148,10 +165,24 @@ func TestBatch_QuerySuccess(t *testing.T) {
 	if err != nil {
 		t.Fatalf("expected non nil error: %s", err.Error())
 	}
-	if len(res.BatchResponse.Responses) != 2 {
+	if len(res.Responses) != 2 {
 		t.Fatal("expected two responses")
 	}
-	testSerde(t, &res.BatchResponse)
+	for _, resp := range res.Responses {
+		if resp.Body.Error != nil {
+			t.Fatal("expected a successful response")
+		}
+		if resp.Body.Tables == nil {
+			t.Fatal("expected a response")
+		}
+		if *resp.ID == "1" && len(resp.Body.Tables[0].Rows) != 100 {
+			t.Fatal("expected 100 rows from batch request 1")
+		}
+		if *resp.ID == "2" && len(resp.Body.Tables[0].Rows) != 2 {
+			t.Fatal("expected 100 rows from batch request 1")
+		}
+	}
+	testSerde(t, &res)
 }
 
 func TestBatch_PartialError(t *testing.T) {
@@ -166,9 +197,30 @@ func TestBatch_PartialError(t *testing.T) {
 	if err != nil {
 		t.Fatalf("expected non nil error: %s", err.Error())
 	}
-	if len(res.BatchResponse.Responses) != 2 {
+	if len(res.Responses) != 2 {
 		t.Fatal("expected two responses")
 	}
+	for _, resp := range res.Responses {
+		if *resp.ID == "1" {
+			if resp.Body.Error == nil {
+				t.Fatal("expected batch request 1 to fail")
+			}
+			if resp.Body.Error.Code != "BadArgumentError" {
+				t.Fatal("expected BadArgumentError")
+			}
+			if !strings.Contains(resp.Body.Error.Error(), "BadArgumentError") {
+				t.Fatal("expected error message to contain BadArgumentError")
+			}
+		}
+		if *resp.ID == "2" {
+			if resp.Body.Error != nil {
+				t.Fatal("expected batch request 2 to succeed")
+			}
+			if len(resp.Body.Tables[0].Rows) != 100 {
+				t.Fatal("expected 100 rows")
+			}
+		}
+	}
 }
 
 func TestLogConstants(t *testing.T) {