Skip to content

Commit

Permalink
[monitor-query] Work around the double-JSON encoding bug (#15718)
Browse files Browse the repository at this point in the history
The Log Analytics service has a bug where it sometimes returns the batch query output encoded twice. 

The issue is still under investigation (#15688) but in the meantime this workaround will enable people to use the API, which is critical since we're in preview.

Mitigation for #15688
  • Loading branch information
richardpark-msft authored Jun 14, 2021
1 parent 9167bc3 commit aa12ada
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 8 deletions.
56 changes: 56 additions & 0 deletions sdk/monitor/monitor-query/src/internal/modelConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export function convertRequestForQueryBatch(batch: QueryLogsBatch): GeneratedBat
export function convertResponseForQueryBatch(
generatedResponse: GeneratedQueryBatchResponse
): QueryLogsBatchResult {
const fixApplied = fixInvalidBatchQueryResponse(generatedResponse);

const newResponse: QueryLogsBatchResult = {
results: generatedResponse.responses
?.sort((a, b) => {
Expand All @@ -102,9 +104,63 @@ export function convertResponseForQueryBatch(
}))
};

(newResponse as any)["__fixApplied"] = fixApplied;
return newResponse;
}

/**
* This is a workaround for a service bug that we're investigating. The 'body' column will occasionally come
* back as a JSON string, instead of being a JSON object.
*
* (examples, with excess stuff trimmed)
* Correct: `{"responses":[{"body":{"tables":[{"name":"PrimaryResult","columns":[{"name":"stringcolumn","type":"string"}],"rows":[["hello"]}`
* Broken: `{"responses":[{"body":"{\"tables\":[{\"name\":\"PrimaryResult\",\"columns\":[{\"name\":\"stringcolumn\",\"type\":\"string\"}],\"rows\":[[\"hello\"]}`
*
* Issue here: https://github.com/Azure/azure-sdk-for-js/issues/15688
*
* @internal
*/
export function fixInvalidBatchQueryResponse(
generatedResponse: GeneratedQueryBatchResponse
): boolean {
if (generatedResponse.responses == null) {
return false;
}

let wholeResponse: GeneratedQueryBatchResponse | undefined;
let hadToFix = false;

// fix whichever responses are in this broken state (each query has it's own
// response, so they're not all always broken)
for (let i = 0; i < generatedResponse.responses.length; ++i) {
if (
generatedResponse.responses[i].body?.tables != null ||
generatedResponse.responses[i].body?.error != null
) {
continue;
}

// the body here is incorrect, deserialize the correct one from the raw response itself.

// deserialize the raw response from the service, since we'll need index into it.
if (!wholeResponse) {
wholeResponse = JSON.parse(
generatedResponse["_response"].bodyAsText
) as GeneratedQueryBatchResponse;
}

// now grab the individual batch query response and deserialize that
// incorrectly typed string...
generatedResponse.responses[i].body = JSON.parse(
(wholeResponse.responses![i].body as any) as string
);

hadToFix = true;
}

return hadToFix;
}

/**
* @internal
*/
Expand Down
22 changes: 14 additions & 8 deletions sdk/monitor/monitor-query/test/public/logsQueryClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,7 @@ describe("LogsQueryClient live tests", function() {
});
});

// TODO: there is something odd happening here where the body is coming
// back as a string, rather than an object.
it("queryBatch with types", async () => {
it("queryLogsBatch with types", async () => {
const constantsQuery = `print "hello", true, make_datetime("2000-01-02 03:04:05Z"), toint(100), long(101), 102.1, dynamic({ "hello": "world" })
| project
stringcolumn=print_0,
Expand All @@ -232,7 +230,7 @@ describe("LogsQueryClient live tests", function() {
dynamiccolumn=print_6
`;

const results = await createClient().queryLogsBatch({
const result = await createClient().queryLogsBatch({
queries: [
{
workspace: monitorWorkspaceId,
Expand All @@ -242,7 +240,11 @@ describe("LogsQueryClient live tests", function() {
]
});

const table = results.results?.[0].tables?.[0];
if ((result as any)["__fixApplied"]) {
console.log(`TODO: Fix was required to pass`);
}

const table = result.results?.[0].tables?.[0];

if (table == null) {
throw new Error("No table returned for query");
Expand Down Expand Up @@ -385,10 +387,14 @@ describe("LogsQueryClient live tests", function() {
]
};

const response = await createClient().queryLogsBatch(batchRequest);
const result = await createClient().queryLogsBatch(batchRequest);

if ((result as any)["__fixApplied"]) {
console.log(`TODO: Fix was required to pass`);
}

assertQueryTable(
response.results?.[0].tables?.[0],
result.results?.[0].tables?.[0],
{
name: "PrimaryResult",
columns: ["Kind", "Name", "Target", "TestRunId"],
Expand All @@ -398,7 +404,7 @@ describe("LogsQueryClient live tests", function() {
);

assertQueryTable(
response.results?.[1].tables?.[0],
result.results?.[1].tables?.[0],
{
name: "PrimaryResult",
columns: ["Count"],
Expand Down

0 comments on commit aa12ada

Please sign in to comment.