From 3e6d92ea4087b85fdcb4f4b090d7df6104a5f77e Mon Sep 17 00:00:00 2001 From: Vignesh Raja Date: Sat, 15 Jul 2017 16:13:17 -0700 Subject: [PATCH 1/2] Add support for numeric metrics --- .../ab/event/internal/EventBuilderV2.java | 14 +++++++--- .../event/internal/payload/EventMetric.java | 26 +++++++++---------- .../optimizely/ab/internal/EventTagUtils.java | 18 +++++++++++++ .../ab/internal/ReservedEventKey.java | 3 ++- .../ab/event/internal/EventBuilderV2Test.java | 19 +++++++++----- 5 files changed, 55 insertions(+), 25 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java index a4a25a671..ae13183ae 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventBuilderV2.java @@ -112,10 +112,16 @@ public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig, List layerStates = createLayerStates(projectConfig, experimentVariationMap); - Long eventValue = EventTagUtils.getRevenueValue(eventTags); - List eventMetrics = Collections.emptyList(); - if (eventValue != null) { - eventMetrics = Collections.singletonList(new EventMetric(EventMetric.REVENUE_METRIC_TYPE, eventValue)); + List eventMetrics = new ArrayList(); + + Long revenueValue = EventTagUtils.getRevenueValue(eventTags); + if (revenueValue != null) { + eventMetrics.add(new EventMetric(EventMetric.REVENUE_METRIC_TYPE, revenueValue)); + } + + Double numericMetricValue = EventTagUtils.getNumericValue(eventTags); + if (numericMetricValue != null) { + eventMetrics.add(new EventMetric(EventMetric.NUMERIC_METRIC_TYPE, numericMetricValue)); } Conversion conversionPayload = new Conversion(); diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java index 5303871c9..ad6b4753e 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java @@ -19,13 +19,14 @@ public class EventMetric { public static final String REVENUE_METRIC_TYPE = "revenue"; + public static final String NUMERIC_METRIC_TYPE = "value"; private String name; - private long value; + private Number value; public EventMetric() { } - public EventMetric(String name, long value) { + public EventMetric(String name, Number value) { this.name = name; this.value = value; } @@ -38,30 +39,29 @@ public void setName(String name) { this.name = name; } - public long getValue() { + public Number getValue() { return value; } - public void setValue(long value) { + public void setValue(Number value) { this.value = value; } - @Override - public boolean equals(Object other) { - if (!(other instanceof EventMetric)) - return false; + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; - EventMetric otherEventMetric = (EventMetric)other; + EventMetric that = (EventMetric) o; - return name.equals(otherEventMetric.getName()) && value == otherEventMetric.getValue(); + if (!name.equals(that.name)) return false; + return value.equals(that.value); } - @Override public int hashCode() { - int result = name != null ? name.hashCode() : 0; - result = 31 * result + (int) (value ^ (value >>> 32)); + int result = name.hashCode(); + result = 31 * result + value.hashCode(); return result; } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java index 42dacfc5e..ea5f87cdf 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java @@ -47,4 +47,22 @@ public static Long getRevenueValue(@Nonnull Map eventTags) { } return eventValue; } + + /** + * Fetch the numeric metric value from event tags. "value" is a reserved keyword. + */ + public static Double getNumericValue(@Nonnull Map eventTags) { + Double eventValue = null; + if (eventTags.containsKey(ReservedEventKey.VALUE.toString())) { + Object rawValue = eventTags.get(ReservedEventKey.VALUE.toString()); + if (rawValue instanceof Double) { + eventValue = (Double)rawValue; + logger.info("Parsed numeric metric value \"{}\" from event tags.", eventValue); + } else { + logger.warn("Failed to parse numeric metric value \"{}\" from event tags.", rawValue); + } + } + + return eventValue; + } } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ReservedEventKey.java b/core-api/src/main/java/com/optimizely/ab/internal/ReservedEventKey.java index ecc30f5b6..91d8729d6 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ReservedEventKey.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ReservedEventKey.java @@ -17,7 +17,8 @@ package com.optimizely.ab.internal; public enum ReservedEventKey { - REVENUE("revenue"); + REVENUE("revenue"), + VALUE("value"); private final String key; diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java index 446eed4cb..841315924 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventBuilderV2Test.java @@ -17,6 +17,7 @@ package com.optimizely.ab.event.internal; import com.google.gson.Gson; +import com.google.gson.internal.LazilyParsedNumber; import com.optimizely.ab.bucketing.Bucketer; import com.optimizely.ab.bucketing.DecisionService; import com.optimizely.ab.bucketing.UserProfileService; @@ -314,11 +315,13 @@ public void createConversionEvent() throws Exception { } /** - * Verify that eventValue is properly recorded in a conversion request as an {@link EventMetric} + * Verify that "revenue" and "value" are properly recorded in a conversion request as {@link EventMetric} objects. + * "revenue" is fixed-point and "value" is floating-point. */ @Test - public void createConversionParamsWithRevenue() throws Exception { - long revenue = 1234L; + public void createConversionParamsWithEventMetrics() throws Exception { + Long revenue = 1234L; + Double value = 13.37; // use the "valid" project config and its associated experiment, variation, and attributes Attribute attribute = validProjectConfig.getAttributes().get(0); @@ -341,6 +344,7 @@ public void createConversionParamsWithRevenue() throws Exception { Map attributeMap = Collections.singletonMap(attribute.getKey(), "value"); Map eventTagMap = new HashMap(); eventTagMap.put(ReservedEventKey.REVENUE.toString(), revenue); + eventTagMap.put(ReservedEventKey.VALUE.toString(), value); Map experimentVariationMap = createExperimentVariationMap( validProjectConfig, decisionService, @@ -352,10 +356,11 @@ public void createConversionParamsWithRevenue() throws Exception { eventTagMap); Conversion conversion = gson.fromJson(conversionEvent.getBody(), Conversion.class); - - // we're not going to verify everything, only revenue - assertThat(conversion.getEventMetrics(), - is(Collections.singletonList(new EventMetric(EventMetric.REVENUE_METRIC_TYPE, revenue)))); + List eventMetrics = Arrays.asList( + new EventMetric(EventMetric.REVENUE_METRIC_TYPE, new LazilyParsedNumber(revenue.toString())), + new EventMetric(EventMetric.NUMERIC_METRIC_TYPE, new LazilyParsedNumber(value.toString()))); + // we're not going to verify everything, only the event metrics + assertThat(conversion.getEventMetrics(), is(eventMetrics)); } /** From d5af36ae460a6f8f82f01d16ac036526a8985a8b Mon Sep 17 00:00:00 2001 From: Vignesh Raja Date: Mon, 17 Jul 2017 11:07:20 -0700 Subject: [PATCH 2/2] Address feedback --- .../optimizely/ab/event/internal/payload/EventMetric.java | 8 ++++---- .../java/com/optimizely/ab/internal/EventTagUtils.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java index ad6b4753e..f77f7e578 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventMetric.java @@ -48,11 +48,11 @@ public void setValue(Number value) { } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; - EventMetric that = (EventMetric) o; + EventMetric that = (EventMetric) obj; if (!name.equals(that.name)) return false; return value.equals(that.value); diff --git a/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java index ea5f87cdf..e7c0359c5 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/EventTagUtils.java @@ -55,8 +55,8 @@ public static Double getNumericValue(@Nonnull Map eventTags) { Double eventValue = null; if (eventTags.containsKey(ReservedEventKey.VALUE.toString())) { Object rawValue = eventTags.get(ReservedEventKey.VALUE.toString()); - if (rawValue instanceof Double) { - eventValue = (Double)rawValue; + if (rawValue instanceof Number) { + eventValue = ((Number) rawValue).doubleValue(); logger.info("Parsed numeric metric value \"{}\" from event tags.", eventValue); } else { logger.warn("Failed to parse numeric metric value \"{}\" from event tags.", rawValue);