From 91b1b34fcb5bcb2b1d70ea6af021aced316c19cf Mon Sep 17 00:00:00 2001 From: SrikarMannepalli <37926341+SrikarMannepalli@users.noreply.github.com> Date: Thu, 21 Oct 2021 00:14:08 +0530 Subject: [PATCH] add event time to configChangeEventValue (#74) * add event time to configChangeEventValue * make event time optional * revert making event time field optional --- .../event/v1/config_change_event_value.proto | 1 + .../impl/ConfigChangeEventGeneratorFactory.java | 6 ++++-- .../impl/ConfigChangeEventGeneratorImpl.java | 12 ++++++++++-- .../ConfigChangeEventGeneratorFactoryTest.java | 7 +++++-- .../impl/ConfigChangeEventGeneratorImplTest.java | 15 ++++++++++++++- .../config/service/ConfigServicesFactory.java | 4 +++- 6 files changed, 37 insertions(+), 8 deletions(-) diff --git a/config-service-change-event-api/src/main/proto/org/hypertrace/config/change/event/v1/config_change_event_value.proto b/config-service-change-event-api/src/main/proto/org/hypertrace/config/change/event/v1/config_change_event_value.proto index 94bdf025..92cc6b6b 100644 --- a/config-service-change-event-api/src/main/proto/org/hypertrace/config/change/event/v1/config_change_event_value.proto +++ b/config-service-change-event-api/src/main/proto/org/hypertrace/config/change/event/v1/config_change_event_value.proto @@ -14,6 +14,7 @@ message ConfigChangeEventValue { } optional string user_id = 4; optional string user_name = 5; + int64 event_time_millis = 6; } message ConfigCreateEvent { diff --git a/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactory.java b/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactory.java index ea6afc52..52be0e1c 100644 --- a/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactory.java +++ b/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactory.java @@ -1,6 +1,7 @@ package org.hypertrace.config.service.change.event.impl; import com.typesafe.config.Config; +import java.time.Clock; import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator; public class ConfigChangeEventGeneratorFactory { @@ -17,9 +18,10 @@ public static ConfigChangeEventGeneratorFactory getInstance() { return instance; } - public ConfigChangeEventGenerator createConfigChangeEventGenerator(Config appConfig) { + public ConfigChangeEventGenerator createConfigChangeEventGenerator( + Config appConfig, Clock clock) { if (appConfig.getBoolean(GENERIC_CONFIG_SERVICE_PUBLISH_CHANGE_EVENTS)) { - return new ConfigChangeEventGeneratorImpl(appConfig); + return new ConfigChangeEventGeneratorImpl(appConfig, clock); } else { return new NoopConfigChangeEventGenerator(); } diff --git a/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImpl.java b/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImpl.java index 9dbdbb7b..688244c5 100644 --- a/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImpl.java +++ b/config-service-change-event-generator/src/main/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImpl.java @@ -3,6 +3,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.protobuf.Value; import com.typesafe.config.Config; +import java.time.Clock; import java.util.Optional; import lombok.extern.slf4j.Slf4j; import org.hypertrace.config.change.event.v1.ConfigChangeEventKey; @@ -31,9 +32,11 @@ public class ConfigChangeEventGeneratorImpl implements ConfigChangeEventGenerato private final EventProducer configChangeEventProducer; + private Clock clock; - ConfigChangeEventGeneratorImpl(Config appConfig) { + ConfigChangeEventGeneratorImpl(Config appConfig, Clock clock) { Config config = appConfig.getConfig(EVENT_STORE); + this.clock = clock; String storeType = config.getString(EVENT_STORE_TYPE_CONFIG); EventStore eventStore = EventStoreProvider.getEventStore(storeType, config); configChangeEventProducer = @@ -45,7 +48,9 @@ public class ConfigChangeEventGeneratorImpl implements ConfigChangeEventGenerato @VisibleForTesting ConfigChangeEventGeneratorImpl( - EventProducer configChangeEventProducer) { + EventProducer configChangeEventProducer, + Clock clock) { + this.clock = clock; this.configChangeEventProducer = configChangeEventProducer; } @@ -103,6 +108,7 @@ private void produceCreateNotification( ConfigCreateEvent.newBuilder() .setCreatedConfigJson(ConfigProtoConverter.convertToJsonString(config)) .build()); + builder.setEventTimeMillis(clock.millis()); populateUserDetails(requestContext, builder); configChangeEventProducer.send( KeyUtil.getKey(tenantId, configType, contextOptional), builder.build()); @@ -130,6 +136,7 @@ private void produceUpdateNotification( .setPreviousConfigJson(ConfigProtoConverter.convertToJsonString(prevConfig)) .setLatestConfigJson(ConfigProtoConverter.convertToJsonString(latestConfig)) .build()); + builder.setEventTimeMillis(clock.millis()); populateUserDetails(requestContext, builder); configChangeEventProducer.send( KeyUtil.getKey(tenantId, configType, contextOptional), builder.build()); @@ -155,6 +162,7 @@ private void produceDeleteNotification( ConfigDeleteEvent.newBuilder() .setDeletedConfigJson(ConfigProtoConverter.convertToJsonString(config)) .build()); + builder.setEventTimeMillis(clock.millis()); populateUserDetails(requestContext, builder); configChangeEventProducer.send( KeyUtil.getKey(tenantId, configType, contextOptional), builder.build()); diff --git a/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactoryTest.java b/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactoryTest.java index c095c418..8fac596e 100644 --- a/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactoryTest.java +++ b/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorFactoryTest.java @@ -5,6 +5,7 @@ import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; +import java.time.Clock; import java.util.Map; import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator; import org.junit.jupiter.api.Test; @@ -16,7 +17,8 @@ void createNoopConfigChangeEventGenerator() { Config config = ConfigFactory.parseMap(Map.of(GENERIC_CONFIG_SERVICE_PUBLISH_CHANGE_EVENTS, "false")); ConfigChangeEventGenerator configChangeEventGenerator = - ConfigChangeEventGeneratorFactory.getInstance().createConfigChangeEventGenerator(config); + ConfigChangeEventGeneratorFactory.getInstance() + .createConfigChangeEventGenerator(config, Clock.systemUTC()); assertTrue(configChangeEventGenerator instanceof NoopConfigChangeEventGenerator); } @@ -24,7 +26,8 @@ void createNoopConfigChangeEventGenerator() { void createConfigChangeEventGeneratorImpl() { Config config = getEventStoreConfig(); ConfigChangeEventGenerator configChangeEventGenerator = - ConfigChangeEventGeneratorFactory.getInstance().createConfigChangeEventGenerator(config); + ConfigChangeEventGeneratorFactory.getInstance() + .createConfigChangeEventGenerator(config, Clock.systemUTC()); assertTrue(configChangeEventGenerator instanceof ConfigChangeEventGeneratorImpl); } diff --git a/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImplTest.java b/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImplTest.java index 700a14b5..f6d7e3df 100644 --- a/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImplTest.java +++ b/config-service-change-event-generator/src/test/java/org/hypertrace/config/service/change/event/impl/ConfigChangeEventGeneratorImplTest.java @@ -1,9 +1,12 @@ package org.hypertrace.config.service.change.event.impl; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.Value; +import java.time.Clock; import java.util.Optional; import org.hypertrace.config.change.event.v1.ConfigChangeEventKey; import org.hypertrace.config.change.event.v1.ConfigChangeEventValue; @@ -29,15 +32,19 @@ class ConfigChangeEventGeneratorImplTest { private static final String TEST_CONTEXT = "test-context"; private static final String TEST_VALUE = "test-value"; private static final String TEST_NEW_VALUE = "test-new-value"; + private static final long CURRENT_TIME_MILLIS = 1000; @Mock EventProducer eventProducer; ConfigChangeEventGeneratorImpl changeEventGenerator; RequestContext requestContext; + private Clock mockClock; @BeforeEach void setup() { - changeEventGenerator = new ConfigChangeEventGeneratorImpl(eventProducer); + mockClock = mock(Clock.class); + when(mockClock.millis()).thenReturn(CURRENT_TIME_MILLIS); + changeEventGenerator = new ConfigChangeEventGeneratorImpl(eventProducer, mockClock); requestContext = RequestContext.forTenantId(TEST_TENANT_ID_1); } @@ -50,6 +57,7 @@ void sendCreateNotification() throws InvalidProtocolBufferException { .send( KeyUtil.getKey(TEST_TENANT_ID_1, TEST_CONFIG_TYPE, Optional.of(TEST_CONTEXT)), ConfigChangeEventValue.newBuilder() + .setEventTimeMillis(CURRENT_TIME_MILLIS) .setCreateEvent( ConfigCreateEvent.newBuilder() .setCreatedConfigJson(ConfigProtoConverter.convertToJsonString(config)) @@ -65,6 +73,7 @@ void sendCreateNotificationWithNoContext() throws InvalidProtocolBufferException .send( KeyUtil.getKey(TEST_TENANT_ID_1, TEST_CONFIG_TYPE, Optional.empty()), ConfigChangeEventValue.newBuilder() + .setEventTimeMillis(CURRENT_TIME_MILLIS) .setCreateEvent( ConfigCreateEvent.newBuilder() .setCreatedConfigJson(ConfigProtoConverter.convertToJsonString(config)) @@ -81,6 +90,7 @@ void sendDeleteNotification() throws InvalidProtocolBufferException { .send( KeyUtil.getKey(TEST_TENANT_ID_1, TEST_CONFIG_TYPE, Optional.of(TEST_CONTEXT)), ConfigChangeEventValue.newBuilder() + .setEventTimeMillis(CURRENT_TIME_MILLIS) .setDeleteEvent( ConfigDeleteEvent.newBuilder() .setDeletedConfigJson(ConfigProtoConverter.convertToJsonString(config)) @@ -96,6 +106,7 @@ void sendDeleteNotificationWithNoContext() throws InvalidProtocolBufferException .send( KeyUtil.getKey(TEST_TENANT_ID_1, TEST_CONFIG_TYPE, Optional.empty()), ConfigChangeEventValue.newBuilder() + .setEventTimeMillis(CURRENT_TIME_MILLIS) .setDeleteEvent( ConfigDeleteEvent.newBuilder() .setDeletedConfigJson(ConfigProtoConverter.convertToJsonString(config)) @@ -114,6 +125,7 @@ void sendChangeNotification() throws InvalidProtocolBufferException { .send( KeyUtil.getKey(TEST_TENANT_ID_1, TEST_CONFIG_TYPE, Optional.of(TEST_CONTEXT)), ConfigChangeEventValue.newBuilder() + .setEventTimeMillis(CURRENT_TIME_MILLIS) .setUpdateEvent( ConfigUpdateEvent.newBuilder() .setPreviousConfigJson(ConfigProtoConverter.convertToJsonString(prevConfig)) @@ -133,6 +145,7 @@ void sendChangeNotificationWithNoContext() throws InvalidProtocolBufferException .send( KeyUtil.getKey(TEST_TENANT_ID_1, TEST_CONFIG_TYPE, Optional.empty()), ConfigChangeEventValue.newBuilder() + .setEventTimeMillis(CURRENT_TIME_MILLIS) .setUpdateEvent( ConfigUpdateEvent.newBuilder() .setPreviousConfigJson(ConfigProtoConverter.convertToJsonString(prevConfig)) diff --git a/config-service/src/main/java/org/hypertrace/config/service/ConfigServicesFactory.java b/config-service/src/main/java/org/hypertrace/config/service/ConfigServicesFactory.java index bba9eac8..69a7ee70 100644 --- a/config-service/src/main/java/org/hypertrace/config/service/ConfigServicesFactory.java +++ b/config-service/src/main/java/org/hypertrace/config/service/ConfigServicesFactory.java @@ -4,6 +4,7 @@ import io.grpc.BindableService; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; +import java.time.Clock; import java.util.List; import org.hypertrace.alerting.config.service.EventConditionConfigServiceImpl; import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator; @@ -33,7 +34,8 @@ public static List buildAllConfigServices( lifecycle.shutdownComplete().thenRun(configChannel::shutdown); ConfigChangeEventGenerator configChangeEventGenerator = - ConfigChangeEventGeneratorFactory.getInstance().createConfigChangeEventGenerator(config); + ConfigChangeEventGeneratorFactory.getInstance() + .createConfigChangeEventGenerator(config, Clock.systemUTC()); return List.of( new ConfigServiceGrpcImpl(configStore),