Throttle LSP error telemetry reporting (#599)

* Throttle LSP error telemetry reporting

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>

* Reduce timeout

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>

* Fix error messages for telemetry

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>

Author: evidolob

src/telemetry.ts

@@ -46,6 +46,8 @@ export class TelemetryErrorHandler implements ErrorHandler {
 const errorMassagesToSkip = [{ text: 'Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED', contains: true }];
 
 export class TelemetryOutputChannel implements vscode.OutputChannel {
+  private errors: string[] | undefined;
+  private throttleTimeout: NodeJS.Timeout | undefined;
   constructor(private readonly delegate: vscode.OutputChannel, private readonly telemetry: TelemetryService) {}
 
   get name(): string {
@@ -65,7 +67,17 @@ export class TelemetryOutputChannel implements vscode.OutputChannel {
       if (this.isNeedToSkip(value)) {
         return;
       }
-      this.telemetry.send({ name: 'yaml.server.error', properties: { error: this.createErrorMessage(value) } });
+      if (!this.errors) {
+        this.errors = [];
+      }
+      if (this.throttleTimeout) {
+        clearTimeout(this.throttleTimeout);
+      }
+      this.errors.push(value);
+      this.throttleTimeout = setTimeout(() => {
+        this.telemetry.send({ name: 'yaml.server.error', properties: { error: this.createErrorMessage() } });
+        this.errors = undefined;
+      }, 50);
     }
   }
 
@@ -86,12 +98,17 @@ export class TelemetryOutputChannel implements vscode.OutputChannel {
     return false;
   }
 
-  private createErrorMessage(value: string): string {
-    if (value.startsWith('[Error')) {
-      value = value.substr(value.indexOf(']') + 1, value.length).trim();
+  private createErrorMessage(): string {
+    const result = [];
+    for (const value of this.errors) {
+      if (value.startsWith('[Error')) {
+        result.push(value.substr(value.indexOf(']') + 1, value.length).trim());
+      } else {
+        result.push(value);
+      }
     }
 
-    return value;
+    return result.join('\n');
   }
 
   clear(): void {

test/telemetry.test.ts

@@ -39,13 +39,20 @@ describe('Telemetry Test', () => {
     let telemetryChannel: TelemetryOutputChannel;
     let outputChannel: sinon.SinonStubbedInstance<vscode.OutputChannel>;
     let telemetry: sinon.SinonStubbedInstance<TelemetryService>;
+    let clock: sinon.SinonFakeTimers;
+
     beforeEach(() => {
       outputChannel = sandbox.stub(testOutputChannel);
       telemetry = sandbox.stub(new TelemetryStub());
       telemetryChannel = new TelemetryOutputChannel(
         (outputChannel as unknown) as vscode.OutputChannel,
         (telemetry as unknown) as TelemetryService
       );
+      clock = sinon.useFakeTimers();
+    });
+
+    afterEach(() => {
+      clock.restore();
     });
 
     it('should delegate "append" method', () => {
@@ -80,20 +87,41 @@ describe('Telemetry Test', () => {
 
     it('should send telemetry if log error in "append"', () => {
       telemetryChannel.append('[Error] Some');
+      clock.tick(51);
       expect(telemetry.send).calledOnceWith({ name: 'yaml.server.error', properties: { error: 'Some' } });
     });
 
     it('should send telemetry if log error on "appendLine"', () => {
       telemetryChannel.appendLine('[Error] Some error');
+      clock.tick(51);
       expect(telemetry.send).calledOnceWith({ name: 'yaml.server.error', properties: { error: 'Some error' } });
     });
 
     it("shouldn't send telemetry if error should be skipped", () => {
       telemetryChannel.append(
         "[Error - 15:10:33] (node:25052) Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to '0' makes TLS connections and HTTPS requests insecure by disabling certificate verification."
       );
+      clock.tick(51);
       expect(telemetry.send).not.called;
     });
+
+    it('should throttle send telemetry if "append" called multiple times', () => {
+      telemetryChannel.append('[Error] Some');
+      telemetryChannel.append('[Error] Second Error');
+      clock.tick(51);
+      expect(telemetry.send).calledOnceWith({ name: 'yaml.server.error', properties: { error: 'Some\nSecond Error' } });
+    });
+
+    it('should throttle send telemetry if "appendLine" called multiple times', () => {
+      telemetryChannel.appendLine('[Error] Some');
+      telemetryChannel.appendLine('[Error] Second Error');
+      telemetryChannel.appendLine('[Error] Third Error');
+      clock.tick(51);
+      expect(telemetry.send).calledOnceWith({
+        name: 'yaml.server.error',
+        properties: { error: 'Some\nSecond Error\nThird Error' },
+      });
+    });
   });
 
   describe('TelemetryErrorHandler', () => {