Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Monitor Exporter] Move the Monitor package to use core-rest-pipeline #16513

Merged
merged 10 commits into from
Jul 29, 2021
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ export class AzureMonitorTraceExporter implements SpanExporter {
}
} catch (error) {
const restError = error as RestError;
if (restError.statusCode && restError.statusCode === 308) {
if (
restError.statusCode &&
(restError.statusCode === 307 || // Temporary redirect
restError.statusCode === 308)
) {
// Permanent redirect
this._numConsecutiveRedirects++;
// To prevent circular redirects
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import {
PipelineResponse,
PipelineRequest,
SendRequest,
PipelinePolicy
} from "@azure/core-rest-pipeline";
import { URL } from "url";

/**
* The programmatic identifier of the redirectPolicy.
*/
export const redirectPolicyName = "customRedirectPolicy";

/**
* Methods that are allowed to follow redirects 301 and 302
*/
const allowedRedirect = ["GET", "HEAD"];

/**
* Options for how redirect responses are handled.
*/
export interface RedirectPolicyOptions {
/**
* The maximum number of times the redirect URL will be tried before
* failing. Defaults to 20.
*/
maxRetries?: number;
}

/**
* A policy to follow Location headers from the server in order
* to support server-side redirection.
* @param options - Options to control policy behavior.
*/
export function customRedirectPolicy(options: RedirectPolicyOptions = {}): PipelinePolicy {
const { maxRetries = 20 } = options;
return {
name: redirectPolicyName,
async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> {
const response = await next(request);
return handleRedirect(next, response, maxRetries);
}
};
}

async function handleRedirect(
next: SendRequest,
response: PipelineResponse,
maxRetries: number,
currentRetries: number = 0
): Promise<PipelineResponse> {
const { request, status, headers } = response;
const locationHeader = headers.get("location");
if (
locationHeader &&
(status === 300 ||
(status === 301 && allowedRedirect.includes(request.method)) ||
(status === 302 && allowedRedirect.includes(request.method)) ||
(status === 303 && request.method === "POST")) &&
currentRetries < maxRetries
) {
const url = new URL(locationHeader, request.url);
request.url = url.toString();

// POST request with Status code 303 should be converted into a
// redirected GET request if the redirect url is present in the location header
if (status === 303) {
request.method = "GET";
request.headers.delete("Content-Length");
delete request.body;
}

const res = await next(request);
return handleRedirect(next, res, maxRetries, currentRetries + 1);
}

return response;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import url from "url";
import { diag } from "@opentelemetry/api";
import { FullOperationResponse } from "@azure/core-client";
import { RestError } from "@azure/core-rest-pipeline";
import { RestError, redirectPolicyName } from "@azure/core-rest-pipeline";
import { Sender, SenderResult } from "../../types";
import {
TelemetryItem as Envelope,
Expand All @@ -12,6 +12,7 @@ import {
ApplicationInsightsClientTrackOptionalParams
} from "../../generated";
import { AzureExporterInternalConfig } from "../../config";
import { customRedirectPolicy } from "./customRedirectPolicy";

/**
* Exporter HTTP sender class
Expand All @@ -30,6 +31,9 @@ export class HttpSender implements Sender {
this._appInsightsClient = new ApplicationInsightsClient({
...this._appInsightsClientOptions
});

this._appInsightsClient.pipeline.removePolicy({ name: redirectPolicyName });
xiao-lix marked this conversation as resolved.
Show resolved Hide resolved
this._appInsightsClient.pipeline.addPolicy(customRedirectPolicy(), { afterPhase: "Retry" });
xiao-lix marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -51,14 +55,6 @@ export class HttpSender implements Sender {
onResponse
});

if (response?.status === 404) {
throw new RestError(response?.bodyAsText!, {
statusCode: response?.status,
request: response?.request,
response: response
});
}

return { statusCode: response?.status, result: response?.bodyAsText ?? "" };
} catch (e) {
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,74 @@ describe("#AzureMonitorBaseExporter", () => {
redirectHost
);
});

it("should handle temporary redirects in Azure Monitor", async () => {
const exporter = new TestExporter();
const redirectHost = "https://ukwest-0.in.applicationinsights.azure.com";
const redirectLocation = redirectHost + "/v2/track";
// Redirect endpoint
const redirectScope = nock(redirectHost).post("/v2/track", () => {
return true;
});
redirectScope.reply(200, JSON.stringify(successfulBreezeResponse(1)));
scope.reply(307, {}, { location: redirectLocation });

const result = await exporter.exportEnvelopesPrivate([envelope]);
const persistedEnvelopes = (await exporter["_persister"].shift()) as Envelope[];
assert.strictEqual(persistedEnvelopes, null);
assert.strictEqual(result.code, ExportResultCode.SUCCESS);
assert.strictEqual(
(<HttpSender>exporter["_sender"])["_appInsightsClient"]["host"],
redirectHost
);
});

it("should use redirect URL for following requests", async () => {
const exporter = new TestExporter();
const redirectHost = "https://ukwest-0.in.applicationinsights.azure.com";
const redirectLocation = redirectHost + "/v2/track";
// Redirect endpoint
const redirectScope = nock(redirectHost).post("/v2/track", () => {
return true;
});
redirectScope.twice().reply(200, JSON.stringify(successfulBreezeResponse(1)));
scope.reply(307, {}, { location: redirectLocation });
let result = await exporter.exportEnvelopesPrivate([envelope]);
assert.strictEqual(result.code, ExportResultCode.SUCCESS);
assert.strictEqual(
(<HttpSender>exporter["_sender"])["_appInsightsClient"]["host"],
redirectHost
);
result = await exporter.exportEnvelopesPrivate([envelope]);
assert.strictEqual(result.code, ExportResultCode.SUCCESS);
assert.strictEqual(
(<HttpSender>exporter["_sender"])["_appInsightsClient"]["host"],
redirectHost
);
});

it("should stop redirecting when circular redirect is triggered", async () => {
const exporter = new TestExporter();
const redirectHost = "https://ukwest-0.in.applicationinsights.azure.com";
const redirectLocation = redirectHost + "/v2/track";
// Redirect endpoint
const redirectScope = nock(redirectHost).post("/v2/track", () => {
return true;
});
// Circle redirect
scope
.reply(307, JSON.stringify(successfulBreezeResponse(1)), { location: redirectLocation })
.persist();
redirectScope
.reply(307, JSON.stringify(successfulBreezeResponse(1)), {
location: DEFAULT_BREEZE_ENDPOINT
})
.persist();

const result = await exporter.exportEnvelopesPrivate([envelope]);
assert.strictEqual(result.code, ExportResultCode.FAILED);
assert.strictEqual(result.error?.message, "Circular redirect");
});
});
});
});