update exclude span rules apis to expose creation and last updated timestamp (#96)

SrikarMannepalli · web-flow · commit 3a2276fec73d · 2022-02-24T12:13:04.000+05:30
* update exclude span rules apis to expose creation and last updated timestamp

* impl changrs

* cleanup

* address review comments
diff --git a/span-processing-config-service-api/src/main/proto/org/hypertrace/span/processing/config/service/v1/span_processing_config_service.proto b/span-processing-config-service-api/src/main/proto/org/hypertrace/span/processing/config/service/v1/span_processing_config_service.proto
@@ -4,6 +4,8 @@ package org.hypertrace.span.processing.config.service.v1;
 
 option java_multiple_files = true;
 
+import "google/protobuf/timestamp.proto";
+
 service SpanProcessingConfigService {
   rpc CreateExcludeSpanRule(CreateExcludeSpanRuleRequest) returns (CreateExcludeSpanRuleResponse) {}
 
@@ -19,21 +21,21 @@ message CreateExcludeSpanRuleRequest {
 }
 
 message CreateExcludeSpanRuleResponse {
-  ExcludeSpanRule rule = 1;
+  ExcludeSpanRuleDetails rule_details = 1;
 }
 
 message GetAllExcludeSpanRulesRequest {}
 
 message GetAllExcludeSpanRulesResponse {
-  repeated ExcludeSpanRule rules = 1;
+  repeated ExcludeSpanRuleDetails rule_details = 1;
 }
 
 message UpdateExcludeSpanRuleRequest {
   UpdateExcludeSpanRule rule = 1;
 }
 
 message UpdateExcludeSpanRuleResponse {
-  ExcludeSpanRule rule = 1;
+  ExcludeSpanRuleDetails rule_details = 1;
 }
 
 message DeleteExcludeSpanRuleRequest {
@@ -47,11 +49,21 @@ message ExcludeSpanRule {
   ExcludeSpanRuleInfo rule_info = 2;
 }
 
+message ExcludeSpanRuleDetails {
+  ExcludeSpanRule rule = 1;
+  ExcludeSpanRuleMetadata metadata = 2;
+}
+
 message ExcludeSpanRuleInfo {
   string name = 1;
   SpanFilter filter = 2;
 }
 
+message ExcludeSpanRuleMetadata {
+  google.protobuf.Timestamp creation_timestamp = 1;
+  google.protobuf.Timestamp last_updated_timestamp = 2;
+}
+
 message UpdateExcludeSpanRule {
   string id = 1;
   string name = 2;
diff --git a/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/SpanProcessingConfigServiceImpl.java b/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/SpanProcessingConfigServiceImpl.java
@@ -5,14 +5,18 @@
 import io.grpc.stub.StreamObserver;
 import java.util.UUID;
 import lombok.extern.slf4j.Slf4j;
+import org.hypertrace.config.objectstore.ContextualConfigObject;
 import org.hypertrace.core.grpcutils.context.RequestContext;
 import org.hypertrace.span.processing.config.service.store.ExcludeSpanRulesConfigStore;
+import org.hypertrace.span.processing.config.service.utils.TimestampConverter;
 import org.hypertrace.span.processing.config.service.v1.CreateExcludeSpanRuleRequest;
 import org.hypertrace.span.processing.config.service.v1.CreateExcludeSpanRuleResponse;
 import org.hypertrace.span.processing.config.service.v1.DeleteExcludeSpanRuleRequest;
 import org.hypertrace.span.processing.config.service.v1.DeleteExcludeSpanRuleResponse;
 import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRule;
+import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleDetails;
 import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleInfo;
+import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleMetadata;
 import org.hypertrace.span.processing.config.service.v1.GetAllExcludeSpanRulesRequest;
 import org.hypertrace.span.processing.config.service.v1.GetAllExcludeSpanRulesResponse;
 import org.hypertrace.span.processing.config.service.v1.SpanProcessingConfigServiceGrpc;
@@ -26,13 +30,16 @@ class SpanProcessingConfigServiceImpl
     extends SpanProcessingConfigServiceGrpc.SpanProcessingConfigServiceImplBase {
   private final SpanProcessingConfigRequestValidator validator;
   private final ExcludeSpanRulesConfigStore ruleStore;
+  private final TimestampConverter timestampConverter;
 
   @Inject
   SpanProcessingConfigServiceImpl(
       ExcludeSpanRulesConfigStore ruleStore,
-      SpanProcessingConfigRequestValidator requestValidator) {
+      SpanProcessingConfigRequestValidator requestValidator,
+      TimestampConverter timestampConverter) {
     this.validator = requestValidator;
     this.ruleStore = ruleStore;
+    this.timestampConverter = timestampConverter;
   }
 
   @Override
@@ -45,7 +52,7 @@ public void getAllExcludeSpanRules(
 
       responseObserver.onNext(
           GetAllExcludeSpanRulesResponse.newBuilder()
-              .addAllRules(ruleStore.getAllData(requestContext))
+              .addAllRuleDetails(ruleStore.getAllData(requestContext))
               .build());
       responseObserver.onCompleted();
     } catch (Exception e) {
@@ -62,7 +69,7 @@ public void createExcludeSpanRule(
       RequestContext requestContext = RequestContext.CURRENT.get();
       this.validator.validateOrThrow(requestContext, request);
 
-      // TODO: need to handle prorities
+      // TODO: need to handle priorities
       ExcludeSpanRule newRule =
           ExcludeSpanRule.newBuilder()
               .setId(UUID.randomUUID().toString())
@@ -71,7 +78,8 @@ public void createExcludeSpanRule(
 
       responseObserver.onNext(
           CreateExcludeSpanRuleResponse.newBuilder()
-              .setRule(this.ruleStore.upsertObject(requestContext, newRule).getData())
+              .setRuleDetails(
+                  buildExcludeSpanRuleDetails(this.ruleStore.upsertObject(requestContext, newRule)))
               .build());
       responseObserver.onCompleted();
     } catch (Exception exception) {
@@ -97,7 +105,9 @@ public void updateExcludeSpanRule(
 
       responseObserver.onNext(
           UpdateExcludeSpanRuleResponse.newBuilder()
-              .setRule(this.ruleStore.upsertObject(requestContext, updatedRule).getData())
+              .setRuleDetails(
+                  buildExcludeSpanRuleDetails(
+                      this.ruleStore.upsertObject(requestContext, updatedRule)))
               .build());
       responseObserver.onCompleted();
     } catch (Exception exception) {
@@ -137,4 +147,18 @@ private ExcludeSpanRule buildUpdatedRule(
                 .build())
         .build();
   }
+
+  private ExcludeSpanRuleDetails buildExcludeSpanRuleDetails(
+      ContextualConfigObject<ExcludeSpanRule> configObject) {
+    return ExcludeSpanRuleDetails.newBuilder()
+        .setRule(configObject.getData())
+        .setMetadata(
+            ExcludeSpanRuleMetadata.newBuilder()
+                .setCreationTimestamp(
+                    timestampConverter.convert(configObject.getCreationTimestamp()))
+                .setLastUpdatedTimestamp(
+                    timestampConverter.convert(configObject.getLastUpdatedTimestamp()))
+                .build())
+        .build();
+  }
 }
diff --git a/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/store/ExcludeSpanRulesConfigStore.java b/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/store/ExcludeSpanRulesConfigStore.java
@@ -6,31 +6,49 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 import lombok.SneakyThrows;
-import org.hypertrace.config.objectstore.ContextualConfigObject;
 import org.hypertrace.config.objectstore.IdentifiedObjectStore;
 import org.hypertrace.config.proto.converter.ConfigProtoConverter;
 import org.hypertrace.config.service.v1.ConfigServiceGrpc;
 import org.hypertrace.core.grpcutils.context.RequestContext;
+import org.hypertrace.span.processing.config.service.utils.TimestampConverter;
 import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRule;
+import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleDetails;
+import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleMetadata;
 
 public class ExcludeSpanRulesConfigStore extends IdentifiedObjectStore<ExcludeSpanRule> {
 
   private static final String EXCLUDE_SPAN_RULES_RESOURCE_NAME = "exclude-span-rules";
   private static final String EXCLUDE_SPAN_RULES_CONFIG_RESOURCE_NAMESPACE =
       "exclude-span-rules-config";
+  private final TimestampConverter timestampConverter;
 
   @Inject
   public ExcludeSpanRulesConfigStore(
-      ConfigServiceGrpc.ConfigServiceBlockingStub configServiceBlockingStub) {
+      ConfigServiceGrpc.ConfigServiceBlockingStub configServiceBlockingStub,
+      TimestampConverter timestampConverter) {
     super(
         configServiceBlockingStub,
         EXCLUDE_SPAN_RULES_CONFIG_RESOURCE_NAMESPACE,
         EXCLUDE_SPAN_RULES_RESOURCE_NAME);
+    this.timestampConverter = timestampConverter;
   }
 
-  public List<ExcludeSpanRule> getAllData(RequestContext requestContext) {
+  public List<ExcludeSpanRuleDetails> getAllData(RequestContext requestContext) {
     return this.getAllObjects(requestContext).stream()
-        .map(ContextualConfigObject::getData)
+        .map(
+            contextualConfigObject ->
+                ExcludeSpanRuleDetails.newBuilder()
+                    .setRule(contextualConfigObject.getData())
+                    .setMetadata(
+                        ExcludeSpanRuleMetadata.newBuilder()
+                            .setCreationTimestamp(
+                                timestampConverter.convert(
+                                    contextualConfigObject.getCreationTimestamp()))
+                            .setLastUpdatedTimestamp(
+                                timestampConverter.convert(
+                                    contextualConfigObject.getLastUpdatedTimestamp()))
+                            .build())
+                    .build())
         .collect(Collectors.toUnmodifiableList());
   }
 
diff --git a/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/utils/TimestampConverter.java b/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/utils/TimestampConverter.java
@@ -0,0 +1,13 @@
+package org.hypertrace.span.processing.config.service.utils;
+
+import com.google.protobuf.Timestamp;
+import java.time.Instant;
+
+public class TimestampConverter {
+  public Timestamp convert(Instant instant) {
+    return Timestamp.newBuilder()
+        .setSeconds(instant.toEpochMilli())
+        .setNanos(instant.getNano())
+        .build();
+  }
+}
diff --git a/span-processing-config-service-impl/src/test/java/org/hypertrace/span/processing/config/service/SpanProcessingConfigServiceImplTest.java b/span-processing-config-service-impl/src/test/java/org/hypertrace/span/processing/config/service/SpanProcessingConfigServiceImplTest.java
@@ -2,14 +2,21 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+import com.google.protobuf.Timestamp;
 import java.util.List;
+import java.util.stream.Collectors;
 import org.hypertrace.config.service.test.MockGenericConfigService;
 import org.hypertrace.config.service.v1.ConfigServiceGrpc;
 import org.hypertrace.span.processing.config.service.store.ExcludeSpanRulesConfigStore;
+import org.hypertrace.span.processing.config.service.utils.TimestampConverter;
 import org.hypertrace.span.processing.config.service.v1.CreateExcludeSpanRuleRequest;
 import org.hypertrace.span.processing.config.service.v1.DeleteExcludeSpanRuleRequest;
 import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRule;
+import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleDetails;
 import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleInfo;
 import org.hypertrace.span.processing.config.service.v1.Field;
 import org.hypertrace.span.processing.config.service.v1.GetAllExcludeSpanRulesRequest;
@@ -29,6 +36,7 @@ class SpanProcessingConfigServiceImplTest {
   SpanProcessingConfigServiceGrpc.SpanProcessingConfigServiceBlockingStub
       spanProcessingConfigServiceStub;
   MockGenericConfigService mockGenericConfigService;
+  TimestampConverter timestampConverter;
 
   @BeforeEach
   void beforeEach() {
@@ -43,15 +51,20 @@ void beforeEach() {
     ConfigServiceGrpc.ConfigServiceBlockingStub genericStub =
         ConfigServiceGrpc.newBlockingStub(this.mockGenericConfigService.channel());
 
+    this.timestampConverter = mock(TimestampConverter.class);
     this.mockGenericConfigService
         .addService(
             new SpanProcessingConfigServiceImpl(
-                new ExcludeSpanRulesConfigStore(genericStub),
-                new SpanProcessingConfigRequestValidator()))
+                new ExcludeSpanRulesConfigStore(genericStub, this.timestampConverter),
+                new SpanProcessingConfigRequestValidator(),
+                this.timestampConverter))
         .start();
 
     this.spanProcessingConfigServiceStub =
         SpanProcessingConfigServiceGrpc.newBlockingStub(this.mockGenericConfigService.channel());
+
+    when(this.timestampConverter.convert(any()))
+        .thenReturn(Timestamp.newBuilder().setSeconds(100).build());
   }
 
   @AfterEach
@@ -61,7 +74,7 @@ void afterEach() {
 
   @Test
   void testCrud() {
-    ExcludeSpanRule firstCreatedExcludeSpanRule =
+    ExcludeSpanRuleDetails firstCreatedExcludeSpanRuleDetails =
         this.spanProcessingConfigServiceStub
             .createExcludeSpanRule(
                 CreateExcludeSpanRuleRequest.newBuilder()
@@ -78,7 +91,14 @@ void testCrud() {
                                             .setRightOperand(
                                                 SpanFilterValue.newBuilder().setStringValue("a")))))
                     .build())
-            .getRule();
+            .getRuleDetails();
+    ExcludeSpanRule firstCreatedExcludeSpanRule = firstCreatedExcludeSpanRuleDetails.getRule();
+    Timestamp expectedTimestamp = Timestamp.newBuilder().setSeconds(100).build();
+    assertEquals(
+        expectedTimestamp, firstCreatedExcludeSpanRuleDetails.getMetadata().getCreationTimestamp());
+    assertEquals(
+        expectedTimestamp,
+        firstCreatedExcludeSpanRuleDetails.getMetadata().getLastUpdatedTimestamp());
 
     ExcludeSpanRule secondCreatedExcludeSpanRule =
         this.spanProcessingConfigServiceStub
@@ -97,13 +117,16 @@ void testCrud() {
                                             .setRightOperand(
                                                 SpanFilterValue.newBuilder().setStringValue("a")))))
                     .build())
+            .getRuleDetails()
             .getRule();
 
     List<ExcludeSpanRule> excludeSpanRules =
         this.spanProcessingConfigServiceStub
-            .getAllExcludeSpanRules(
-                GetAllExcludeSpanRulesRequest.newBuilder().build().newBuilder().build())
-            .getRulesList();
+            .getAllExcludeSpanRules(GetAllExcludeSpanRulesRequest.newBuilder().build())
+            .getRuleDetailsList()
+            .stream()
+            .map(ExcludeSpanRuleDetails::getRule)
+            .collect(Collectors.toUnmodifiableList());
     assertEquals(2, excludeSpanRules.size());
     assertTrue(excludeSpanRules.contains(firstCreatedExcludeSpanRule));
     assertTrue(excludeSpanRules.contains(secondCreatedExcludeSpanRule));
@@ -126,13 +149,17 @@ void testCrud() {
                                             .setRightOperand(
                                                 SpanFilterValue.newBuilder().setStringValue("a")))))
                     .build())
+            .getRuleDetails()
             .getRule();
     assertEquals("updatedRuleName1", updatedFirstExcludeSpanRule.getRuleInfo().getName());
 
     excludeSpanRules =
         this.spanProcessingConfigServiceStub
             .getAllExcludeSpanRules(GetAllExcludeSpanRulesRequest.newBuilder().build())
-            .getRulesList();
+            .getRuleDetailsList()
+            .stream()
+            .map(ExcludeSpanRuleDetails::getRule)
+            .collect(Collectors.toUnmodifiableList());
     assertEquals(2, excludeSpanRules.size());
     assertTrue(excludeSpanRules.contains(updatedFirstExcludeSpanRule));
 
@@ -144,7 +171,10 @@ void testCrud() {
     excludeSpanRules =
         this.spanProcessingConfigServiceStub
             .getAllExcludeSpanRules(GetAllExcludeSpanRulesRequest.newBuilder().build())
-            .getRulesList();
+            .getRuleDetailsList()
+            .stream()
+            .map(ExcludeSpanRuleDetails::getRule)
+            .collect(Collectors.toUnmodifiableList());
     assertEquals(1, excludeSpanRules.size());
     assertEquals(secondCreatedExcludeSpanRule, excludeSpanRules.get(0));
   }
