Add detailed logging to measurement consumers service (#1569)

world-federation-of-advertisers · Apr 17, 2024 · 026155e · 026155e
1 parent 1f30099
commit 026155e
Show file tree

Hide file tree

Showing 10 changed files with 279 additions and 52 deletions.
diff --git a/...rg/wfanet/measurement/kingdom/deploy/gcloud/spanner/SpannerMeasurementConsumersService.kt b/...rg/wfanet/measurement/kingdom/deploy/gcloud/spanner/SpannerMeasurementConsumersService.kt
@@ -15,7 +15,6 @@
 package org.wfanet.measurement.kingdom.deploy.gcloud.spanner
 
 import io.grpc.Status
-import org.wfanet.measurement.common.grpc.failGrpc
 import org.wfanet.measurement.common.grpc.grpcRequire
 import org.wfanet.measurement.common.identity.ExternalId
 import org.wfanet.measurement.common.identity.IdGenerator
@@ -78,12 +77,12 @@ class SpannerMeasurementConsumersService(
   override suspend fun getMeasurementConsumer(
     request: GetMeasurementConsumerRequest
   ): MeasurementConsumer {
+    val externalMeasurementConsumerId = ExternalId(request.externalMeasurementConsumerId)
     return MeasurementConsumerReader()
-      .readByExternalMeasurementConsumerId(
-        client.singleUse(),
-        ExternalId(request.externalMeasurementConsumerId),
-      )
-      ?.measurementConsumer ?: failGrpc(Status.NOT_FOUND) { "MeasurementConsumer not found" }
+      .readByExternalMeasurementConsumerId(client.singleUse(), externalMeasurementConsumerId)
+      ?.measurementConsumer
+      ?: throw MeasurementConsumerNotFoundException(externalMeasurementConsumerId)
+        .asStatusRuntimeException(Status.Code.NOT_FOUND, "MeasurementConsumer not found")
   }
 
   override suspend fun addMeasurementConsumerOwner(

diff --git a/src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/BUILD.bazel b/src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/BUILD.bazel
@@ -288,6 +288,7 @@ kt_jvm_library(
         "//src/main/kotlin/org/wfanet/measurement/api/v2alpha:packed_messages",
         "//src/main/kotlin/org/wfanet/measurement/api/v2alpha:principal_server_interceptor",
         "//src/main/kotlin/org/wfanet/measurement/api/v2alpha:resource_key",
+        "//src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha:internal_status_conversion",
         "//src/main/proto/wfa/measurement/api/v2alpha:crypto_kt_jvm_proto",
         "//src/main/proto/wfa/measurement/api/v2alpha:measurement_consumer_kt_jvm_proto",
         "//src/main/proto/wfa/measurement/api/v2alpha:measurement_consumers_service_kt_jvm_grpc_proto",

diff --git a/...ain/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/InternalStatusConversion.kt b/...ain/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/InternalStatusConversion.kt
@@ -86,7 +86,7 @@ fun Status.toExternalStatusRuntimeException(
                 )
               )
               .toName()
-          put("measurement_consumer", measurementConsumerName)
+          put("measurementConsumer", measurementConsumerName)
           errorMessage = "MeasurementConsumer $measurementConsumerName not found."
         }
         ErrorCode.DATA_PROVIDER_NOT_FOUND -> {
@@ -302,7 +302,7 @@ fun Status.toExternalStatusRuntimeException(
               .toActivationState()
               .toString()
           put("account", accountName)
-          put("account_activation_state", accountActivationState)
+          put("accountActivationState", accountActivationState)
           errorMessage =
             "Account $accountName is in illegal activation state: $accountActivationState."
         }

diff --git a/.../kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementConsumersService.kt b/.../kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementConsumersService.kt
@@ -101,18 +101,13 @@ class MeasurementConsumersService(
     val internalMeasurementConsumer =
       try {
         internalClient.createMeasurementConsumer(createRequest)
-      } catch (ex: StatusException) {
-        when (ex.status.code) {
-          Status.Code.INVALID_ARGUMENT ->
-            failGrpc(Status.INVALID_ARGUMENT, ex) { "Required field unspecified or invalid." }
-          Status.Code.PERMISSION_DENIED ->
-            failGrpc(Status.PERMISSION_DENIED, ex) {
-              "Measurement Consumer creation token is not valid."
-            }
-          Status.Code.FAILED_PRECONDITION ->
-            failGrpc(Status.FAILED_PRECONDITION, ex) { "Account not found or inactivated." }
-          else -> failGrpc(Status.UNKNOWN, ex) { "Unknown exception." }
-        }
+      } catch (e: StatusException) {
+        throw when (e.status.code) {
+          Status.Code.INVALID_ARGUMENT -> Status.INVALID_ARGUMENT
+          Status.Code.PERMISSION_DENIED -> Status.PERMISSION_DENIED
+          Status.Code.FAILED_PRECONDITION -> Status.FAILED_PRECONDITION
+          else -> Status.UNKNOWN
+        }.toExternalStatusRuntimeException(e)
       }
 
     return internalMeasurementConsumer.toMeasurementConsumer()
@@ -152,12 +147,11 @@ class MeasurementConsumersService(
     val internalResponse =
       try {
         internalClient.getMeasurementConsumer(getRequest)
-      } catch (ex: StatusException) {
-        when (ex.status.code) {
-          Status.Code.NOT_FOUND ->
-            failGrpc(Status.NOT_FOUND, ex) { "MeasurementConsumer not found" }
-          else -> failGrpc(Status.UNKNOWN, ex) { "Unknown exception." }
-        }
+      } catch (e: StatusException) {
+        throw when (e.status.code) {
+          Status.Code.NOT_FOUND -> Status.NOT_FOUND
+          else -> Status.UNKNOWN
+        }.toExternalStatusRuntimeException(e)
       }
 
     return internalResponse.toMeasurementConsumer()
@@ -191,14 +185,12 @@ class MeasurementConsumersService(
     val internalMeasurementConsumer =
       try {
         internalClient.addMeasurementConsumerOwner(internalAddMeasurementConsumerOwnerRequest)
-      } catch (ex: StatusException) {
-        when (ex.status.code) {
-          Status.Code.FAILED_PRECONDITION ->
-            failGrpc(Status.FAILED_PRECONDITION, ex) { "Account not found." }
-          Status.Code.NOT_FOUND ->
-            failGrpc(Status.NOT_FOUND, ex) { "MeasurementConsumer not found." }
-          else -> failGrpc(Status.UNKNOWN, ex) { "Unknown exception." }
-        }
+      } catch (e: StatusException) {
+        throw when (e.status.code) {
+          Status.Code.FAILED_PRECONDITION -> Status.FAILED_PRECONDITION
+          Status.Code.NOT_FOUND -> Status.NOT_FOUND
+          else -> Status.UNKNOWN
+        }.toExternalStatusRuntimeException(e)
       }
     return internalMeasurementConsumer.toMeasurementConsumer()
   }
@@ -231,16 +223,12 @@ class MeasurementConsumersService(
     val internalMeasurementConsumer =
       try {
         internalClient.removeMeasurementConsumerOwner(internalRemoveMeasurementConsumerOwnerRequest)
-      } catch (ex: StatusException) {
-        when (ex.status.code) {
-          Status.Code.FAILED_PRECONDITION ->
-            failGrpc(Status.FAILED_PRECONDITION, ex) {
-              "Account not found or Account doesn't own the MeasurementConsumer."
-            }
-          Status.Code.NOT_FOUND ->
-            failGrpc(Status.NOT_FOUND, ex) { "MeasurementConsumer not found." }
-          else -> failGrpc(Status.UNKNOWN, ex) { "Unknown exception." }
-        }
+      } catch (e: StatusException) {
+        throw when (e.status.code) {
+          Status.Code.FAILED_PRECONDITION -> Status.FAILED_PRECONDITION
+          Status.Code.NOT_FOUND -> Status.NOT_FOUND
+          else -> Status.UNKNOWN
+        }.toExternalStatusRuntimeException(e)
       }
     return internalMeasurementConsumer.toMeasurementConsumer()
   }

diff --git a/src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/AccountsServiceTest.kt b/src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/AccountsServiceTest.kt
@@ -528,7 +528,7 @@ class AccountsServiceTest {
 
     assertThat(exception.status.code).isEqualTo(Status.Code.PERMISSION_DENIED)
     assertThat(exception.errorInfo?.metadataMap)
-      .containsEntry("account_activation_state", Account.ActivationState.ACTIVATED.toString())
+      .containsEntry("accountActivationState", Account.ActivationState.ACTIVATED.toString())
     assertThat(exception.errorInfo?.metadataMap).containsEntry("account", ACCOUNT_NAME)
   }
 

diff --git a/src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/ApiKeysServiceTest.kt b/src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/ApiKeysServiceTest.kt
@@ -207,7 +207,7 @@ class ApiKeysServiceTest {
 
     assertThat(exception.status.code).isEqualTo(Status.Code.NOT_FOUND)
     assertThat(exception.errorInfo?.metadataMap)
-      .containsEntry("measurement_consumer", MEASUREMENT_CONSUMER_NAME)
+      .containsEntry("measurementConsumer", MEASUREMENT_CONSUMER_NAME)
   }
 
   @Test

diff --git a/src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/BUILD.bazel b/src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/BUILD.bazel
@@ -236,6 +236,7 @@ kt_jvm_test(
     test_class = "org.wfanet.measurement.kingdom.service.api.v2alpha.MeasurementConsumersServiceTest",
     deps = [
         "//src/main/kotlin/org/wfanet/measurement/api/v2alpha/testing",
+        "//src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/common",
         "//src/main/proto/wfa/measurement/internal/kingdom:measurement_consumers_service_kt_jvm_grpc_proto",
         "@wfa_common_jvm//imports/java/com/google/common/truth",
         "@wfa_common_jvm//imports/java/com/google/common/truth/extensions/proto",