Merge pull request #16 from ExpediaDotCom/blob-key-updates

use opentracing span in blob makekey
ExpediaDotCom · Sep 9, 2019 · 43d3daa · 43d3daa
2 parents e38ac99 + 419a41d
commit 43d3daa
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 38 deletions.
diff --git a/core/src/main/java/com/expedia/blobs/core/BlobContext.java b/core/src/main/java/com/expedia/blobs/core/BlobContext.java
@@ -49,11 +49,11 @@ default String getOperationId() {
      * @return string
      */
     default String makeKey(BlobType type) {
-        return String.format("%s_%s_%s_%s_%s",
+        return String.format("%s_%s_%s_%s",
                              getServiceName(),
                              getOperationName(),
                              getOperationId(),
-                             type.getType(), UUID.randomUUID().toString());
+                             type.getType());
     }
 
     /**

diff --git a/core/src/test/scala/com/expedia/blobs/core/BlobWriterImplSpec.scala b/core/src/test/scala/com/expedia/blobs/core/BlobWriterImplSpec.scala
@@ -28,7 +28,7 @@ class BlobWriterImplSpec extends FunSpec with GivenWhenThen with BeforeAndAfter
       captured.hasCaptured should be (true)
       And("blob's key is generated as expected")
       val capturedBlob = captured.getValue
-      """service1_operation1_.*_request_.*""".r.pattern.matcher(capturedBlob.build.getKey).matches() should be (true)
+      """service1_operation1_.*_request""".r.pattern.matcher(capturedBlob.build.getKey).matches() should be (true)
     }
     it("should call store to with a blob object that invokes the callback to serialize blob only " +
       "the first time when data is fetched") {

diff --git a/core/src/test/scala/com/expedia/blobs/core/io/AsyncSupportSpec.scala b/core/src/test/scala/com/expedia/blobs/core/io/AsyncSupportSpec.scala
@@ -68,7 +68,7 @@ class AsyncSupportSpec extends FunSpec with GivenWhenThen with BeforeAndAfter wi
     }
 
     it("should store a blob") {
-      Given(" a simple blob")
+      Given("a simple blob")
       val blob = Support.newBlob()
       val blobBuilder = mock[BlobWriterImpl.BlobBuilder]
 

diff --git a/...er/src/main/java/com/expedia/www/haystack/agent/blobs/server/api/BlobAgentGrpcServer.java b/...er/src/main/java/com/expedia/www/haystack/agent/blobs/server/api/BlobAgentGrpcServer.java
@@ -42,10 +42,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.io.StringWriter;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
 
 public class BlobAgentGrpcServer extends BlobAgentGrpc.BlobAgentImplBase {
     private final Logger LOGGER = LoggerFactory.getLogger(BlobAgentGrpcServer.class);
@@ -158,7 +155,7 @@ public void readBlobAsString(final BlobSearch blobSearch, final StreamObserver<F
     String parseBlob(ByteString content, String contentType) {
         String blob = null;
         try {
-            if (contentType == ContentType.FAST_INFOSET.getType()) {
+            if (Objects.equals(contentType, ContentType.FAST_INFOSET.getType())) {
                 blob = parseFastInfosetToString(content);
             } else {
                 blob = IOUtils.toString(content.toByteArray());

diff --git a/haystack-blobs/span-blob-context/pom.xml b/haystack-blobs/span-blob-context/pom.xml
@@ -11,16 +11,20 @@
     <artifactId>span-blob-context</artifactId>
     <packaging>jar</packaging>
 
+    <properties>
+        <opentracing.version>0.31.0</opentracing.version>
+    </properties>
+
     <dependencies>
         <dependency>
             <groupId>com.expedia.www</groupId>
             <artifactId>blobs-core</artifactId>
             <version>${parent.version}</version>
         </dependency>
         <dependency>
-            <groupId>com.expedia.www</groupId>
-            <artifactId>haystack-client-core</artifactId>
-            <version>${haystack-client-core.version}</version>
+            <groupId>io.opentracing</groupId>
+            <artifactId>opentracing-api</artifactId>
+            <version>${opentracing.version}</version>
         </dependency>
 
         <!--Test-->
@@ -94,6 +98,21 @@
             <plugin>
                 <groupId>org.jacoco</groupId>
                 <artifactId>jacoco-maven-plugin</artifactId>
+                <configuration>
+                    <haltOnFailure>true</haltOnFailure>
+                    <rules>
+                        <rule>
+                            <element>CLASS</element>
+                            <limits>
+                                <limit>
+                                    <counter>LINE</counter>
+                                    <value>COVEREDRATIO</value>
+                                    <minimum>0.00</minimum>
+                                </limit>
+                            </limits>
+                        </rule>
+                    </rules>
+                </configuration>
             </plugin>
         </plugins>
     </build>

diff --git a/haystack-blobs/span-blob-context/src/main/java/com/expedia/blobs/core/SpanBlobContext.java b/haystack-blobs/span-blob-context/src/main/java/com/expedia/blobs/core/SpanBlobContext.java
@@ -1,6 +1,6 @@
 package com.expedia.blobs.core;
 
-import com.expedia.www.haystack.client.Span;
+import io.opentracing.Span;
 import org.apache.commons.lang3.Validate;
 
 /**
@@ -9,27 +9,35 @@
  */
 
 public class SpanBlobContext implements BlobContext {
-    private Span span;
+
+    private final Span span;
+    private final String serviceName;
     private final static String PARTIAL_BLOB_KEY = "-blob";
+    private final String operationName;
 
     /**
      * constructor
-     * @param span for a specific service operation
+     * @param span span object
+     * @param serviceName for a specific service name, can't be null or empty
+     * @param operationName for a specific operation name
      */
+    public SpanBlobContext(Span span, String serviceName, String operationName) {
+        Validate.notNull(span, "span cannot be null in context");
+        Validate.notEmpty(serviceName, "service name cannot be null in context");
 
-    public SpanBlobContext(Span span) {
-        Validate.notNull(span, "Span cannot be null in context");
         this.span = span;
+        this.serviceName = serviceName;
+        this.operationName = operationName;
     }
 
     @Override
     public String getOperationName() {
-        return span.getOperationName();
+        return operationName;
     }
 
     @Override
     public String getServiceName() {
-        return span.getServiceName();
+        return serviceName;
     }
 
     /**

diff --git a/...k-blobs/span-blob-context/src/test/scala/com/expedia/blobs/core/SpanBlobContextSpec.scala b/...k-blobs/span-blob-context/src/test/scala/com/expedia/blobs/core/SpanBlobContextSpec.scala
@@ -1,35 +1,18 @@
 package com.expedia.blobs.core
 
-import com.expedia.www.haystack.client.Tracer
-import com.expedia.www.haystack.client.dispatchers.NoopDispatcher
-import com.expedia.www.haystack.client.metrics.NoopMetricsRegistry
-import org.junit.Assert
 import org.scalatest.easymock.EasyMockSugar
 import org.scalatest.{FunSpec, Matchers}
 
 class SpanBlobContextSpec extends FunSpec with Matchers with EasyMockSugar {
 
-  private val tracer = new Tracer.Builder(new NoopMetricsRegistry, "TestService", new NoopDispatcher).build()
-  private val span = tracer.buildSpan("TestOperation").start
-
   describe("com.expedia.blobs.core.SpanBlobContext") {
 
     it("should throw an error if span is not present") {
       val catchExpection = intercept[Exception] {
-        val spanBlobContext: SpanBlobContext = new SpanBlobContext(null)
+        val _ = new SpanBlobContext(null, "", "")
       }
 
-      catchExpection.getMessage shouldEqual "Span cannot be null in context"
-    }
-
-    it("should return the correct operation name") {
-      val operationName = new SpanBlobContext(span).getOperationName
-      Assert.assertEquals("TestOperation", operationName)
-    }
-
-    it("should return the correct service name") {
-      val serviceName = new SpanBlobContext(span).getServiceName
-      Assert.assertEquals("TestService", serviceName)
+      catchExpection.getMessage shouldEqual "span cannot be null in context"
     }
   }
 }