From d502907e774e4b7045f05ceb528d1cbfa7625e82 Mon Sep 17 00:00:00 2001 From: Shreyoshi Mahato <142842915+shreyoshi-mahato@users.noreply.github.com> Date: Thu, 14 Mar 2024 15:26:10 -0500 Subject: [PATCH] [VAULT-3057] Transaction Metrics Logging (#458) * added MetricsModule, NoopMetrics and DefaultMetrics implementation * fix : Unable to find resource: vgs/metrics.star and additionally there was no module for vgs/metrics/__init__.star found) * removed useStarlarkThread : true; because the StarlarkThread will be passed implicitly, and you don't need to pass it explicitly * moved the MetricsModule class inside the vgs.metrics folder * used Dict dict instead of Dict dict * Modules alphabetized * Transaction metrics keys shifted to starlarky instead of atlas-commons * removed the MetricsModule.ENABLE_DEFAULT_PROPERTY * fix : java.lang.IllegalArgumentException: MetricsModule expecting only 1 metrics provider of type LarkyMetrics * [VAULT-3077] Change the arguments of track from dictionary to list of parameters * fix : added tests for metrics track method * extra keys will be mapped to a **kwargs * removed unused imports * added few test and keys and starlark Int logic to Int * removed allowed values in metrics.track parameters * changed attributes to dict> * when psp not given its default value will be NOT_SET --- .../security/larky/ModuleSupplier.java | 12 +- .../modules/vgs/metrics/MetricsModule.java | 130 ++++++++++++++ .../constants/TransactionMetricsKeys.java | 12 ++ .../vgs/metrics/impl/DefaultMetrics.java | 44 +++++ .../modules/vgs/metrics/impl/NoopMetrics.java | 25 +++ .../modules/vgs/metrics/spi/LarkyMetrics.java | 20 +++ larky/src/main/resources/vgs/metrics.star | 25 +++ .../vgs/metrics/MetricsModuleSPITest.java | 129 ++++++++++++++ ...larky.modules.vgs.metrics.spi.LarkyMetrics | 1 + .../metrics/test_default_metrics.star | 168 ++++++++++++++++++ .../vgs_tests/metrics/test_noop_metrics.star | 21 +++ 11 files changed, 582 insertions(+), 5 deletions(-) create mode 100644 larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModule.java create mode 100644 larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/constants/TransactionMetricsKeys.java create mode 100644 larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/DefaultMetrics.java create mode 100644 larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/NoopMetrics.java create mode 100644 larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/spi/LarkyMetrics.java create mode 100644 larky/src/main/resources/vgs/metrics.star create mode 100644 larky/src/test/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModuleSPITest.java create mode 100644 larky/src/test/resources/META-INF/services/com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics create mode 100644 larky/src/test/resources/vgs_tests/metrics/test_default_metrics.star create mode 100644 larky/src/test/resources/vgs_tests/metrics/test_noop_metrics.star diff --git a/larky/src/main/java/com/verygood/security/larky/ModuleSupplier.java b/larky/src/main/java/com/verygood/security/larky/ModuleSupplier.java index 40a6806b1..0fd1d9beb 100644 --- a/larky/src/main/java/com/verygood/security/larky/ModuleSupplier.java +++ b/larky/src/main/java/com/verygood/security/larky/ModuleSupplier.java @@ -22,6 +22,7 @@ import com.verygood.security.larky.modules.BinasciiModule; import com.verygood.security.larky.modules.C99MathModule; +import com.verygood.security.larky.modules.vgs.metrics.MetricsModule; import com.verygood.security.larky.modules.NetworkTokenModule; import com.verygood.security.larky.modules.CerebroModule; import com.verygood.security.larky.modules.ChaseModule; @@ -96,16 +97,17 @@ public class ModuleSupplier { ); public static final ImmutableSet VGS_MODULES = ImmutableSet.of( - VaultModule.INSTANCE, - NetworkTokenModule.INSTANCE, CerebroModule.INSTANCE, ChaseModule.INSTANCE, - JKSModule.INSTANCE + JKSModule.INSTANCE, + MetricsModule.INSTANCE, + NetworkTokenModule.INSTANCE, + VaultModule.INSTANCE ); public static final ImmutableSet TEST_MODULES = ImmutableSet.of( - UnittestModule.INSTANCE, - AssertionsModule.INSTANCE + AssertionsModule.INSTANCE, + UnittestModule.INSTANCE ); private final Map environment; diff --git a/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModule.java b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModule.java new file mode 100644 index 000000000..f27d05e27 --- /dev/null +++ b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModule.java @@ -0,0 +1,130 @@ +package com.verygood.security.larky.modules.vgs.metrics; + +import com.google.common.collect.ImmutableList; +import com.verygood.security.larky.modules.vgs.metrics.impl.NoopMetrics; +import com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; +import net.starlark.java.annot.Param; +import net.starlark.java.annot.ParamType; +import net.starlark.java.annot.StarlarkBuiltin; +import net.starlark.java.annot.StarlarkMethod; +import net.starlark.java.eval.Dict; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.NoneType; +import org.apache.commons.lang3.StringUtils; + +import java.util.List; +import java.util.ServiceLoader; + +import static com.verygood.security.larky.modules.vgs.metrics.constants.TransactionMetricsKeys.*; + +@Getter +@Slf4j +@StarlarkBuiltin( + name = "native_metrics", + category = "BUILTIN", + doc = "Overridable Metrics API in Larky") +public class MetricsModule implements LarkyMetrics { + + public static final MetricsModule INSTANCE = new MetricsModule(); + private final LarkyMetrics metrics; + + public MetricsModule() { + ServiceLoader loader = ServiceLoader.load(LarkyMetrics.class); + List metricsProviders = ImmutableList.copyOf(loader.iterator()); + + if (metricsProviders.isEmpty()) { + log.error("Using NoopMetrics, should not be used in Production"); + metrics = new NoopMetrics(); + } else if (metricsProviders.size() == 1) { + metrics = metricsProviders.get(0); + } else { + throw new IllegalArgumentException( + String.format( + "MetricsModule expecting only 1 metrics provider of type LarkyMetrics, found %d", + metricsProviders.size())); + + } + } + + @StarlarkMethod( + name = "track", + doc = "Logs the amount, bin, currency, psp, result and type", + parameters = { + @Param( + name = KEY_AMOUNT, + named = true, + positional = false, + doc = "Amount" + ), + @Param( + name = KEY_BIN, + named = true, + positional = false, + doc = "Bank Identification Number" + ), + @Param( + name = KEY_CURRENCY, + named = true, + positional = false, + doc = "Currency" + ), + @Param( + name = KEY_PSP, + named = true, + positional = false, + doc = "Payment Service Provider" + ), + @Param( + name = KEY_RESULT, + named = true, + positional = false, + doc = "Transaction Result" + ), + @Param( + name = KEY_TYPE, + named = true, + positional = false, + doc = "Transaction Type" + ), + @Param( + name = KEY_ATTRIBUTES, + named = true, + positional = false, + doc = "kwargs", + defaultValue = "{}", + allowedTypes = { + @ParamType(type = Dict.class) + } + ), + } + ) + public void track( + Object amount, + Object bin, + Object currency, + Object psp, + Object result, + Object type, + Dict attributes + ) throws EvalException { + metrics.track( + getNullIfNoneOrBlank(amount), + getNullIfNoneOrBlank(bin), + getNullIfNoneOrBlank(currency), + getNullIfNoneOrBlank(psp), + getNullIfNoneOrBlank(result), + getNullIfNoneOrBlank(type), + attributes + ); + } + + private static Object getNullIfNoneOrBlank(Object value) { + if (value instanceof NoneType + || (value instanceof String valStr && StringUtils.isBlank(valStr))) { + return null; + } + return value.toString(); + } +} diff --git a/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/constants/TransactionMetricsKeys.java b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/constants/TransactionMetricsKeys.java new file mode 100644 index 000000000..cbf4bd05e --- /dev/null +++ b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/constants/TransactionMetricsKeys.java @@ -0,0 +1,12 @@ +package com.verygood.security.larky.modules.vgs.metrics.constants; + +public interface TransactionMetricsKeys { + String KEY_AMOUNT = "amount"; + String KEY_BIN = "bin"; + String KEY_CURRENCY = "currency"; + String KEY_PSP = "psp"; + String KEY_RESULT = "result"; + String KEY_TYPE = "type"; + String KEY_ATTRIBUTES = "attributes"; + +} diff --git a/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/DefaultMetrics.java b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/DefaultMetrics.java new file mode 100644 index 000000000..b4eb2d23f --- /dev/null +++ b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/DefaultMetrics.java @@ -0,0 +1,44 @@ +package com.verygood.security.larky.modules.vgs.metrics.impl; + +import com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics; +import net.starlark.java.eval.Dict; +import net.starlark.java.eval.EvalException; + +public class DefaultMetrics implements LarkyMetrics { + + public static final String OUTPUT_STRING = """ + amount %s + bin %s + currency %s + psp %s + result %s + type %s + dict %s + _________________ + """; + + /** + * Not used in production + */ + @Override + public void track( + Object amount, + Object bin, + Object currency, + Object psp, + Object result, + Object type, + Dict attributes + ) throws EvalException { + System.out.printf( + OUTPUT_STRING, + amount, + bin, + currency, + psp, + result, + type, + attributes.toString() + ); + } +} diff --git a/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/NoopMetrics.java b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/NoopMetrics.java new file mode 100644 index 000000000..635dcea3e --- /dev/null +++ b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/impl/NoopMetrics.java @@ -0,0 +1,25 @@ +package com.verygood.security.larky.modules.vgs.metrics.impl; + +import com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics; +import net.starlark.java.eval.Dict; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Starlark; + +public class NoopMetrics implements LarkyMetrics { + + /** + * Not used in production + */ + @Override + public void track( + Object amount, + Object bin, + Object currency, + Object psp, + Object result, + Object type, + Dict attributes + ) throws EvalException { + throw Starlark.errorf("metrics.track operation must be overridden"); + } +} diff --git a/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/spi/LarkyMetrics.java b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/spi/LarkyMetrics.java new file mode 100644 index 000000000..5781a7781 --- /dev/null +++ b/larky/src/main/java/com/verygood/security/larky/modules/vgs/metrics/spi/LarkyMetrics.java @@ -0,0 +1,20 @@ +package com.verygood.security.larky.modules.vgs.metrics.spi; + +import net.starlark.java.eval.Dict; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.StarlarkValue; + + +public interface LarkyMetrics extends StarlarkValue { + + void track( + Object amount, + Object bin, + Object currency, + Object psp, + Object result, + Object type, + Dict attributes + ) throws EvalException; + +} diff --git a/larky/src/main/resources/vgs/metrics.star b/larky/src/main/resources/vgs/metrics.star new file mode 100644 index 000000000..ec8407c5e --- /dev/null +++ b/larky/src/main/resources/vgs/metrics.star @@ -0,0 +1,25 @@ +load("@stdlib//larky", larky="larky") +load("@vgs//native_metrics", _metrics="native_metrics") + +def track( + amount=None, + bin=None, + currency=None, + psp="NOT_SET", + result=None, + type=None, + **kwargs): + _metrics.track( + amount=amount, + bin=bin, + currency=currency, + psp=psp, + result=result, + type=type, + attributes=kwargs + ) + + +metrics = larky.struct( + track=track +) diff --git a/larky/src/test/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModuleSPITest.java b/larky/src/test/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModuleSPITest.java new file mode 100644 index 000000000..fe261d7a4 --- /dev/null +++ b/larky/src/test/java/com/verygood/security/larky/modules/vgs/metrics/MetricsModuleSPITest.java @@ -0,0 +1,129 @@ +package com.verygood.security.larky.modules.vgs.metrics; + +import com.verygood.security.larky.modules.vgs.metrics.impl.NoopMetrics; +import net.starlark.java.eval.Dict; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.StarlarkInt; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static com.verygood.security.larky.modules.vgs.metrics.impl.DefaultMetrics.OUTPUT_STRING; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class MetricsModuleSPITest { + + private static final Path METRICS_CONFIG_PATH = Paths.get( + "target", "test-classes", "META-INF", "services", + "com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics" + ); + + private static String METRICS_SAVED_CONFIG; + private MetricsModule metrics; + + @BeforeAll + public static void setUp() throws Exception { + METRICS_SAVED_CONFIG = getMetricsImpl(); + } + + @AfterAll + public static void tearDown() throws Exception { + setMetricsImpl(METRICS_SAVED_CONFIG); + } + + private static void setMetricsImpl(String implementationURI) throws Exception { + Files.writeString( + METRICS_CONFIG_PATH, + implementationURI + ); + } + + private static String getMetricsImpl() throws Exception { + return new String(Files.readAllBytes(METRICS_CONFIG_PATH)); + } + + @Test + public void testNoopModule_exception() throws Exception { + setMetricsImpl(NoopMetrics.class.getName()); + metrics = new MetricsModule(); + // Assert Exceptions + assertThrows(EvalException.class, + () -> metrics.track( + 0, + 0, + "USD", + "ADYEN", + "SUCCESS", + "AUTHORIZATION", + Dict.empty()), + "metrics.track operation must be overridden" + ); + } + + @Test + public void testDefaultModule_ok() throws Exception { + metrics = new MetricsModule(); + PrintStream originalSystemOut = System.out; + ByteArrayOutputStream systemOutContent = new ByteArrayOutputStream(); + System.setOut(new PrintStream(systemOutContent)); + + int amount = 1234; + int bin = 123456; + String usd = "USD"; + String adyen = "ADYEN"; + String success = "SUCCESS"; + String authorization = "AUTHORIZATION"; + metrics.track( + amount, + bin, + usd, + adyen, + success, + authorization, + Dict.empty() + ); + + assertEquals( + systemOutContent.toString(), + String.format(OUTPUT_STRING, amount, bin, usd, adyen, success, authorization, Dict.empty())); + + System.setOut(originalSystemOut); + } + + @Test + public void testDefaultModule_starlarkInt() throws Exception { + metrics = new MetricsModule(); + PrintStream originalSystemOut = System.out; + ByteArrayOutputStream systemOutContent = new ByteArrayOutputStream(); + System.setOut(new PrintStream(systemOutContent)); + + int amount = 1234; + int bin = 123456; + String usd = "USD"; + String adyen = "ADYEN"; + String success = "SUCCESS"; + String authorization = "AUTHORIZATION"; + metrics.track( + StarlarkInt.of(amount), + StarlarkInt.of(bin), + usd, + adyen, + success, + authorization, + Dict.empty() + ); + + assertEquals( + systemOutContent.toString(), + String.format(OUTPUT_STRING, amount, bin, usd, adyen, success, authorization, Dict.empty())); + + System.setOut(originalSystemOut); + } +} diff --git a/larky/src/test/resources/META-INF/services/com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics b/larky/src/test/resources/META-INF/services/com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics new file mode 100644 index 000000000..c4650a572 --- /dev/null +++ b/larky/src/test/resources/META-INF/services/com.verygood.security.larky.modules.vgs.metrics.spi.LarkyMetrics @@ -0,0 +1 @@ +com.verygood.security.larky.modules.vgs.metrics.impl.DefaultMetrics diff --git a/larky/src/test/resources/vgs_tests/metrics/test_default_metrics.star b/larky/src/test/resources/vgs_tests/metrics/test_default_metrics.star new file mode 100644 index 000000000..f80b644ca --- /dev/null +++ b/larky/src/test/resources/vgs_tests/metrics/test_default_metrics.star @@ -0,0 +1,168 @@ +"""Unit tests for MetricsModule.java using DefaultMetrics API""" + +load("@vendor//asserts", "asserts") +load("@stdlib//unittest", "unittest") +load("@vgs//metrics", "metrics") + + +def _test_default_track_no_args(): + metrics.track() + + +def _test_default_track_None_args(): + metrics.track(None, None, None, None, None, None) + + +def _test_default_track_blank_values(): + metrics.track("", "", "", "", "", "") + + +def _test_default_track_extra_params_with_kwargs_keys(): + metrics.track(a="", b="", c="", d="", e="", f="", g="") + + +def _test_default_track_kwargs_values(): + metrics.track(foo="bar", color="red") + + +def _test_default_track_kwargs_dict(): + metrics.track(**{"foo": "bar"}) + + +def _test_default_track_kwargs_dict_mapped_keys(): + metrics.track(**{"amount": "123"}) + + +def _test_default_track_without_keys(): + # will be mapped to amount and bin + metrics.track("bar", "red") + + +def _test_default_track_amount_bin_with_keys(): + metrics.track(bin="abcde", amount="23wed") + + +def _test_default_track_extra_values_with_keys_dict(): + metrics.track("123", "123456", "c", "p", "r", "t", d="d", e="e") + + +def _test_default_track_named_key_values(): + metrics.track( + amount=123, + bin=123456, + currency="USD", + psp="ADYEN", + result="SUCCESS", + type="AUTHORIZATION") + + +def _test_default_track_named_key_values_dict(): + metrics.track( + amount=123, + bin=123456, + currency="USD", + psp="ADYEN", + result="SUCCESS", + type="AUTHORIZATION", + key1="value1", + key2="value2", + ) + + +def _test_default_track_named_key_values_kwargs(): + metrics.track( + amount=123, + bin=123456, + currency="USD", + psp="ADYEN", + result="SUCCESS", + type="AUTHORIZATION", + **{"key3": "value1", "key4": "value2"} + ) + + +def _test_default_track_keys_unordered(): + metrics.track( + bin=123456, + amount=123, + psp="ADYEN", + key1="value1", + currency="USD", + type="AUTHORIZATION", + result="SUCCESS", + key2="value2", + ) + + +def _test_default_track_amt_dict(): + metrics.track({"foo": "bar"}) + + +def _test_default_track_float_amount(): + metrics.track(amount=11.21, bin=11.21) + + +def _test_default_track_amount_starlark_int(): + # much larger than java.util.Integer + metrics.track(amount="1234567890123") + + +def _test_default_track_amount_negative(): + metrics.track(amount=-11, bin=-11) + + +def _test_default_track_kwargs_object(): + metrics.track(amount={"1": 2}, bin={2: 1}) + + +def _test_default_track_kwargs_map(): + metrics.track(foo={1: {1: 2}}, color={1: 2}) + + +def _test_default_track_fail_extra_params_without_keys(): + asserts.assert_fails( + lambda: metrics.track("", "", "", "", "", "", ""), + ".*accepts no more than 6 positional arguments but got 7.*") + + +def _test_default_track_fail_kwargs_key_non_string(): + asserts.assert_fails( + lambda: metrics.track(**{1: 2}), + "keywords must be strings, not int") + + +def _test_default_track_fail_multiple_same_key(): + asserts.assert_fails( + lambda: metrics.track(amount=444, **{"amount": "123"}), + ".*got multiple values for parameter 'amount'.*") + + +def _suite(): + _suite = unittest.TestSuite() + _suite.addTest(unittest.FunctionTestCase(_test_default_track_no_args)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_None_args)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_blank_values)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_extra_params_with_kwargs_keys)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_kwargs_values)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_kwargs_dict)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_kwargs_dict_mapped_keys)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_without_keys)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_amount_bin_with_keys)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_extra_values_with_keys_dict)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_named_key_values)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_named_key_values_dict)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_named_key_values_kwargs)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_keys_unordered)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_float_amount)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_amount_starlark_int)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_amount_negative)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_kwargs_object)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_kwargs_map)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_fail_extra_params_without_keys)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_fail_kwargs_key_non_string)) + _suite.addTest(unittest.FunctionTestCase(_test_default_track_fail_multiple_same_key)) + return _suite + + +_runner = unittest.TextTestRunner() +_runner.run(_suite()) diff --git a/larky/src/test/resources/vgs_tests/metrics/test_noop_metrics.star b/larky/src/test/resources/vgs_tests/metrics/test_noop_metrics.star new file mode 100644 index 000000000..4e683b2e1 --- /dev/null +++ b/larky/src/test/resources/vgs_tests/metrics/test_noop_metrics.star @@ -0,0 +1,21 @@ +"""Unit tests for MetricsModule.java using NoopMetrics API""" + +load("@vendor//asserts", "asserts") +load("@stdlib//unittest", "unittest") +load("@vgs//metrics", "metrics") + + +def _test_noop_track(): + asserts.assert_fails( + lambda: metrics.track(0, 0, "USD", "ADYEN", "SUCCESS", "AUTHORIZATION"), + "metrics.track operation must be overridden") + + +def _suite(): + _suite = unittest.TestSuite() + _suite.addTest(unittest.FunctionTestCase(_test_noop_track)) + return _suite + + +_runner = unittest.TextTestRunner() +_runner.run(_suite())