From 548aaf6e4bf0aaefc98eb184246972aed5461690 Mon Sep 17 00:00:00 2001 From: Piotr Limanowski Date: Tue, 3 Sep 2024 21:17:36 +0200 Subject: [PATCH] Remove body read timeout feature The goal of the feature was to prevent long body reads in GCP, this however does not prevent the slow incoming connection handling at the framework level. Therefore, as this adds unnecessary complexity with possible negative performance impact, the feature is removed. The configuration parameter is no longer used, but can remain as is. --- core/src/main/resources/reference.conf | 1 - .../Config.scala | 1 - .../Pipes.scala | 29 ------------------- .../Routes.scala | 4 +-- .../Run.scala | 1 - .../PipesSpec.scala | 22 -------------- .../RoutesSpec.scala | 4 +-- .../TestUtils.scala | 1 - kafka/src/it/resources/collector.hocon | 1 - .../KafkaConfigSpec.scala | 1 - .../collector-cookie-anonymous.hocon | 1 - .../collector-cookie-attributes-1.hocon | 1 - .../collector-cookie-attributes-2.hocon | 1 - .../resources/collector-cookie-domain.hocon | 1 - .../resources/collector-cookie-fallback.hocon | 1 - .../collector-cookie-no-domain.hocon | 1 - .../it/resources/collector-custom-paths.hocon | 1 - .../collector-doNotTrackCookie-disabled.hocon | 1 - .../collector-doNotTrackCookie-enabled.hocon | 1 - kinesis/src/it/resources/collector.hocon | 1 - .../sinks/KinesisConfigSpec.scala | 1 - .../NsqConfigSpec.scala | 1 - pubsub/src/it/resources/collector.hocon | 1 - .../ConfigSpec.scala | 1 - .../SqsConfigSpec.scala | 1 - 25 files changed, 2 insertions(+), 78 deletions(-) delete mode 100644 core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Pipes.scala delete mode 100644 core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/PipesSpec.scala diff --git a/core/src/main/resources/reference.conf b/core/src/main/resources/reference.conf index 4ad566183..aa92fc969 100644 --- a/core/src/main/resources/reference.conf +++ b/core/src/main/resources/reference.conf @@ -95,7 +95,6 @@ maxConnections = 1024 idleTimeout = 610 seconds responseHeaderTimeout = 5 seconds - bodyReadTimeout = 1 second maxRequestLineLength = 20480 maxHeadersLength = 40960 } diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Config.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Config.scala index 5d2d335a3..8f9061018 100644 --- a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Config.scala +++ b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Config.scala @@ -161,7 +161,6 @@ object Config { maxConnections: Int, idleTimeout: FiniteDuration, responseHeaderTimeout: FiniteDuration, - bodyReadTimeout: FiniteDuration, maxRequestLineLength: Int, maxHeadersLength: Int ) diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Pipes.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Pipes.scala deleted file mode 100644 index 10f1b1aea..000000000 --- a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Pipes.scala +++ /dev/null @@ -1,29 +0,0 @@ -/** - * Copyright (c) 2013-present Snowplow Analytics Ltd. - * All rights reserved. - * - * This software is made available by Snowplow Analytics, Ltd., - * under the terms of the Snowplow Limited Use License Agreement, Version 1.0 - * located at https://docs.snowplow.io/limited-use-license-1.0 - * BY INSTALLING, DOWNLOADING, ACCESSING, USING OR DISTRIBUTING ANY PORTION - * OF THE SOFTWARE, YOU AGREE TO THE TERMS OF SUCH LICENSE AGREEMENT. - */ -package com.snowplowanalytics.snowplow.collector.core - -import scala.concurrent.duration.FiniteDuration -import cats.effect.Async -import fs2.{Pipe, Pull} - -object Pipes { - def timeoutOnIdle[F[_]: Async, A](duration: FiniteDuration): Pipe[F, A, A] = - _.pull.timed { timedPull => - def go(timedPull: Pull.Timed[F, A]): Pull[F, A, Unit] = - timedPull.timeout(duration) >> - timedPull.uncons.flatMap { - case Some((Right(elems), next)) => Pull.output(elems) >> go(next) - case _ => Pull.done - } - - go(timedPull) - }.stream -} diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Routes.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Routes.scala index e80f239ba..fd699a15a 100644 --- a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Routes.scala +++ b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Routes.scala @@ -10,7 +10,6 @@ */ package com.snowplowanalytics.snowplow.collector.core -import scala.concurrent.duration.FiniteDuration import cats.implicits._ import cats.effect.{Async, Sync} import org.http4s._ @@ -22,7 +21,6 @@ class Routes[F[_]: Async]( enableDefaultRedirect: Boolean, enableRootResponse: Boolean, enableCrossdomainTracking: Boolean, - bodyReadTimeout: FiniteDuration, service: IService[F] ) extends Http4sDsl[F] { @@ -51,7 +49,7 @@ class Routes[F[_]: Async]( case req @ POST -> Root / vendor / version => val path = service.determinePath(vendor, version) service.cookie( - body = req.bodyText.through(Pipes.timeoutOnIdle(bodyReadTimeout)).compile.string.map(Some(_)), + body = req.bodyText.compile.string.map(Some(_)), path = path, request = req, pixelExpected = false, diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Run.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Run.scala index 4553955ff..bcf0c60d9 100644 --- a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Run.scala +++ b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Run.scala @@ -98,7 +98,6 @@ object Run { config.enableDefaultRedirect, config.rootResponse.enabled, config.crossDomain.enabled, - config.networking.responseHeaderTimeout, collectorService ).value, if (config.ssl.enable) config.ssl.port else config.port, diff --git a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/PipesSpec.scala b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/PipesSpec.scala deleted file mode 100644 index bfd6803b1..000000000 --- a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/PipesSpec.scala +++ /dev/null @@ -1,22 +0,0 @@ -package com.snowplowanalytics.snowplow.collector.core - -import scala.concurrent.duration._ -import org.specs2.mutable.Specification -import cats.effect.IO -import cats.effect.unsafe.implicits.global -import fs2.Stream - -class PipesSpec extends Specification { - - "Pipes#timeoutOnIdle" should { - "allow terminating a stream early when idle" in { - Stream - .emits[IO, Int](Vector(1, 2, 3)) - .onComplete(Stream.empty[IO].delayBy(20.seconds)) - .through(Pipes.timeoutOnIdle(100.millis)) - .compile - .count - .unsafeRunSync() must beEqualTo(3) - } - } -} diff --git a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/RoutesSpec.scala b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/RoutesSpec.scala index 2df2627f8..d75249b8b 100644 --- a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/RoutesSpec.scala +++ b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/RoutesSpec.scala @@ -73,9 +73,7 @@ class RoutesSpec extends Specification { ) = { val service = new TestService() val routes = - new Routes(enabledDefaultRedirect, enableRootResponse, enableCrossdomainTracking, 500.millis, service) - .value - .orNotFound + new Routes(enabledDefaultRedirect, enableRootResponse, enableCrossdomainTracking, service).value.orNotFound val routesWithHsts = HttpServer.hstsMiddleware(Config.HSTS(enableHsts, 180.days), routes) (service, routesWithHsts) } diff --git a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/TestUtils.scala b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/TestUtils.scala index 1c0a5da72..e30ea43b4 100644 --- a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/TestUtils.scala +++ b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/TestUtils.scala @@ -121,7 +121,6 @@ object TestUtils { 1024, 610.seconds, 5.seconds, - 1.second, 20480, 40960 ), diff --git a/kafka/src/it/resources/collector.hocon b/kafka/src/it/resources/collector.hocon index e46b803ad..03b97888d 100644 --- a/kafka/src/it/resources/collector.hocon +++ b/kafka/src/it/resources/collector.hocon @@ -26,6 +26,5 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } } \ No newline at end of file diff --git a/kafka/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/KafkaConfigSpec.scala b/kafka/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/KafkaConfigSpec.scala index 3b402dafc..ba4593e60 100644 --- a/kafka/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/KafkaConfigSpec.scala +++ b/kafka/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/KafkaConfigSpec.scala @@ -172,7 +172,6 @@ object KafkaConfigSpec { maxConnections = 1024, idleTimeout = 610.seconds, responseHeaderTimeout = 5.seconds, - bodyReadTimeout = 1.second, maxRequestLineLength = 20480, maxHeadersLength = 40960 ), diff --git a/kinesis/src/it/resources/collector-cookie-anonymous.hocon b/kinesis/src/it/resources/collector-cookie-anonymous.hocon index 1b2ae929e..85c51eca6 100644 --- a/kinesis/src/it/resources/collector-cookie-anonymous.hocon +++ b/kinesis/src/it/resources/collector-cookie-anonymous.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "cookie": { diff --git a/kinesis/src/it/resources/collector-cookie-attributes-1.hocon b/kinesis/src/it/resources/collector-cookie-attributes-1.hocon index 7e316aecd..34d393521 100644 --- a/kinesis/src/it/resources/collector-cookie-attributes-1.hocon +++ b/kinesis/src/it/resources/collector-cookie-attributes-1.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "cookie": { diff --git a/kinesis/src/it/resources/collector-cookie-attributes-2.hocon b/kinesis/src/it/resources/collector-cookie-attributes-2.hocon index 1b2ae929e..85c51eca6 100644 --- a/kinesis/src/it/resources/collector-cookie-attributes-2.hocon +++ b/kinesis/src/it/resources/collector-cookie-attributes-2.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "cookie": { diff --git a/kinesis/src/it/resources/collector-cookie-domain.hocon b/kinesis/src/it/resources/collector-cookie-domain.hocon index 94298cb0d..1f2749e23 100644 --- a/kinesis/src/it/resources/collector-cookie-domain.hocon +++ b/kinesis/src/it/resources/collector-cookie-domain.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "cookie": { diff --git a/kinesis/src/it/resources/collector-cookie-fallback.hocon b/kinesis/src/it/resources/collector-cookie-fallback.hocon index 45d5786a2..17361150d 100644 --- a/kinesis/src/it/resources/collector-cookie-fallback.hocon +++ b/kinesis/src/it/resources/collector-cookie-fallback.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "cookie": { diff --git a/kinesis/src/it/resources/collector-cookie-no-domain.hocon b/kinesis/src/it/resources/collector-cookie-no-domain.hocon index 1b2ae929e..85c51eca6 100644 --- a/kinesis/src/it/resources/collector-cookie-no-domain.hocon +++ b/kinesis/src/it/resources/collector-cookie-no-domain.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "cookie": { diff --git a/kinesis/src/it/resources/collector-custom-paths.hocon b/kinesis/src/it/resources/collector-custom-paths.hocon index c100e92fc..cc4d7cc0c 100644 --- a/kinesis/src/it/resources/collector-custom-paths.hocon +++ b/kinesis/src/it/resources/collector-custom-paths.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } diff --git a/kinesis/src/it/resources/collector-doNotTrackCookie-disabled.hocon b/kinesis/src/it/resources/collector-doNotTrackCookie-disabled.hocon index 8e63d4ee2..4e1481b63 100644 --- a/kinesis/src/it/resources/collector-doNotTrackCookie-disabled.hocon +++ b/kinesis/src/it/resources/collector-doNotTrackCookie-disabled.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "doNotTrackCookie": { diff --git a/kinesis/src/it/resources/collector-doNotTrackCookie-enabled.hocon b/kinesis/src/it/resources/collector-doNotTrackCookie-enabled.hocon index db836b82f..32720eebf 100644 --- a/kinesis/src/it/resources/collector-doNotTrackCookie-enabled.hocon +++ b/kinesis/src/it/resources/collector-doNotTrackCookie-enabled.hocon @@ -33,7 +33,6 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } "doNotTrackCookie": { diff --git a/kinesis/src/it/resources/collector.hocon b/kinesis/src/it/resources/collector.hocon index 4040f1764..f3e058e8a 100644 --- a/kinesis/src/it/resources/collector.hocon +++ b/kinesis/src/it/resources/collector.hocon @@ -33,6 +33,5 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } } \ No newline at end of file diff --git a/kinesis/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/sinks/KinesisConfigSpec.scala b/kinesis/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/sinks/KinesisConfigSpec.scala index 1574ee9c5..2ac2b4c6f 100644 --- a/kinesis/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/sinks/KinesisConfigSpec.scala +++ b/kinesis/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/sinks/KinesisConfigSpec.scala @@ -126,7 +126,6 @@ object KinesisConfigSpec { maxConnections = 1024, idleTimeout = 610.seconds, responseHeaderTimeout = 5.seconds, - bodyReadTimeout = 1.second, maxRequestLineLength = 20480, maxHeadersLength = 40960 ), diff --git a/nsq/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/NsqConfigSpec.scala b/nsq/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/NsqConfigSpec.scala index 8cc536d12..f08badb6a 100644 --- a/nsq/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/NsqConfigSpec.scala +++ b/nsq/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/NsqConfigSpec.scala @@ -159,7 +159,6 @@ object NsqConfigSpec { maxConnections = 1024, idleTimeout = 610.seconds, responseHeaderTimeout = 5.seconds, - bodyReadTimeout = 1.second, maxRequestLineLength = 20480, maxHeadersLength = 40960 ), diff --git a/pubsub/src/it/resources/collector.hocon b/pubsub/src/it/resources/collector.hocon index 0439687df..89005cbed 100644 --- a/pubsub/src/it/resources/collector.hocon +++ b/pubsub/src/it/resources/collector.hocon @@ -18,6 +18,5 @@ collector { networking { responseHeaderTimeout = 10 seconds - bodyReadTimeout = 2 seconds } } \ No newline at end of file diff --git a/pubsub/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/ConfigSpec.scala b/pubsub/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/ConfigSpec.scala index 1392133de..e1d66d3fc 100644 --- a/pubsub/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/ConfigSpec.scala +++ b/pubsub/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/ConfigSpec.scala @@ -115,7 +115,6 @@ object ConfigSpec { maxConnections = 1024, idleTimeout = 610.seconds, responseHeaderTimeout = 5.seconds, - bodyReadTimeout = 1.second, maxRequestLineLength = 20480, maxHeadersLength = 40960 ), diff --git a/sqs/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/SqsConfigSpec.scala b/sqs/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/SqsConfigSpec.scala index 86a8a8d76..762be70ac 100644 --- a/sqs/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/SqsConfigSpec.scala +++ b/sqs/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/SqsConfigSpec.scala @@ -116,7 +116,6 @@ object SqsConfigSpec { maxConnections = 1024, idleTimeout = 610.seconds, responseHeaderTimeout = 5.seconds, - bodyReadTimeout = 1.second, maxRequestLineLength = 20480, maxHeadersLength = 40960 ),