add system config options for success / failure levels to write blocking / non-blocking activations to db (#5169)

* add config option to disable successful non-blocking activation writes to db

* fix check

* feedback

* fix tests

* rebase

* feedback

* update config precedence

* Apply suggestions from code review

Co-authored-by: rodric rabbah <[email protected]>

Co-authored-by: Brendan Doyle <[email protected]>
Co-authored-by: rodric rabbah <[email protected]>

Author: 3 people

common/scala/src/main/resources/application.conf

@@ -438,10 +438,19 @@ whisk {
         #           uniqueName + displayName    253 (max pod name length in Kube)
         serdes-overhead = 6068 // 3034 bytes of metadata * 2 for extra headroom
 
+        # DEPRECATED, use store-blocking-result-level
         # Disables database store for blocking + successful activations
         # invocations made with `X-OW-EXTRA-LOGGING: on` header, will force the activation to be stored
         disable-store-result = false
 
+        # Result level to store in db for blocking activations (STORE_ALWAYS, STORE_FAILURES, STORE_FAILURES_NOT_APPLICATION_ERRORS)
+        # invocations made with `X-OW-EXTRA-LOGGING: on` header, will force the activation to be stored
+        store-blocking-result-level = "STORE_ALWAYS"
+
+        # Result level to store in db for non-blocking activations (STORE_ALWAYS, STORE_FAILURES, STORE_FAILURES_NOT_APPLICATION_ERRORS)
+        # invocations made with `X-OW-EXTRA-LOGGING: on` header, will force the activation to be stored
+        store-non-blocking-result-level = "STORE_ALWAYS"
+
         # Enable metadata logging of activations not stored in the database
         unstored-logs-enabled = false
     }

common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala

@@ -286,7 +286,10 @@ object ConfigKeys {
   val sharedPackageExecuteOnly = s"whisk.shared-packages-execute-only"
   val swaggerUi = "whisk.swagger-ui"
 
+  /* DEPRECATED: disableStoreResult is deprecated for storeBlockingResultLevel */
   val disableStoreResult = s"$activation.disable-store-result"
+  val storeBlockingResultLevel = s"$activation.store-blocking-result-level"
+  val storeNonBlockingResultLevel = s"$activation.store-non-blocking-result-level"
   val unstoredLogsEnabled = s"$activation.unstored-logs-enabled"
 
   val apacheClientConfig = "whisk.apache-client"

common/scala/src/main/scala/org/apache/openwhisk/core/database/ActivationStore.scala

@@ -18,7 +18,6 @@
 package org.apache.openwhisk.core.database
 
 import java.time.Instant
-
 import akka.actor.ActorSystem
 import akka.http.scaladsl.model.HttpRequest
 import spray.json.JsObject
@@ -33,32 +32,52 @@ import scala.concurrent.Future
 case class UserContext(user: Identity, request: HttpRequest = HttpRequest())
 
 trait ActivationStore {
+  val logging: Logging
 
+  /* DEPRECATED: disableStoreResult config is now deprecated replaced with blocking activation store level (storeBlockingResultLevel) */
   protected val disableStoreResultConfig = loadConfigOrThrow[Boolean](ConfigKeys.disableStoreResult)
+  protected val storeBlockingResultLevelConfig = {
+    try {
+      ActivationStoreLevel.valueOf(loadConfigOrThrow[String](ConfigKeys.storeBlockingResultLevel))
+    } catch {
+      case _: Exception =>
+        val disableStoreResultConfig = loadConfigOrThrow[Boolean](ConfigKeys.disableStoreResult)
+        logging.warn(
+          this,
+          s"The config ${ConfigKeys.disableStoreResult} being used is deprecated. Please use the replacement config ${ConfigKeys.storeBlockingResultLevel}")
+        if (disableStoreResultConfig) ActivationStoreLevel.STORE_FAILURES else ActivationStoreLevel.STORE_ALWAYS
+    }
+  }
+  protected val storeNonBlockingResultLevelConfig =
+    ActivationStoreLevel.valueOf(loadConfigOrThrow[String](ConfigKeys.storeNonBlockingResultLevel))
   protected val unstoredLogsEnabledConfig = loadConfigOrThrow[Boolean](ConfigKeys.unstoredLogsEnabled)
 
   /**
    * Checks if an activation should be stored in database and stores it.
    *
    * @param activation activation to store
    * @param isBlockingActivation is activation blocking
+   * @param blockingStoreLevel do not store activation if successful and blocking
+   * @param nonBlockingStoreLevel do not store activation if successful and non-blocking
    * @param context user and request context
    * @param transid transaction ID for request
    * @param notifier cache change notifier
    * @return Future containing DocInfo related to stored activation
    */
   def storeAfterCheck(activation: WhiskActivation,
                       isBlockingActivation: Boolean,
-                      disableStore: Option[Boolean],
+                      blockingStoreLevel: Option[ActivationStoreLevel.Value],
+                      nonBlockingStoreLevel: Option[ActivationStoreLevel.Value],
                       context: UserContext)(implicit transid: TransactionId,
                                             notifier: Option[CacheChangeNotification],
                                             logging: Logging): Future[DocInfo] = {
     if (context.user.limits.storeActivations.getOrElse(true) &&
         shouldStoreActivation(
-          activation.response.isSuccess,
+          activation.response,
           isBlockingActivation,
           transid.meta.extraLogging,
-          disableStore.getOrElse(disableStoreResultConfig))) {
+          blockingStoreLevel.getOrElse(storeBlockingResultLevelConfig),
+          nonBlockingStoreLevel.getOrElse(storeNonBlockingResultLevelConfig))) {
 
       store(activation, context)
     } else {
@@ -183,17 +202,29 @@ trait ActivationStore {
    * - an activation in debug mode
    * - activation stores is not disabled via a configuration parameter
    *
-   * @param isSuccess is successful activation
+   * @param activationResponse to check
    * @param isBlocking is blocking activation
    * @param debugMode is logging header set to "on" for the invocation
-   * @param disableStore is disable store configured
+   * @param blockingStoreLevel level of activation status to store for blocking invocations
+   * @param nonBlockingStoreLevel level of activation status to store for blocking invocations
    * @return Should the activation be stored to the database
    */
-  private def shouldStoreActivation(isSuccess: Boolean,
+  private def shouldStoreActivation(activationResponse: ActivationResponse,
                                     isBlocking: Boolean,
                                     debugMode: Boolean,
-                                    disableStore: Boolean): Boolean = {
-    !isSuccess || !isBlocking || debugMode || !disableStore
+                                    blockingStoreLevel: ActivationStoreLevel.Value,
+                                    nonBlockingStoreLevel: ActivationStoreLevel.Value): Boolean = {
+    def shouldStoreOnLevel(storageLevel: ActivationStoreLevel.Value): Boolean = {
+      storageLevel match {
+        case ActivationStoreLevel.STORE_ALWAYS   => true
+        case ActivationStoreLevel.STORE_FAILURES => !activationResponse.isSuccess
+        case ActivationStoreLevel.STORE_FAILURES_NOT_APPLICATION_ERRORS =>
+          activationResponse.isContainerError || activationResponse.isWhiskError
+      }
+    }
+
+    debugMode || (isBlocking && shouldStoreOnLevel(blockingStoreLevel)) || (!isBlocking && shouldStoreOnLevel(
+      nonBlockingStoreLevel))
   }
 }
 

common/scala/src/main/scala/org/apache/openwhisk/core/database/ActivationStoreLevel.scala

@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.openwhisk.core.database
+
+object ActivationStoreLevel extends Enumeration {
+  type ActivationStoreLevel = Value
+  val STORE_ALWAYS, STORE_FAILURES, STORE_FAILURES_NOT_APPLICATION_ERRORS = Value
+
+  def valueOf(value: String): Value = {
+    values
+      .find(_.toString == value.toUpperCase())
+      .getOrElse(throw new IllegalArgumentException(s"Invalid log level: $value"))
+  }
+}

common/scala/src/main/scala/org/apache/openwhisk/core/database/ArtifactActivationStore.scala

@@ -27,7 +27,7 @@ import org.apache.openwhisk.core.entity._
 import scala.concurrent.Future
 import scala.util.{Failure, Success}
 
-class ArtifactActivationStore(actorSystem: ActorSystem, logging: Logging) extends ActivationStore {
+class ArtifactActivationStore(actorSystem: ActorSystem, override val logging: Logging) extends ActivationStore {
 
   implicit val executionContext = actorSystem.dispatcher
 

common/scala/src/main/scala/org/apache/openwhisk/core/database/elasticsearch/ElasticSearchActivationStore.scala

@@ -57,7 +57,7 @@ case class ElasticSearchActivationStoreConfig(protocol: String,
 class ElasticSearchActivationStore(
   httpFlow: Option[Flow[(HttpRequest, Promise[HttpResponse]), (Try[HttpResponse], Promise[HttpResponse]), Any]] = None,
   elasticSearchConfig: ElasticSearchActivationStoreConfig,
-  useBatching: Boolean = false)(implicit actorSystem: ActorSystem, logging: Logging)
+  useBatching: Boolean = false)(implicit actorSystem: ActorSystem, override val logging: Logging)
     extends ActivationStore {
 
   import com.sksamuel.elastic4s.http.ElasticDsl._

common/scala/src/main/scala/org/apache/openwhisk/core/database/memory/NoopActivationStore.scala

@@ -18,21 +18,16 @@
 package org.apache.openwhisk.core.database.memory
 
 import java.time.Instant
-
 import akka.actor.ActorSystem
-import org.apache.openwhisk.common.{Logging, TransactionId, WhiskInstants}
-import org.apache.openwhisk.core.database.{
-  ActivationStore,
-  ActivationStoreProvider,
-  CacheChangeNotification,
-  UserContext
-}
+import org.apache.openwhisk.common.{Logging, PrintStreamLogging, TransactionId, WhiskInstants}
+import org.apache.openwhisk.core.database.{ActivationStore, ActivationStoreProvider, CacheChangeNotification, UserContext}
 import org.apache.openwhisk.core.entity.{ActivationId, DocInfo, EntityName, EntityPath, Subject, WhiskActivation}
 import spray.json.{JsNumber, JsObject}
 
 import scala.concurrent.Future
 
 object NoopActivationStore extends ActivationStore with WhiskInstants {
+  override val logging = new PrintStreamLogging()
   private val emptyInfo = DocInfo("foo")
   private val emptyCount = JsObject("activations" -> JsNumber(0))
   private val dummyActivation = WhiskActivation(

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Triggers.scala

@@ -161,7 +161,7 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
                     triggerActivation
                 }
                 .map { activation =>
-                  activationStore.storeAfterCheck(activation, false, None, context)
+                  activationStore.storeAfterCheck(activation, false, None, None, context)
                 }
 
               respondWithActivationIdHeader(triggerActivationId) {

core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/PrimitiveActions.scala

@@ -600,7 +600,10 @@ protected[actions] trait PrimitiveActions {
       }
     }
 
-    activationStore.storeAfterCheck(activation, blockingComposition, None, context)(transid, notifier = None, logging)
+    activationStore.storeAfterCheck(activation, blockingComposition, None, None, context)(
+      transid,
+      notifier = None,
+      logging)
 
     activation
   }

core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/SequenceActions.scala

@@ -181,7 +181,7 @@ protected[actions] trait SequenceActions {
             }
           }
 
-          activationStore.storeAfterCheck(seqActivation, blockingSequence, None, context)(
+          activationStore.storeAfterCheck(seqActivation, blockingSequence, None, None, context)(
             transid,
             notifier = None,
             logging)