From 42f027f9328f2a465b596144b12e3aa8db8239b3 Mon Sep 17 00:00:00 2001 From: Alex Bishop Date: Thu, 15 Oct 2020 16:32:17 +0100 Subject: [PATCH] PP-6501 Add structured logging for authorisation request data Add structured logging containing information about the authorisation data sent a gateway when we authorise (in addition to the usual stuff available on the charge). New fields are "billing_address" (except sandbox), "data_for_3ds" (except sandbox and Stripe), "data_for_3ds2" (ePDQ only) and "worldpay_3ds_flex_device_data_collection_result" (Worldpay only) and have values of either true or false. The information will match the prose logging introduced in commit 5923a5eec34351875012343a1bdc29574e2eae29 with the same caveats. --- ...sationRequestSummaryStructuredLogging.java | 67 +++++++++++++ .../service/CardAuthoriseService.java | 14 ++- ...onRequestSummaryStructuredLoggingTest.java | 93 +++++++++++++++++++ .../service/CardAuthoriseServiceTest.java | 7 +- 4 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 src/main/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLogging.java create mode 100644 src/test/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLoggingTest.java diff --git a/src/main/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLogging.java b/src/main/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLogging.java new file mode 100644 index 0000000000..37a30330e8 --- /dev/null +++ b/src/main/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLogging.java @@ -0,0 +1,67 @@ +package uk.gov.pay.connector.gateway.util; + +import net.logstash.logback.argument.StructuredArgument; +import uk.gov.pay.connector.gateway.model.AuthorisationRequestSummary; + +import java.util.ArrayList; + +import static net.logstash.logback.argument.StructuredArguments.kv; + +public class AuthorisationRequestSummaryStructuredLogging { + + public static final String BILLING_ADDRESS = "billing_address"; + public static final String DATA_FOR_3DS = "data_for_3ds"; + public static final String DATA_FOR_3DS2 = "data_for_3ds2"; + public static final String WORLDPAY_3DS_FLEX_DEVICE_DATA_COLLECTION_RESULT = "worldpay_3ds_flex_device_data_collection_result"; + + public StructuredArgument[] createArgs(AuthorisationRequestSummary authorisationRequestSummary) { + var structuredArguments = new ArrayList(); + + switch (authorisationRequestSummary.billingAddress()) { + case PRESENT: + structuredArguments.add(kv(BILLING_ADDRESS, true)); + break; + case NOT_PRESENT: + structuredArguments.add(kv(BILLING_ADDRESS, false)); + break; + default: + break; + } + + switch (authorisationRequestSummary.dataFor3ds()) { + case PRESENT: + structuredArguments.add(kv(DATA_FOR_3DS, true)); + break; + case NOT_PRESENT: + structuredArguments.add(kv(DATA_FOR_3DS, false)); + break; + default: + break; + } + + switch (authorisationRequestSummary.dataFor3ds2()) { + case PRESENT: + structuredArguments.add(kv(DATA_FOR_3DS2, true)); + break; + case NOT_PRESENT: + structuredArguments.add(kv(DATA_FOR_3DS2, false)); + break; + default: + break; + } + + switch (authorisationRequestSummary.deviceDataCollectionResult()) { + case PRESENT: + structuredArguments.add(kv(WORLDPAY_3DS_FLEX_DEVICE_DATA_COLLECTION_RESULT, true)); + break; + case NOT_PRESENT: + structuredArguments.add(kv(WORLDPAY_3DS_FLEX_DEVICE_DATA_COLLECTION_RESULT, false)); + break; + default: + break; + } + + return structuredArguments.toArray(new StructuredArgument[structuredArguments.size()]); + } + +} diff --git a/src/main/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseService.java b/src/main/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseService.java index 1c74f57d77..bc8b715c8e 100644 --- a/src/main/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseService.java +++ b/src/main/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseService.java @@ -3,6 +3,7 @@ import com.codahale.metrics.MetricRegistry; import com.google.inject.persist.Transactional; import io.dropwizard.setup.Environment; +import org.apache.commons.lang3.ArrayUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import uk.gov.pay.connector.cardtype.dao.CardTypeDao; @@ -23,10 +24,12 @@ import uk.gov.pay.connector.gateway.model.response.BaseAuthoriseResponse; import uk.gov.pay.connector.gateway.model.response.GatewayResponse; import uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStringifier; +import uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStructuredLogging; import uk.gov.pay.connector.paymentprocessor.api.AuthorisationResponse; import uk.gov.pay.connector.paymentprocessor.model.OperationType; import javax.inject.Inject; +import java.util.Locale; import java.util.Optional; import static uk.gov.pay.connector.charge.util.CorporateCardSurchargeCalculator.getCorporateCardSurchargeFor; @@ -39,6 +42,7 @@ public class CardAuthoriseService { private final ChargeService chargeService; private final PaymentProviders providers; private final AuthorisationRequestSummaryStringifier authorisationRequestSummaryStringifier; + private final AuthorisationRequestSummaryStructuredLogging authorisationRequestSummaryStructuredLogging; private final MetricRegistry metricRegistry; private final Logger logger = LoggerFactory.getLogger(getClass()); @@ -48,12 +52,14 @@ public CardAuthoriseService(CardTypeDao cardTypeDao, CardAuthoriseBaseService cardAuthoriseBaseService, ChargeService chargeService, AuthorisationRequestSummaryStringifier authorisationRequestSummaryStringifier, + AuthorisationRequestSummaryStructuredLogging authorisationRequestSummaryStructuredLogging, Environment environment) { this.providers = providers; this.cardAuthoriseBaseService = cardAuthoriseBaseService; this.chargeService = chargeService; this.metricRegistry = environment.metrics(); this.authorisationRequestSummaryStringifier = authorisationRequestSummaryStringifier; + this.authorisationRequestSummaryStructuredLogging = authorisationRequestSummaryStructuredLogging; this.cardTypeDao = cardTypeDao; } @@ -93,13 +99,19 @@ public AuthorisationResponse doAuthorise(String chargeId, AuthCardDetails authCa var authorisationRequestSummary = generateAuthorisationRequestSummary(charge, authCardDetails); // Used by Splunk saved search - logger.info("Authorisation{} for {} ({} {}) for {} ({}) - {} .'. {} -> {}", + var logMessage = String.format(Locale.UK, "Authorisation%s for %s (%s %s) for %s (%s) - %s .'. %s -> %s", authorisationRequestSummaryStringifier.stringify(authorisationRequestSummary), updatedCharge.getExternalId(), updatedCharge.getPaymentGatewayName().getName(), transactionId.orElse("missing transaction ID"), updatedCharge.getGatewayAccount().getAnalyticsId(), updatedCharge.getGatewayAccount().getId(), operationResponse, ChargeStatus.fromString(charge.getStatus()), newStatus); + var structuredLoggingArguments = ArrayUtils.addAll( + charge.getStructuredLoggingArgs(), + authorisationRequestSummaryStructuredLogging.createArgs(authorisationRequestSummary)); + + logger.info(logMessage, structuredLoggingArguments); + metricRegistry.counter(String.format( "gateway-operations.%s.%s.%s.authorise.%s.result.%s", updatedCharge.getGatewayAccount().getGatewayName(), diff --git a/src/test/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLoggingTest.java b/src/test/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLoggingTest.java new file mode 100644 index 0000000000..79675e695d --- /dev/null +++ b/src/test/java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLoggingTest.java @@ -0,0 +1,93 @@ +package uk.gov.pay.connector.gateway.util; + +import net.logstash.logback.argument.StructuredArgument; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import uk.gov.pay.connector.gateway.model.AuthorisationRequestSummary; + +import static net.logstash.logback.argument.StructuredArguments.kv; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.collection.ArrayMatching.arrayContaining; +import static org.hamcrest.collection.IsArrayWithSize.emptyArray; +import static org.mockito.BDDMockito.given; +import static uk.gov.pay.connector.gateway.model.AuthorisationRequestSummary.Presence.NOT_APPLICABLE; +import static uk.gov.pay.connector.gateway.model.AuthorisationRequestSummary.Presence.NOT_PRESENT; +import static uk.gov.pay.connector.gateway.model.AuthorisationRequestSummary.Presence.PRESENT; +import static uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStructuredLogging.BILLING_ADDRESS; +import static uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStructuredLogging.DATA_FOR_3DS; +import static uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStructuredLogging.DATA_FOR_3DS2; +import static uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStructuredLogging.WORLDPAY_3DS_FLEX_DEVICE_DATA_COLLECTION_RESULT; + +@ExtendWith(MockitoExtension.class) +class AuthorisationRequestSummaryStructuredLoggingTest { + + @Mock private AuthorisationRequestSummary mockAuthorisationRequestSummary; + + private final AuthorisationRequestSummaryStructuredLogging structuredLogging = new AuthorisationRequestSummaryStructuredLogging(); + + @Test + void createArgsWithAllPresent() { + given(mockAuthorisationRequestSummary.billingAddress()).willReturn(PRESENT); + given(mockAuthorisationRequestSummary.dataFor3ds()).willReturn(PRESENT); + given(mockAuthorisationRequestSummary.dataFor3ds2()).willReturn(PRESENT); + given(mockAuthorisationRequestSummary.deviceDataCollectionResult()).willReturn(PRESENT); + + StructuredArgument[] result = structuredLogging.createArgs(mockAuthorisationRequestSummary); + + assertThat(result, arrayContaining( + kv(BILLING_ADDRESS, true), + kv(DATA_FOR_3DS, true), + kv(DATA_FOR_3DS2, true), + kv(WORLDPAY_3DS_FLEX_DEVICE_DATA_COLLECTION_RESULT, true) + )); + } + + @Test + void createArgsWithAllNotPresent() { + given(mockAuthorisationRequestSummary.billingAddress()).willReturn(NOT_PRESENT); + given(mockAuthorisationRequestSummary.dataFor3ds()).willReturn(NOT_PRESENT); + given(mockAuthorisationRequestSummary.dataFor3ds2()).willReturn(NOT_PRESENT); + given(mockAuthorisationRequestSummary.deviceDataCollectionResult()).willReturn(NOT_PRESENT); + + StructuredArgument[] result = structuredLogging.createArgs(mockAuthorisationRequestSummary); + + assertThat(result, is(arrayContaining( + kv(BILLING_ADDRESS, false), + kv(DATA_FOR_3DS, false), + kv(DATA_FOR_3DS2, false), + kv(WORLDPAY_3DS_FLEX_DEVICE_DATA_COLLECTION_RESULT, false) + ))); + } + + @Test + void createArgsWithAllNotApplicable() { + given(mockAuthorisationRequestSummary.billingAddress()).willReturn(NOT_APPLICABLE); + given(mockAuthorisationRequestSummary.dataFor3ds()).willReturn(NOT_APPLICABLE); + given(mockAuthorisationRequestSummary.dataFor3ds2()).willReturn(NOT_APPLICABLE); + given(mockAuthorisationRequestSummary.deviceDataCollectionResult()).willReturn(NOT_APPLICABLE); + + StructuredArgument[] result = structuredLogging.createArgs(mockAuthorisationRequestSummary); + + assertThat(result, is(emptyArray())); + } + + @Test + void createArgsWithMixture() { + given(mockAuthorisationRequestSummary.billingAddress()).willReturn(PRESENT); + given(mockAuthorisationRequestSummary.dataFor3ds()).willReturn(PRESENT); + given(mockAuthorisationRequestSummary.dataFor3ds2()).willReturn(NOT_APPLICABLE); + given(mockAuthorisationRequestSummary.deviceDataCollectionResult()).willReturn(NOT_PRESENT); + + StructuredArgument[] result = structuredLogging.createArgs(mockAuthorisationRequestSummary); + + assertThat(result, is(arrayContaining( + kv(BILLING_ADDRESS, true), + kv(DATA_FOR_3DS, true), + kv(WORLDPAY_3DS_FLEX_DEVICE_DATA_COLLECTION_RESULT, false) + ))); + } + +} diff --git a/src/test/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseServiceTest.java b/src/test/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseServiceTest.java index 27f9d6064d..6ee95488a2 100644 --- a/src/test/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseServiceTest.java +++ b/src/test/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseServiceTest.java @@ -40,6 +40,7 @@ import uk.gov.pay.connector.gateway.model.response.GatewayResponse.GatewayResponseBuilder; import uk.gov.pay.connector.gateway.sandbox.SandboxAuthorisationRequestSummary; import uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStringifier; +import uk.gov.pay.connector.gateway.util.AuthorisationRequestSummaryStructuredLogging; import uk.gov.pay.connector.gateway.worldpay.Worldpay3dsFlexRequiredParams; import uk.gov.pay.connector.gateway.worldpay.Worldpay3dsRequiredParams; import uk.gov.pay.connector.gateway.worldpay.WorldpayOrderStatusResponse; @@ -117,7 +118,7 @@ public class CardAuthoriseServiceTest extends CardServiceTest { @Mock private EventService mockEventService; - + @Mock private RefundService mockRefundService; @@ -126,6 +127,9 @@ public class CardAuthoriseServiceTest extends CardServiceTest { @Mock private AuthorisationRequestSummaryStringifier mockAuthorisationRequestSummaryStringifier; + + @Mock + private AuthorisationRequestSummaryStructuredLogging mockAuthorisationRequestSummaryStructuredLogging; private CardAuthoriseService cardAuthorisationService; @@ -149,6 +153,7 @@ public void setUpCardAuthorisationService() { cardAuthoriseBaseService, chargeService, mockAuthorisationRequestSummaryStringifier, + mockAuthorisationRequestSummaryStructuredLogging, mockEnvironment); }