Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PP-6501 Add structured logging for authorisation request data #2618

Merged
merged 1 commit into from Oct 15, 2020
Merged

PP-6501 Add structured logging for authorisation request data #2618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 67 additions & 0 deletions .../java/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLogging.java
View file
Open in desktop
@@ -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<StructuredArgument>();

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()]);
}

}
14 changes: 13 additions & 1 deletion src/main/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseService.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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());

Expand All @@ -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;
}

Expand Down Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noice

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(),
Expand Down
93 changes: 93 additions & 0 deletions ...a/uk/gov/pay/connector/gateway/util/AuthorisationRequestSummaryStructuredLoggingTest.java
View file
Open in desktop
@@ -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)
)));
}

}
7 changes: 6 additions & 1 deletion src/test/java/uk/gov/pay/connector/paymentprocessor/service/CardAuthoriseServiceTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -117,7 +118,7 @@ public class CardAuthoriseServiceTest extends CardServiceTest {

@Mock
private EventService mockEventService;

@Mock
private RefundService mockRefundService;

Expand All @@ -126,6 +127,9 @@ public class CardAuthoriseServiceTest extends CardServiceTest {

@Mock
private AuthorisationRequestSummaryStringifier mockAuthorisationRequestSummaryStringifier;

@Mock
private AuthorisationRequestSummaryStructuredLogging mockAuthorisationRequestSummaryStructuredLogging;

private CardAuthoriseService cardAuthorisationService;

Expand All @@ -149,6 +153,7 @@ public void setUpCardAuthorisationService() {
cardAuthoriseBaseService,
chargeService,
mockAuthorisationRequestSummaryStringifier,
mockAuthorisationRequestSummaryStructuredLogging,
mockEnvironment);
}

Expand Down