[AzureMonitorExporter] Adding support for temporary redirect (Azure#15850)

* Adding support for temporary redirect

* Lint

Author: hectorhdzg

sdk/monitor/monitor-opentelemetry-exporter/package.json

@@ -24,6 +24,7 @@
     "test:browser": "npm run unit-test:browser",
     "unit-test:browser": "echo skipped",
     "unit-test:node": "nyc mocha -r esm --require ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"test/unit/**/*.test.ts\"",
+    "unit-test:node:debug": "nyc mocha --inspect-brk -r esm --require ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"test/unit/**/*.test.ts\"",
     "unit-test:node:no-timeout": "echo skipped",
     "unit-test": "npm run unit-test:node && npm run unit-test:browser",
     "functional-test": "nyc mocha -r esm --require ts-node/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 1200000 --full-trace \"test/functional/**/*.test.ts\"",

sdk/monitor/monitor-opentelemetry-exporter/src/export/trace.ts

@@ -80,6 +80,7 @@ export class AzureMonitorTraceExporter implements SpanExporter {
 
     try {
       const { result, statusCode } = await this._sender.send(envelopes);
+      this._numConsecutiveRedirects = 0;
       if (statusCode === 200) {
         // Success -- @todo: start retry timer
         if (!this._retryTimer) {
@@ -121,12 +122,26 @@ 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
-        if (restError.response && restError.response.headers) {
-          let location = restError.response.headers.get("location");
-          this._handleRedirect(location);
-          return await this._persist(envelopes);
+        this._numConsecutiveRedirects++;
+        // To prevent circular redirects
+        if (this._numConsecutiveRedirects < 10) {
+          if (restError.response && restError.response.headers) {
+            let location = restError.response.headers.get("location");
+            if (location) {
+              // Update sender URL
+              this._sender.handlePermanentRedirect(location);
+              // Send to redirect endpoint as HTTPs library doesn't handle redirect automatically
+              return this.exportEnvelopes(envelopes);
+            }
+          }
+        } else {
+          return { code: ExportResultCode.FAILED, error: new Error("Circular redirect") };
         }
       } else if (restError.statusCode && isRetriable(restError.statusCode)) {
         return await this._persist(envelopes);
@@ -188,14 +203,4 @@ export class AzureMonitorTraceExporter implements SpanExporter {
     }
     return false;
   }
-
-  private _handleRedirect(location: string | undefined) {
-    if (location) {
-      this._numConsecutiveRedirects++;
-      // To prevent circular redirects
-      if (this._numConsecutiveRedirects < 10) {
-        this._sender.handlePermanentRedirect(location);
-      }
-    }
-  }
 }

sdk/monitor/monitor-opentelemetry-exporter/test/unit/export/trace.test.ts

@@ -10,7 +10,7 @@ import {
   partialBreezeResponse,
   successfulBreezeResponse
 } from "../breezeTestUtils";
-import { FileSystemPersist } from "../../../src/platform";
+import { FileSystemPersist, HttpSender } from "../../../src/platform";
 import { TelemetryItem as Envelope } from "../../../src/generated";
 import nock from "nock";
 
@@ -150,9 +150,8 @@ describe("#AzureMonitorBaseExporter", () => {
         assert.strictEqual(exporter["_retryTimer"], "foo");
       });
 
-      it("should handle redirects in Azure Monitor", async () => {
+      it("should handle permanent redirects in Azure Monitor", async () => {
         const exporter = new TestExporter();
-
         let redirectHost = "https://ukwest-0.in.applicationinsights.azure.com";
         let redirectLocation = redirectHost + "/v2/track";
         // Redirect endpoint
@@ -163,17 +162,81 @@ describe("#AzureMonitorBaseExporter", () => {
         scope.reply(308, {}, { location: redirectLocation });
 
         let result = await exporter.exportEnvelopesPrivate([envelope]);
-        // Redirect triggered so telemetry must be persisted
+        let 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 handle temporary redirects in Azure Monitor", async () => {
+        const exporter = new TestExporter();
+        let redirectHost = "https://ukwest-0.in.applicationinsights.azure.com";
+        let 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 });
+
+        let result = await exporter.exportEnvelopesPrivate([envelope]);
         let persistedEnvelopes = (await exporter["_persister"].shift()) as Envelope[];
-        assert.strictEqual(persistedEnvelopes?.length, 1);
-        assert.deepStrictEqual(persistedEnvelopes[0], toObject(envelope));
-        assert.strictEqual(exporter["_numConsecutiveRedirects"], 1);
-        // After redirect return 200
+        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();
+        let redirectHost = "https://ukwest-0.in.applicationinsights.azure.com";
+        let 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);
-        persistedEnvelopes = (await exporter["_persister"].shift()) as Envelope[];
-        assert.strictEqual(persistedEnvelopes, null);
+        assert.strictEqual(
+          (<HttpSender>exporter["_sender"])["_appInsightsClient"]["host"],
+          redirectHost
+        );
+      });
+
+      it("should stop redirecting when circular redirect is triggered", async () => {
+        const exporter = new TestExporter();
+        let redirectHost = "https://ukwest-0.in.applicationinsights.azure.com";
+        let 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();
+
+        let result = await exporter.exportEnvelopesPrivate([envelope]);
+        assert.strictEqual(result.code, ExportResultCode.FAILED);
+        assert.strictEqual(result.error?.message, "Circular redirect");
       });
     });
   });