From 57c92dd3db1c6e20be5cf773ece7ca0a64fe1c32 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Wed, 29 May 2024 15:27:06 +0200 Subject: [PATCH 1/2] add context field in the payload --- .../java/com/spotify/confidence/Confidence.java | 4 ++-- .../java/com/spotify/confidence/EventSender.java | 2 +- .../spotify/confidence/EventSenderEngine.java | 2 +- .../confidence/EventSenderEngineImpl.java | 4 ++-- .../com/spotify/confidence/EventUploader.java | 14 +++++++++----- .../confidence/EventSenderEngineTest.java | 14 +++++++++++++- .../confidence/FakeEventSenderEngine.java | 4 ++-- .../confidence/GrpcEventUploaderTest.java | 16 ++++++++++++++-- 8 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/spotify/confidence/Confidence.java b/src/main/java/com/spotify/confidence/Confidence.java index 1ff6f49c..4a481d57 100644 --- a/src/main/java/com/spotify/confidence/Confidence.java +++ b/src/main/java/com/spotify/confidence/Confidence.java @@ -101,9 +101,9 @@ public void track(String eventName) { } @Override - public void track(String eventName, ConfidenceValue.Struct message) { + public void track(String eventName, ConfidenceValue.Struct data) { try { - client().emit(eventName, getContext(), Optional.of(message)); + client().emit(eventName, getContext(), Optional.of(data)); } catch (IllegalStateException e) { // swallow this exception } diff --git a/src/main/java/com/spotify/confidence/EventSender.java b/src/main/java/com/spotify/confidence/EventSender.java index 97febc29..c7e217ba 100644 --- a/src/main/java/com/spotify/confidence/EventSender.java +++ b/src/main/java/com/spotify/confidence/EventSender.java @@ -5,7 +5,7 @@ @Beta public interface EventSender extends Contextual { - public void track(String eventName, ConfidenceValue.Struct message); + public void track(String eventName, ConfidenceValue.Struct data); public void track(String eventName); diff --git a/src/main/java/com/spotify/confidence/EventSenderEngine.java b/src/main/java/com/spotify/confidence/EventSenderEngine.java index 27d2fc1d..37b3b2a2 100644 --- a/src/main/java/com/spotify/confidence/EventSenderEngine.java +++ b/src/main/java/com/spotify/confidence/EventSenderEngine.java @@ -4,7 +4,7 @@ import java.util.Optional; interface EventSenderEngine extends Closeable { - void emit(String name, ConfidenceValue.Struct context, Optional message); + void emit(String name, ConfidenceValue.Struct context, Optional data); void flush(); } diff --git a/src/main/java/com/spotify/confidence/EventSenderEngineImpl.java b/src/main/java/com/spotify/confidence/EventSenderEngineImpl.java index 9552dbbe..1cf84951 100644 --- a/src/main/java/com/spotify/confidence/EventSenderEngineImpl.java +++ b/src/main/java/com/spotify/confidence/EventSenderEngineImpl.java @@ -75,13 +75,13 @@ class EventSenderEngineImpl implements EventSenderEngine { @Override public void emit( - String name, ConfidenceValue.Struct context, Optional message) { + String name, ConfidenceValue.Struct context, Optional data) { if (intakeClosed) { log.warn("EventSenderEngine is closed, dropping event {}", name); return; } final Event event = - EventUploader.event(name, context, message).setEventTime(clock.getTimestamp()).build(); + EventUploader.event(name, context, data).setEventTime(clock.getTimestamp()).build(); if (estimatedMemoryConsumption.get() + event.getSerializedSize() > maxMemoryConsumption) { log.warn("EventSenderEngine is overloaded, dropping event {}", name); return; diff --git a/src/main/java/com/spotify/confidence/EventUploader.java b/src/main/java/com/spotify/confidence/EventUploader.java index 76872dd4..6aaf372b 100644 --- a/src/main/java/com/spotify/confidence/EventUploader.java +++ b/src/main/java/com/spotify/confidence/EventUploader.java @@ -2,19 +2,23 @@ import com.google.protobuf.Struct; import com.spotify.confidence.events.v1.Event; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; interface EventUploader { static Event.Builder event( - String name, ConfidenceValue.Struct context, Optional message) { + String name, ConfidenceValue.Struct context, Optional data) { + final Map map = new HashMap<>(Map.of()); + final ConfidenceValue.Struct dataStruct = data.orElse(ConfidenceValue.Struct.EMPTY); + map.putAll(dataStruct.asMap()); + map.put("context", context); + return Event.newBuilder() .setEventDefinition(EventSenderEngineImpl.EVENT_NAME_PREFIX + name) - .setPayload( - Struct.newBuilder() - .putAllFields(context.asProtoMap()) - .putAllFields(message.orElse(ConfidenceValue.Struct.EMPTY).asProtoMap())); + .setPayload(Struct.newBuilder().putAllFields(ConfidenceValue.of(map).asProtoMap())); } CompletableFuture upload(List events); diff --git a/src/test/java/com/spotify/confidence/EventSenderEngineTest.java b/src/test/java/com/spotify/confidence/EventSenderEngineTest.java index 53976d2a..a28a1ebe 100644 --- a/src/test/java/com/spotify/confidence/EventSenderEngineTest.java +++ b/src/test/java/com/spotify/confidence/EventSenderEngineTest.java @@ -110,7 +110,19 @@ public void testOverlappingKeysInPayload() throws InterruptedException { "a", Value.newBuilder().setNumberValue(0).build(), "message", - Value.newBuilder().setNumberValue(1).build())) + Value.newBuilder().setNumberValue(1).build(), + "context", + Value.newBuilder() + .setStructValue( + Struct.newBuilder() + .putAllFields( + Map.of( + "a", + Value.newBuilder().setNumberValue(2).build(), + "message", + Value.newBuilder().setNumberValue(3).build())) + .build()) + .build())) .build()); } diff --git a/src/test/java/com/spotify/confidence/FakeEventSenderEngine.java b/src/test/java/com/spotify/confidence/FakeEventSenderEngine.java index 8741e64b..707d1b2b 100644 --- a/src/test/java/com/spotify/confidence/FakeEventSenderEngine.java +++ b/src/test/java/com/spotify/confidence/FakeEventSenderEngine.java @@ -25,8 +25,8 @@ public void close() throws IOException { @Override public void emit( - String name, ConfidenceValue.Struct context, Optional message) { - events.add(event(name, context, message).setEventTime(clock.getTimestamp()).build()); + String name, ConfidenceValue.Struct context, Optional data) { + events.add(event(name, context, data).setEventTime(clock.getTimestamp()).build()); } @Override diff --git a/src/test/java/com/spotify/confidence/GrpcEventUploaderTest.java b/src/test/java/com/spotify/confidence/GrpcEventUploaderTest.java index a4fb052a..31f9e480 100644 --- a/src/test/java/com/spotify/confidence/GrpcEventUploaderTest.java +++ b/src/test/java/com/spotify/confidence/GrpcEventUploaderTest.java @@ -98,7 +98,13 @@ public void testMapsSingleEventBatchToProtobuf() throws ExecutionException, Inte final Map fieldsMap = protoEvent.getPayload().getFieldsMap(); assertThat(fieldsMap.get("messageKey").getStringValue()).isEqualTo("value_1"); - assertThat(fieldsMap.get("contextKey").getStringValue()).isEqualTo("value_1"); + assertThat( + fieldsMap + .get("context") + .getStructValue() + .getFieldsOrThrow("contextKey") + .getStringValue()) + .isEqualTo("value_1"); } @Test @@ -130,7 +136,13 @@ public void testMapsMultiEventBatchToProtobuf() { final Map fieldsMap = protoEvent.getPayload().getFieldsMap(); assertThat(fieldsMap.get("messageKey").getStringValue()).isEqualTo("value_m" + (i + 1)); - assertThat(fieldsMap.get("contextKey").getStringValue()).isEqualTo("value_c" + (i + 1)); + assertThat( + fieldsMap + .get("context") + .getStructValue() + .getFieldsOrThrow("contextKey") + .getStringValue()) + .isEqualTo("value_c" + (i + 1)); } } From 2ffa4e178ae9740bb51a449c05ccdcce1d1a0e44 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Wed, 29 May 2024 16:15:20 +0200 Subject: [PATCH 2/2] throw on context being in data --- src/main/java/com/spotify/confidence/Confidence.java | 4 ++++ src/main/java/com/spotify/confidence/Exceptions.java | 6 ++++++ .../com/spotify/confidence/ConfidenceContextTest.java | 11 +++++++++++ 3 files changed, 21 insertions(+) diff --git a/src/main/java/com/spotify/confidence/Confidence.java b/src/main/java/com/spotify/confidence/Confidence.java index 4a481d57..11704e14 100644 --- a/src/main/java/com/spotify/confidence/Confidence.java +++ b/src/main/java/com/spotify/confidence/Confidence.java @@ -102,6 +102,10 @@ public void track(String eventName) { @Override public void track(String eventName, ConfidenceValue.Struct data) { + if (data.asMap().containsKey("context")) { + throw new Exceptions.InvalidContextInMessaageError( + "Field 'context' is not allowed in event's data"); + } try { client().emit(eventName, getContext(), Optional.of(data)); } catch (IllegalStateException e) { diff --git a/src/main/java/com/spotify/confidence/Exceptions.java b/src/main/java/com/spotify/confidence/Exceptions.java index d3d0978d..6dc6135b 100644 --- a/src/main/java/com/spotify/confidence/Exceptions.java +++ b/src/main/java/com/spotify/confidence/Exceptions.java @@ -20,6 +20,12 @@ public ParseError(String message) { } } + public static class InvalidContextInMessaageError extends RuntimeException { + public InvalidContextInMessaageError(String message) { + super(message); + } + } + public static class IllegalValueType extends Exception { public IllegalValueType(String message) { super(message); diff --git a/src/test/java/com/spotify/confidence/ConfidenceContextTest.java b/src/test/java/com/spotify/confidence/ConfidenceContextTest.java index a70f996e..e36447bd 100644 --- a/src/test/java/com/spotify/confidence/ConfidenceContextTest.java +++ b/src/test/java/com/spotify/confidence/ConfidenceContextTest.java @@ -1,8 +1,10 @@ package com.spotify.confidence; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableMap; +import java.util.Map; import org.junit.jupiter.api.Test; public class ConfidenceContextTest { @@ -11,6 +13,15 @@ public class ConfidenceContextTest { private final ResolverClientTestUtils.FakeFlagResolverClient fakeFlagResolverClient = new ResolverClientTestUtils.FakeFlagResolverClient(); + @Test + public void testThrowInvalidContextInMessage() { + final Confidence root = Confidence.create(fakeEngine, fakeFlagResolverClient); + assertThrows( + Exceptions.InvalidContextInMessaageError.class, + () -> + root.track("hello", ConfidenceValue.of(Map.of("context", ConfidenceValue.NULL_VALUE)))); + } + @Test public void getContextContainsParentContextValues() { final Confidence root = Confidence.create(fakeEngine, fakeFlagResolverClient);