Add support for numeric metrics #129
Conversation
| Double eventValue = null; | ||
| if (eventTags.containsKey(ReservedEventKey.VALUE.toString())) { | ||
| Object rawValue = eventTags.get(ReservedEventKey.VALUE.toString()); | ||
| if (rawValue instanceof Double) { |
There was a problem hiding this comment.
It's reasonable to let this be anything that implements Number IMO. I think this will lead to confusing behavior
There was a problem hiding this comment.
makes sense, the opposite case is the one that we should be wary of (truncating floating-point values)
| eventMetrics.add(new EventMetric(EventMetric.REVENUE_METRIC_TYPE, revenueValue)); | ||
| } | ||
|
|
||
| Double numericMetricValue = EventTagUtils.getNumericValue(eventTags); |
There was a problem hiding this comment.
@vraja2 can there only be one numeric metric value?
There was a problem hiding this comment.
yes, it will correspond to the event being tracked.
| public boolean equals(Object other) { | ||
| if (!(other instanceof EventMetric)) | ||
| return false; | ||
| public boolean equals(Object o) { |
There was a problem hiding this comment.
please don't name variables single characters
There was a problem hiding this comment.
intellij auto-generated, will change to obj
aliabbasrizvi
left a comment
aliabbasrizvi
left a comment
There was a problem hiding this comment.
Request you to go through the prescribed review process.
|
@vraja2 will add docs to explain why they should send us floats or doubles instead of int/long |
@optimizely/fullstack-devs