diff --git a/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryClient.scala b/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryClient.scala index 1ece135d45b..c365c29c8bc 100644 --- a/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryClient.scala +++ b/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryClient.scala @@ -77,13 +77,8 @@ private[meta] class TelemetryClient( private val sendErrorReport0 = new TelemetryRequest(sendErrorReportEndpoint, logger) - private val sendCrashReport0 = - new TelemetryRequest(sendCrashReportEndpoint, logger) def sendErrorReport(report: telemetry.ErrorReport): Unit = if (telemetryLevel().enabled) sendErrorReport0(report) - def sendCrashReport(report: telemetry.CrashReport): Unit = - if (telemetryLevel().enabled) sendCrashReport0(report) - } diff --git a/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryReportContext.scala b/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryReportContext.scala index 1a88e50a6e6..4e8fa5ddf2c 100644 --- a/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryReportContext.scala +++ b/metals/src/main/scala/scala/meta/internal/telemetry/TelemetryReportContext.scala @@ -17,6 +17,7 @@ import scala.meta.pc.Report import scala.meta.pc.ReportContext import scala.meta.pc.Reporter import scala.meta.pc.TimestampedFile +import scala.meta.internal.metals.EmptyReporter object TelemetryReportContext { case class Sanitizers( @@ -54,10 +55,16 @@ class TelemetryReportContext( )(implicit ec: ExecutionContext) extends ReportContext { + val telemetryLevel0 = telemetryLevel() + // Don't send reports with fragile user data - sources etc - override lazy val unsanitized: Reporter = reporter("unsanitized") - override lazy val incognito: Reporter = reporter("incognito") - override lazy val bloop: Reporter = reporter("bloop") + override lazy val unsanitized: Reporter = + if (telemetryLevel0 == TelemetryLevel.Full) reporter("incognito") + else EmptyReporter + override lazy val incognito: Reporter = + if (telemetryLevel0.enabled) reporter("incognito") else EmptyReporter + override lazy val bloop: Reporter = + if (telemetryLevel0.enabled) reporter("bloop") else EmptyReporter private val client = new TelemetryClient( config = telemetryClientConfig, @@ -68,7 +75,6 @@ class TelemetryReportContext( private def reporter(name: String) = new TelemetryReporter( name = name, client = client, - telemetryLevel = telemetryLevel, reporterContext = reporterContext, sanitizers = sanitizers, logger = logger, @@ -78,7 +84,6 @@ class TelemetryReportContext( private class TelemetryReporter( override val name: String, client: TelemetryClient, - telemetryLevel: () => TelemetryLevel, reporterContext: () => telemetry.ReporterContext, sanitizers: TelemetryReportContext.Sanitizers, logger: LoggerAccess, @@ -114,16 +119,14 @@ private class TelemetryReporter( unsanitizedReport: Report, ifVerbose: Boolean, ): ju.Optional[Path] = { - if (telemetryLevel() == TelemetryLevel.Full) { - val report = createSanitizedReport(unsanitizedReport) - if (report.text.isDefined || report.error.isDefined) - client.sendErrorReport(report) - else - logger.info( - "Skipped reporting remotely unmeaningful report, no context or error, reportId=" + - unsanitizedReport.id.orElse("null") - ) - } + val report = createSanitizedReport(unsanitizedReport) + if (report.text.isDefined || report.error.isDefined) + client.sendErrorReport(report) + else + logger.info( + "Skipped reporting remotely unmeaningful report, no context or error, reportId=" + + unsanitizedReport.id.orElse("null") + ) ju.Optional.empty() } } diff --git a/metals/src/main/scala/scala/meta/metals/Main.scala b/metals/src/main/scala/scala/meta/metals/Main.scala index 3b5e1e422cc..309b66d79e4 100644 --- a/metals/src/main/scala/scala/meta/metals/Main.scala +++ b/metals/src/main/scala/scala/meta/metals/Main.scala @@ -10,9 +10,6 @@ import scala.meta.internal.metals.ScalaVersions import scala.meta.internal.metals.Trace import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.metals.logging.MetalsLogger -import scala.meta.internal.telemetry.CrashReport -import scala.meta.internal.telemetry.ExceptionSummary -import scala.meta.internal.telemetry.TelemetryClient import org.eclipse.lsp4j.jsonrpc.Launcher @@ -66,7 +63,6 @@ object Main { launcher.startListening().get() } catch { case NonFatal(e) => - trySendCrashReport(e, server, ec) e.printStackTrace(systemOut) sys.exit(1) } finally { @@ -77,26 +73,4 @@ object Main { sys.exit(0) } } - - private def trySendCrashReport( - error: Throwable, - server: MetalsLanguageServer, - ec: ExecutionContext, - ): Unit = try { - val telemetryLevel = server.getTelemetryLevel() - if (telemetryLevel.enabled) { - val telemetry = new TelemetryClient(() => telemetryLevel)(ec) - telemetry.sendCrashReport( - CrashReport( - error = ExceptionSummary.from(error, identity), - componentName = this.getClass().getName(), - componentVersion = Some(BuildInfo.metalsVersion), - ) - ) - } - } catch { - case err: Throwable => - System.err.println(s"Failed to send crash report, $err") - } - } diff --git a/mtags-shared/src/main/scala/scala/meta/internal/metals/ProvidedReportContexts.scala b/mtags-shared/src/main/scala/scala/meta/internal/metals/ProvidedReportContexts.scala index 9d40f1cee4b..0dc3668f710 100644 --- a/mtags-shared/src/main/scala/scala/meta/internal/metals/ProvidedReportContexts.scala +++ b/mtags-shared/src/main/scala/scala/meta/internal/metals/ProvidedReportContexts.scala @@ -168,7 +168,7 @@ object StdReportContext { } /** - * Fan-out report context delegating reporting to all underlyin reporters of given type. + * Fan-out report context delegating reporting to all underlying reporters of given type. */ class MirroredReportContext(primary: ReportContext, auxilary: ReportContext*) extends ReportContext { diff --git a/telemetry-interfaces/src/main/scala/scala/meta/internal/telemetry/TelemetryService.scala b/telemetry-interfaces/src/main/scala/scala/meta/internal/telemetry/TelemetryService.scala index c6434f0b043..9ffc2c6d5fa 100644 --- a/telemetry-interfaces/src/main/scala/scala/meta/internal/telemetry/TelemetryService.scala +++ b/telemetry-interfaces/src/main/scala/scala/meta/internal/telemetry/TelemetryService.scala @@ -17,13 +17,8 @@ object TelemetryService { "POST", "/v1/telemetry/sendErrorReport", ) - val sendCrashReportEndpoint = new FireAndForgetEndpoint[CrashReport]( - "POST", - "/v1/telemetry/sendCrashReport", - ) } trait TelemetryService { def sendErrorReport(errorReport: ErrorReport): Unit - def sendCrashReport(crashReport: CrashReport): Unit } diff --git a/tests/unit/src/test/scala/tests/telemetry/TelemetryReporterSuite.scala b/tests/unit/src/test/scala/tests/telemetry/TelemetryReporterSuite.scala index 47ec4b0f320..687fa535bae 100644 --- a/tests/unit/src/test/scala/tests/telemetry/TelemetryReporterSuite.scala +++ b/tests/unit/src/test/scala/tests/telemetry/TelemetryReporterSuite.scala @@ -26,7 +26,7 @@ class TelemetryReporterSuite extends BaseSuite { def simpleReport(id: String): Report = StandardReport( name = "name", text = "text", - shortSummary = "sumamry", + shortSummary = "summmary", path = None, id = Some(id), error = Some(new RuntimeException("A", new NullPointerException())), @@ -80,13 +80,65 @@ class TelemetryReporterSuite extends BaseSuite { } { val createdReport = simpleReport(reporterCtx.toString()) reporter.incognito.create(createdReport) - Thread.sleep(5000) // wait for the server to receive the event + Thread.sleep(1000) // wait for the server to receive the event val received = ctx.errors.filter(_.id == createdReport.id.asScala) assert(received.nonEmpty, "Not received matching id") assert(received.size == 1, "Found more then 1 received event") } } finally server.stop() } + + locally { + implicit val ctx = new MockTelemetryServer.Context() + val server = MockTelemetryServer("127.0.0.1", 8081) + + def testCase(level: metals.TelemetryLevel, expected: Set[String]): Unit = + test( + s"Telemetry level: ${level} sends telemetry for ${expected.mkString("(", ", ", ")")}" + ) { + ctx.errors.clear() + + try { + server.start() + val serverEndpoint = MockTelemetryServer.address(server) + for { + reporterCtx <- Seq( + SampleReports.metalsLspReport(), + SampleReports.scalaPresentationCompilerReport(), + ).map(_.reporterContext) + reporter = new TelemetryReportContext( + telemetryClientConfig = TelemetryClient.Config.default + .copy(serverHost = serverEndpoint), + telemetryLevel = () => level, + reporterContext = () => reporterCtx, + sanitizers = new TelemetryReportContext.Sanitizers( + None, + Some(metals.ScalametaSourceCodeTransformer), + ), + ) + } { + + reporter.incognito.create(simpleReport("incognito")) + reporter.bloop.create(simpleReport("bloop")) + reporter.unsanitized.create(simpleReport("unsanitized")) + + def received = ctx.errors.map(_.id.get).toSet + Thread.sleep(1000) + assertEquals(received, expected) + } + } finally { + server.stop() + } + } + + testCase(metals.TelemetryLevel.Off, Set()) + testCase(metals.TelemetryLevel.Anonymous, Set("incognito", "bloop")) + testCase( + metals.TelemetryLevel.Full, + Set("incognito", "bloop", "unsanitized"), + ) + } + } object MockTelemetryServer { @@ -97,8 +149,7 @@ object MockTelemetryServer { import io.undertow.util.Headers case class Context( - errors: mutable.ListBuffer[ErrorReport] = mutable.ListBuffer.empty, - crashes: mutable.ListBuffer[CrashReport] = mutable.ListBuffer.empty, + errors: mutable.ListBuffer[ErrorReport] = mutable.ListBuffer.empty ) def apply( @@ -112,10 +163,6 @@ object MockTelemetryServer { TelemetryService.sendErrorReportEndpoint, _.errors, ) - .withEndpoint( - TelemetryService.sendCrashReportEndpoint, - _.crashes, - ) Undertow.builder .addHttpListener(port, host) .setHandler(baseHandler)