Merge branch 'ENG-12057' of https://github.com/hypertrace/config-service

 into ENG-12057

config-proto-converter/build.gradle.kts

@@ -6,4 +6,9 @@ plugins {
 dependencies {
   api(libs.grpc.protobuf)
   implementation(libs.protobuf.javautil)
+  constraints {
+    implementation(libs.gson) {
+      because("https://snyk.io/vuln/SNYK-JAVA-COMGOOGLECODEGSON-1730327")
+    }
+  }
 }

gradle/libs.versions.toml

@@ -1,7 +1,8 @@
 [versions]
 protoc = "3.17.3"
-grpc = "1.39.0"
-hypertrace-grpcutils = "0.5.2"
+grpc = "1.42.0"
+gson = "2.8.9"
+hypertrace-grpcutils = "0.6.2"
 hypertrace-framework = "0.1.29"
 lombok = "1.18.20"
 jackson = "2.12.4"
@@ -40,6 +41,7 @@ grpc-core = { module = "io.grpc:grpc-core", version.ref = "grpc" }
 grpc-netty = { module = "io.grpc:grpc-netty", version.ref = "grpc" }
 protobuf-javautil = { module = "com.google.protobuf:protobuf-java-util", version.ref = "protoc" }
 protobuf-java = { module = "com.google.protobuf:protobuf-java", version.ref = "protoc" }
+gson = { module = "com.google.code.gson:gson", version.ref = "gson" }
 
 kafka-protobuf-serializer = { module = "io.confluent:kafka-protobuf-serializer", version.ref = "confluent" }
 

labels-config-service-api/src/main/proto/org/hypertrace/label/config/service/v1/label.proto

@@ -6,9 +6,12 @@ option java_multiple_files = true;
 
 message Label {
   string id = 1;
-  string key = 2;
+  LabelData data = 2;
+  optional string created_by_application_rule_id = 3;
 }
 
-message CreateLabel {
-  string key = 2;
+message LabelData {
+  string key = 1;
+  optional string color = 2;
+  optional string description = 3;
 }

labels-config-service-api/src/main/proto/org/hypertrace/label/config/service/v1/labels_config_service.proto

@@ -26,7 +26,8 @@ service LabelsConfigService {
 }
 
 message CreateLabelRequest {
-  CreateLabel label = 1;
+  LabelData data = 1;
+  optional string created_by_application_rule_id = 2;
 }
 
 message CreateLabelResponse {
@@ -49,7 +50,8 @@ message GetLabelsResponse {
 }
 
 message UpdateLabelRequest {
-  Label label = 1;
+  string id = 1;
+  LabelData data = 2;
 }
 
 message UpdateLabelResponse {

labels-config-service-impl/src/main/java/org/hypertrace/label/config/service/LabelsConfigServiceImpl.java

@@ -13,7 +13,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
@@ -26,7 +25,6 @@
 import org.hypertrace.config.service.v1.ConfigServiceGrpc;
 import org.hypertrace.core.grpcutils.client.RequestContextClientCallCredsProviderFactory;
 import org.hypertrace.core.grpcutils.context.RequestContext;
-import org.hypertrace.label.config.service.v1.CreateLabel;
 import org.hypertrace.label.config.service.v1.CreateLabelRequest;
 import org.hypertrace.label.config.service.v1.CreateLabelResponse;
 import org.hypertrace.label.config.service.v1.DeleteLabelRequest;
@@ -36,6 +34,7 @@
 import org.hypertrace.label.config.service.v1.GetLabelsRequest;
 import org.hypertrace.label.config.service.v1.GetLabelsResponse;
 import org.hypertrace.label.config.service.v1.Label;
+import org.hypertrace.label.config.service.v1.LabelData;
 import org.hypertrace.label.config.service.v1.LabelsConfigServiceGrpc;
 import org.hypertrace.label.config.service.v1.UpdateLabelRequest;
 import org.hypertrace.label.config.service.v1.UpdateLabelResponse;
@@ -76,7 +75,9 @@ public LabelsConfigServiceImpl(
               .collect(Collectors.toUnmodifiableMap(Label::getId, Function.identity()));
       systemLabelsKeyLabelMap =
           systemLabels.stream()
-              .collect(Collectors.toUnmodifiableMap(Label::getKey, Function.identity()));
+              .collect(
+                  Collectors.toUnmodifiableMap(
+                      (label) -> label.getData().getKey(), Function.identity()));
     } else {
       systemLabels = Collections.emptyList();
       systemLabelsIdLabelMap = Collections.emptyMap();
@@ -106,18 +107,21 @@ public void createLabel(
       Lock labelsLock = this.stripedLabelsLock.get(requestContext.getTenantId());
       if (labelsLock.tryLock(WAIT_TIME.getSeconds(), TimeUnit.SECONDS)) {
         try {
-          CreateLabel createLabel = request.getLabel();
-          if (systemLabelsKeyLabelMap.containsKey(createLabel.getKey())
-              || isDuplicateKey(createLabel.getKey())) {
+          LabelData labelData = request.getData();
+          if (systemLabelsKeyLabelMap.containsKey(labelData.getKey())
+              || isDuplicateKey(labelData.getKey())) {
             // Creating a label with a name that clashes with one of system labels name
             responseObserver.onError(new StatusRuntimeException(Status.ALREADY_EXISTS));
             return;
           }
           Label label =
-              Label.newBuilder()
-                  .setId(UUID.randomUUID().toString())
-                  .setKey(createLabel.getKey())
-                  .build();
+              Label.newBuilder().setId(UUID.randomUUID().toString()).setData(labelData).build();
+          if (request.hasCreatedByApplicationRuleId()) {
+            label =
+                label.toBuilder()
+                    .setCreatedByApplicationRuleId(request.getCreatedByApplicationRuleId())
+                    .build();
+          }
           Label createdLabel = labelStore.upsertObject(requestContext, label).getData();
           responseObserver.onNext(CreateLabelResponse.newBuilder().setLabel(createdLabel).build());
           responseObserver.onCompleted();
@@ -157,8 +161,7 @@ public void getLabel(GetLabelRequest request, StreamObserver<GetLabelResponse> r
   public void getLabels(
       GetLabelsRequest request, StreamObserver<GetLabelsResponse> responseObserver) {
     RequestContext requestContext = RequestContext.CURRENT.get();
-    List<Label> allLabels = new ArrayList<>();
-    allLabels.addAll(systemLabels);
+    List<Label> allLabels = new ArrayList<>(systemLabels);
     List<Label> tenantLabels =
         labelStore.getAllObjects(requestContext).stream()
             .map(ContextualConfigObject::getData)
@@ -172,28 +175,29 @@ public void getLabels(
   public void updateLabel(
       UpdateLabelRequest request, StreamObserver<UpdateLabelResponse> responseObserver) {
     try {
-      Label updatedLabelInReq = request.getLabel();
+      LabelData updateLabelData = request.getData();
       RequestContext requestContext = RequestContext.CURRENT.get();
       Lock labelsLock = this.stripedLabelsLock.get(requestContext.getTenantId());
       if (labelsLock.tryLock(WAIT_TIME.getSeconds(), TimeUnit.SECONDS)) {
         try {
-          if (systemLabelsIdLabelMap.containsKey(updatedLabelInReq.getId())
-              || systemLabelsKeyLabelMap.containsKey(updatedLabelInReq.getKey())) {
+          if (systemLabelsIdLabelMap.containsKey(request.getId())
+              || systemLabelsKeyLabelMap.containsKey(updateLabelData.getKey())) {
             // Updating a system label will error
             responseObserver.onError(new StatusRuntimeException(Status.INVALID_ARGUMENT));
             return;
           }
-          if (isDuplicateKey(updatedLabelInReq.getKey())) {
+          if (isDuplicateKey(updateLabelData.getKey())) {
             responseObserver.onError(new StatusRuntimeException(Status.ALREADY_EXISTS));
             return;
           }
-          labelStore
-              .getData(requestContext, updatedLabelInReq.getId())
-              .orElseThrow(Status.NOT_FOUND::asRuntimeException);
-          Label updatedLabelInRes =
-              labelStore.upsertObject(requestContext, updatedLabelInReq).getData();
+          Label oldLabel =
+              labelStore
+                  .getData(requestContext, request.getId())
+                  .orElseThrow(Status.NOT_FOUND::asRuntimeException);
+          Label updateLabel = oldLabel.toBuilder().setData(updateLabelData).build();
+          Label updateLabelInRes = labelStore.upsertObject(requestContext, updateLabel).getData();
           responseObserver.onNext(
-              UpdateLabelResponse.newBuilder().setLabel(updatedLabelInRes).build());
+              UpdateLabelResponse.newBuilder().setLabel(updateLabelInRes).build());
           responseObserver.onCompleted();
         } finally {
           labelsLock.unlock();
@@ -243,8 +247,9 @@ private boolean isDuplicateKey(String key) {
         labelStore.getAllObjects(requestContext).stream()
             .map(ContextualConfigObject::getData)
             .collect(Collectors.toUnmodifiableList());
-    Optional<Label> match =
-        labelList.stream().filter(label -> label.getKey().equals(key)).findAny();
-    return match.isPresent();
+    return labelList.stream()
+        .map(Label::getData)
+        .map(LabelData::getKey)
+        .anyMatch(labelKey -> labelKey.equals(key));
   }
 }

labels-config-service-impl/src/test/java/org/hypertrace/label/config/service/LabelsConfigServiceImplTest.java

@@ -15,16 +15,17 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.hypertrace.config.service.change.event.api.ConfigChangeEventGenerator;
 import org.hypertrace.config.service.test.MockGenericConfigService;
-import org.hypertrace.label.config.service.v1.CreateLabel;
 import org.hypertrace.label.config.service.v1.CreateLabelRequest;
 import org.hypertrace.label.config.service.v1.CreateLabelResponse;
 import org.hypertrace.label.config.service.v1.DeleteLabelRequest;
 import org.hypertrace.label.config.service.v1.GetLabelRequest;
 import org.hypertrace.label.config.service.v1.GetLabelResponse;
 import org.hypertrace.label.config.service.v1.GetLabelsRequest;
 import org.hypertrace.label.config.service.v1.Label;
+import org.hypertrace.label.config.service.v1.LabelData;
 import org.hypertrace.label.config.service.v1.LabelsConfigServiceGrpc;
 import org.hypertrace.label.config.service.v1.LabelsConfigServiceGrpc.LabelsConfigServiceBlockingStub;
 import org.hypertrace.label.config.service.v1.UpdateLabelRequest;
@@ -39,11 +40,16 @@ public final class LabelsConfigServiceImplTest {
   MockGenericConfigService mockGenericConfigService;
   List<Label> systemLabels =
       Arrays.asList("Critical", "Sensitive", "External", "Sentry").stream()
-          .map(key -> Label.newBuilder().setId(key).setKey(key).build())
+          .map(
+              key ->
+                  Label.newBuilder()
+                      .setId(key)
+                      .setData(LabelData.newBuilder().setKey(key).build())
+                      .build())
           .collect(Collectors.toList());
-  List<CreateLabel> createLabelsList =
-      Arrays.asList(0, 1, 2, 3, 4).stream()
-          .map(id -> CreateLabel.newBuilder().setKey("Label-" + id).build())
+  List<LabelData> createLabelDataList =
+      Stream.of(0, 1, 2, 3, 4)
+          .map(id -> LabelData.newBuilder().setKey("Label-" + id).build())
           .collect(Collectors.toList());
 
   @BeforeEach
@@ -54,7 +60,9 @@ void setUp() {
     systemLabelsConfigMap.put(
         "system.labels",
         systemLabels.stream()
-            .map(systemLabel -> Map.of("id", systemLabel.getId(), "key", systemLabel.getKey()))
+            .map(
+                systemLabel ->
+                    Map.of("id", systemLabel.getId(), "data.key", systemLabel.getData().getKey()))
             .collect(Collectors.toList()));
     Map<String, Object> configMap = new HashMap<>();
     configMap.put("labels.config.service", systemLabelsConfigMap);
@@ -74,30 +82,28 @@ void afterEach() {
 
   private List<Label> createLabels() {
     // Creating or inserting labels
-    return createLabelsList.stream()
+    return createLabelDataList.stream()
         .map(
             createLabel -> {
               CreateLabelResponse response =
                   labelConfigStub.createLabel(
-                      CreateLabelRequest.newBuilder().setLabel(createLabel).build());
+                      CreateLabelRequest.newBuilder().setData(createLabel).build());
               return response.getLabel();
             })
         .collect(Collectors.toList());
   }
 
   @Test
   void test_createLabel() {
-    List<CreateLabel> createdLabelsList =
-        createLabels().stream()
-            .map(label -> CreateLabel.newBuilder().setKey(label.getKey()).build())
-            .collect(Collectors.toList());
-    assertEquals(createLabelsList, createdLabelsList);
+    List<LabelData> createdLabelsList =
+        createLabels().stream().map(Label::getData).collect(Collectors.toList());
+    assertEquals(createdLabelsList, createdLabelsList);
     Throwable exception =
         assertThrows(
             StatusRuntimeException.class,
             () -> {
               labelConfigStub.createLabel(
-                  CreateLabelRequest.newBuilder().setLabel(createLabelsList.get(0)).build());
+                  CreateLabelRequest.newBuilder().setData(createLabelDataList.get(0)).build());
             });
     assertEquals(Status.ALREADY_EXISTS, Status.fromThrowable(exception));
   }
@@ -107,7 +113,7 @@ void test_system_createLabel() {
     for (Label systemLabel : systemLabels) {
       CreateLabelRequest createLabelRequest =
           CreateLabelRequest.newBuilder()
-              .setLabel(CreateLabel.newBuilder().setKey(systemLabel.getKey()).build())
+              .setData(LabelData.newBuilder().setKey(systemLabel.getData().getKey()).build())
               .build();
       Throwable exception =
           assertThrows(
@@ -121,7 +127,10 @@ void test_system_createLabel() {
 
   @Test
   void test_getLabel() {
-    List<Label> createdLabelsList = createLabels();
+    List<Label> createdLabelsList =
+        createLabels().stream()
+            .map(label -> label.toBuilder().build())
+            .collect(Collectors.toList());
     // Querying each label one by one for the created or inserted labels in the previous step
     List<Label> getLabelList =
         createdLabelsList.stream()
@@ -174,10 +183,10 @@ void test_updateLabel() {
             () -> {
               labelConfigStub.updateLabel(
                   UpdateLabelRequest.newBuilder()
-                      .setLabel(
-                          Label.newBuilder()
-                              .setId(createdLabelsList.get(1).getId())
-                              .setKey(createdLabelsList.get(0).getKey())
+                      .setId(createdLabelsList.get(1).getId())
+                      .setData(
+                          LabelData.newBuilder()
+                              .setKey(createdLabelsList.get(0).getData().getKey())
                               .build())
                       .build());
             });
@@ -187,15 +196,22 @@ void test_updateLabel() {
         createdLabelsList.stream()
             .map(
                 label ->
-                    Label.newBuilder().setId(label.getId()).setKey(label.getKey() + "new").build())
+                    Label.newBuilder()
+                        .setId(label.getId())
+                        .setData(
+                            LabelData.newBuilder().setKey(label.getData().getKey() + "new").build())
+                        .build())
             .collect(Collectors.toList());
     List<Label> updatedLabelsList =
         updateLabelsList.stream()
             .map(
                 updateLabel -> {
                   UpdateLabelResponse response =
                       labelConfigStub.updateLabel(
-                          UpdateLabelRequest.newBuilder().setLabel(updateLabel).build());
+                          UpdateLabelRequest.newBuilder()
+                              .setId(updateLabel.getId())
+                              .setData(updateLabel.getData())
+                              .build());
                   return response.getLabel();
                 })
             .collect(Collectors.toList());
@@ -207,7 +223,8 @@ void test_updateLabel() {
             () -> {
               labelConfigStub.updateLabel(
                   UpdateLabelRequest.newBuilder()
-                      .setLabel(Label.newBuilder().setId("1").setKey("API-X").build())
+                      .setId("1")
+                      .setData(LabelData.newBuilder().setKey("API-X").build())
                       .build());
             });
     assertEquals(Status.NOT_FOUND, Status.fromThrowable(exception));
@@ -219,7 +236,8 @@ void test_system_updateLabel() {
     for (Label systemLabel : systemLabels) {
       UpdateLabelRequest updateLabelRequest =
           UpdateLabelRequest.newBuilder()
-              .setLabel(Label.newBuilder().setId(systemLabel.getId()).setKey("1").build())
+              .setId(systemLabel.getId())
+              .setData(LabelData.newBuilder().setKey("1").build())
               .build();
       Throwable exception =
           assertThrows(
@@ -232,11 +250,8 @@ void test_system_updateLabel() {
     for (Label systemLabel : systemLabels) {
       UpdateLabelRequest updateLabelRequest =
           UpdateLabelRequest.newBuilder()
-              .setLabel(
-                  Label.newBuilder()
-                      .setId(createdLabelsList.get(0).getId())
-                      .setKey(systemLabel.getKey())
-                      .build())
+              .setId(createdLabelsList.get(0).getId())
+              .setData(LabelData.newBuilder().setKey(systemLabel.getData().getKey()).build())
               .build();
       Throwable exception =
           assertThrows(