This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository 
https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git

commit ad3e655d464ff115d38775cda388a5404daaf509
Author: wujimin <[email protected]>
AuthorDate: Wed Aug 29 22:28:32 2018 +0800

    [SCB-859] move generate traceId logic from TracingFilter to invocation
---
 .../common/rest/AbstractRestInvocation.java        |  2 +-
 .../common/rest/filter/tracing/TracingFilter.java  | 75 -----------------
 ...servicecomb.common.rest.filter.HttpServerFilter |  1 -
 .../common/rest/TestAbstractRestInvocation.java    | 21 +++--
 .../rest/filter/tracing/TracingFilterTest.java     | 79 ------------------
 .../org/apache/servicecomb/core/Invocation.java    | 60 ++++++++++++++
 .../core/tracing/BraveTraceIdGenerator.java        | 12 +--
 .../servicecomb/core/tracing/TraceIdGenerator.java | 26 +++++-
 ...pache.servicecomb.core.tracing.TraceIdGenerator |  3 +-
 .../apache/servicecomb/core/TestInvocation.java    | 94 ++++++++++++++++++++++
 .../core/tracing/BraveTraceIdGeneratorTest.java    |  6 +-
 .../foundation/common/utils/SPIServiceUtils.java   | 15 ++++
 .../common/utils/TestSPIServiceUtils.java          | 61 ++++++++++++++
 13 files changed, 281 insertions(+), 174 deletions(-)

diff --git 
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
 
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
index a467f0b..65ef363 100644
--- 
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
+++ 
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
@@ -118,7 +118,7 @@ public abstract class AbstractRestInvocation {
       return;
     }
 
-    invocation.onStart();
+    invocation.onStart(requestEx);
     OperationMeta operationMeta = restOperationMeta.getOperationMeta();
 
     operationMeta.getExecutor().execute(() -> {
diff --git 
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/tracing/TracingFilter.java
 
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/tracing/TracingFilter.java
deleted file mode 100644
index 94b2fc5..0000000
--- 
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/filter/tracing/TracingFilter.java
+++ /dev/null
@@ -1,75 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.servicecomb.common.rest.filter.tracing;
-
-import org.apache.servicecomb.common.rest.filter.HttpServerFilter;
-import org.apache.servicecomb.core.Const;
-import org.apache.servicecomb.core.Invocation;
-import org.apache.servicecomb.core.tracing.BraveTraceIdGenerator;
-import org.apache.servicecomb.core.tracing.TraceIdGenerator;
-import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
-import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
-import org.apache.servicecomb.swagger.invocation.Response;
-import org.springframework.util.StringUtils;
-
-/**
- * Ensure the invocation contains traceId
- */
-public class TracingFilter implements HttpServerFilter {
-  private TraceIdGenerator traceIdGenerator = getTraceIdGenerator();
-
-  @Override
-  public int getOrder() {
-    return 0;
-  }
-
-  /**
-   * Ensure the invocation contains traceId
-   * @return {@code null}
-   */
-  @Override
-  public Response afterReceiveRequest(Invocation invocation, 
HttpServletRequestEx requestEx) {
-    if (!StringUtils.isEmpty(invocation.getContext(Const.TRACE_ID_NAME))) {
-      // if invocation context contains traceId, nothing needed to do
-      return null;
-    }
-
-    String traceId = requestEx.getHeader(Const.TRACE_ID_NAME);
-    if (!StringUtils.isEmpty(traceId)) {
-      // if request header contains traceId, move traceId into invocation 
context
-      invocation.addContext(Const.TRACE_ID_NAME, traceId);
-      return null;
-    }
-
-    // if traceId not found, generate a traceId
-    invocation.addContext(Const.TRACE_ID_NAME, 
traceIdGenerator.generateStringId());
-
-    return null;
-  }
-
-  /**
-   * nothing to do
-   */
-  @Override
-  public void beforeSendResponse(Invocation invocation, HttpServletResponseEx 
responseEx) {
-  }
-
-  protected TraceIdGenerator getTraceIdGenerator() {
-    return BraveTraceIdGenerator.INSTANCE;
-  }
-}
diff --git 
a/common/common-rest/src/main/resources/META-INF/services/org.apache.servicecomb.common.rest.filter.HttpServerFilter
 
b/common/common-rest/src/main/resources/META-INF/services/org.apache.servicecomb.common.rest.filter.HttpServerFilter
index 91e25af..13ee39b 100644
--- 
a/common/common-rest/src/main/resources/META-INF/services/org.apache.servicecomb.common.rest.filter.HttpServerFilter
+++ 
b/common/common-rest/src/main/resources/META-INF/services/org.apache.servicecomb.common.rest.filter.HttpServerFilter
@@ -16,4 +16,3 @@
 #
 
 org.apache.servicecomb.common.rest.filter.inner.ServerRestArgsFilter
-org.apache.servicecomb.common.rest.filter.tracing.TracingFilter
\ No newline at end of file
diff --git 
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
 
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
index 2b0fffa..dcc6110 100644
--- 
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
+++ 
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
@@ -102,6 +102,13 @@ public class TestAbstractRestInvocation {
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
+  class AbstractHttpServletRequestForTest extends AbstractHttpServletRequest {
+    @Override
+    public String getHeader(String name) {
+      return null;
+    }
+  }
+
   class AbstractRestInvocationForTest extends AbstractRestInvocation {
     @Override
     protected OperationLocator locateOperation(ServicePathManager 
servicePathManager) {
@@ -730,8 +737,7 @@ public class TestAbstractRestInvocation {
   @Test
   public void scheduleInvocationException(@Mocked OperationMeta operationMeta) 
{
     Executor executor = new ReactiveExecutor();
-    requestEx = new AbstractHttpServletRequest() {
-    };
+    requestEx = new AbstractHttpServletRequestForTest();
     requestEx.setAttribute(RestConst.REST_REQUEST, requestEx);
     new Expectations() {
       {
@@ -778,8 +784,7 @@ public class TestAbstractRestInvocation {
       }
     };
 
-    requestEx = new AbstractHttpServletRequest() {
-    };
+    requestEx = new AbstractHttpServletRequestForTest();
 
     restInvocation = new AbstractRestInvocationForTest() {
       @Override
@@ -818,15 +823,16 @@ public class TestAbstractRestInvocation {
     EventManager.register(subscriber);
 
     Executor executor = new ReactiveExecutor();
-    requestEx = new AbstractHttpServletRequest() {
-    };
+    requestEx = new AbstractHttpServletRequestForTest();
     requestEx.setAttribute(RestConst.REST_REQUEST, requestEx);
-    new Expectations() {
+    new Expectations(requestEx) {
       {
         restOperation.getOperationMeta();
         result = operationMeta;
         operationMeta.getExecutor();
         result = executor;
+        requestEx.getHeader(Const.TRACE_ID_NAME);
+        result = "tid";
       }
     };
 
@@ -846,6 +852,7 @@ public class TestAbstractRestInvocation {
     Assert.assertTrue(result.value);
     Assert.assertEquals(time, invocation.getStartTime());
     Assert.assertSame(invocation, eventHolder.value.getInvocation());
+    Assert.assertEquals("tid", invocation.getTraceId());
   }
 
   @Test
diff --git 
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/tracing/TracingFilterTest.java
 
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/tracing/TracingFilterTest.java
deleted file mode 100644
index 141acf8..0000000
--- 
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/filter/tracing/TracingFilterTest.java
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.servicecomb.common.rest.filter.tracing;
-
-import org.apache.servicecomb.core.Const;
-import org.apache.servicecomb.core.Invocation;
-import org.apache.servicecomb.core.tracing.TraceIdGenerator;
-import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
-import org.junit.Test;
-import org.mockito.Mockito;
-
-public class TracingFilterTest {
-  private static final TestTracingFilter FILTER = new TestTracingFilter();
-
-  @Test
-  public void testAfterReceiveRequestOnInvocationContainsTraceId() {
-    Invocation invocation = Mockito.mock(Invocation.class);
-    String traceId = "traceIdTest";
-    HttpServletRequestEx requestEx = Mockito.mock(HttpServletRequestEx.class);
-
-    
Mockito.when(invocation.getContext(Const.TRACE_ID_NAME)).thenReturn(traceId);
-
-    FILTER.afterReceiveRequest(invocation, requestEx);
-
-    Mockito.verify(requestEx, Mockito.times(0)).getHeader(Const.TRACE_ID_NAME);
-  }
-
-  @Test
-  public void testAfterReceiveRequestOnHeaderContainsTraceId() {
-    Invocation invocation = Mockito.mock(Invocation.class);
-    String traceId = "traceIdTest";
-    HttpServletRequestEx requestEx = Mockito.mock(HttpServletRequestEx.class);
-
-    Mockito.when(invocation.getContext(Const.TRACE_ID_NAME)).thenReturn(null);
-    Mockito.when(requestEx.getHeader(Const.TRACE_ID_NAME)).thenReturn(traceId);
-
-    FILTER.afterReceiveRequest(invocation, requestEx);
-
-    Mockito.verify(invocation).addContext(Const.TRACE_ID_NAME, traceId);
-  }
-
-  @Test
-  public void testAfterReceiveRequestOnGenerateTraceId() {
-    Invocation invocation = Mockito.mock(Invocation.class);
-    HttpServletRequestEx requestEx = Mockito.mock(HttpServletRequestEx.class);
-
-    Mockito.when(invocation.getContext(Const.TRACE_ID_NAME)).thenReturn(null);
-    Mockito.when(requestEx.getHeader(Const.TRACE_ID_NAME)).thenReturn(null);
-
-    FILTER.afterReceiveRequest(invocation, requestEx);
-
-    Mockito.verify(invocation).addContext(Const.TRACE_ID_NAME, 
TestTracingFilter.TRACE_ID);
-  }
-
-  static class TestTracingFilter extends TracingFilter {
-
-    static final String TRACE_ID = "" + Long.MAX_VALUE;
-
-    @Override
-    protected TraceIdGenerator getTraceIdGenerator() {
-      return () -> TRACE_ID;
-    }
-  }
-}
diff --git a/core/src/main/java/org/apache/servicecomb/core/Invocation.java 
b/core/src/main/java/org/apache/servicecomb/core/Invocation.java
index 6b5be13..ceb5f92 100644
--- a/core/src/main/java/org/apache/servicecomb/core/Invocation.java
+++ b/core/src/main/java/org/apache/servicecomb/core/Invocation.java
@@ -17,22 +17,33 @@
 
 package org.apache.servicecomb.core;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Executor;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.core.definition.SchemaMeta;
 import org.apache.servicecomb.core.event.InvocationFinishEvent;
 import org.apache.servicecomb.core.event.InvocationStartEvent;
 import org.apache.servicecomb.core.provider.consumer.ReferenceConfig;
+import org.apache.servicecomb.core.tracing.TraceIdGenerator;
 import org.apache.servicecomb.foundation.common.event.EventManager;
+import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
+import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
 import org.apache.servicecomb.swagger.invocation.AsyncResponse;
 import org.apache.servicecomb.swagger.invocation.InvocationType;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.apache.servicecomb.swagger.invocation.SwaggerInvocation;
 
 public class Invocation extends SwaggerInvocation {
+  private static final Collection<TraceIdGenerator> TRACE_ID_GENERATORS = 
loadTraceIdGenerators();
+
+  static Collection<TraceIdGenerator> loadTraceIdGenerators() {
+    return SPIServiceUtils.getPriorityHighestServices(generator -> 
generator.getName(), TraceIdGenerator.class);
+  }
+
   private ReferenceConfig referenceConfig;
 
   // 本次调用对应的schemaMeta
@@ -63,6 +74,20 @@ public class Invocation extends SwaggerInvocation {
 
   private boolean sync = true;
 
+  private HttpServletRequestEx requestEx;
+
+  public HttpServletRequestEx getRequestEx() {
+    return requestEx;
+  }
+
+  public String getTraceId() {
+    return getContext(Const.TRACE_ID_NAME);
+  }
+
+  public String getTraceId(String traceIdName) {
+    return getContext(traceIdName);
+  }
+
   public long getStartTime() {
     return startTime;
   }
@@ -188,11 +213,46 @@ public class Invocation extends SwaggerInvocation {
     return operationMeta.getMicroserviceQualifiedName();
   }
 
+  protected void initTraceId() {
+    for (TraceIdGenerator traceIdGenerator : TRACE_ID_GENERATORS) {
+      initTraceId(traceIdGenerator);
+    }
+  }
+
+  protected void initTraceId(TraceIdGenerator traceIdGenerator) {
+    if 
(!StringUtils.isEmpty(getTraceId(traceIdGenerator.getTraceIdKeyName()))) {
+      // if invocation context contains traceId, nothing needed to do
+      return;
+    }
+
+    if (requestEx == null) {
+      // it's a new consumer invocation, must generate a traceId
+      addContext(traceIdGenerator.getTraceIdKeyName(), 
traceIdGenerator.generate());
+      return;
+    }
+
+    String traceId = requestEx.getHeader(traceIdGenerator.getTraceIdKeyName());
+    if (!StringUtils.isEmpty(traceId)) {
+      // if request header contains traceId, save traceId into invocation 
context
+      addContext(traceIdGenerator.getTraceIdKeyName(), traceId);
+      return;
+    }
+
+    // if traceId not found, generate a traceId
+    addContext(traceIdGenerator.getTraceIdKeyName(), 
traceIdGenerator.generate());
+  }
+
   public void onStart() {
     this.startTime = System.nanoTime();
+    initTraceId();
     EventManager.post(new InvocationStartEvent(this));
   }
 
+  public void onStart(HttpServletRequestEx requestEx) {
+    this.requestEx = requestEx;
+    onStart();
+  }
+
   public void onStartExecute() {
     this.startExecutionTime = System.nanoTime();
   }
diff --git 
a/core/src/main/java/org/apache/servicecomb/core/tracing/BraveTraceIdGenerator.java
 
b/core/src/main/java/org/apache/servicecomb/core/tracing/BraveTraceIdGenerator.java
index c740b79..7a2eee0 100644
--- 
a/core/src/main/java/org/apache/servicecomb/core/tracing/BraveTraceIdGenerator.java
+++ 
b/core/src/main/java/org/apache/servicecomb/core/tracing/BraveTraceIdGenerator.java
@@ -17,16 +17,18 @@
 
 package org.apache.servicecomb.core.tracing;
 
+import org.apache.servicecomb.core.Const;
+
 import brave.internal.Platform;
 
 public class BraveTraceIdGenerator implements TraceIdGenerator {
-  public static final BraveTraceIdGenerator INSTANCE = new 
BraveTraceIdGenerator();
-
   @Override
-  public String generateStringId() {
-    return Long.toHexString(Platform.get().nextTraceIdHigh());
+  public String getTraceIdKeyName() {
+    return Const.TRACE_ID_NAME;
   }
 
-  private BraveTraceIdGenerator() {
+  @Override
+  public String generate() {
+    return Long.toHexString(Platform.get().nextTraceIdHigh());
   }
 }
diff --git 
a/core/src/main/java/org/apache/servicecomb/core/tracing/TraceIdGenerator.java 
b/core/src/main/java/org/apache/servicecomb/core/tracing/TraceIdGenerator.java
index 94bd9f7..7f807be 100644
--- 
a/core/src/main/java/org/apache/servicecomb/core/tracing/TraceIdGenerator.java
+++ 
b/core/src/main/java/org/apache/servicecomb/core/tracing/TraceIdGenerator.java
@@ -18,5 +18,29 @@
 package org.apache.servicecomb.core.tracing;
 
 public interface TraceIdGenerator {
-  String generateStringId();
+  default int getOrder() {
+    return 1000;
+  }
+
+  /**
+   * <pre>
+   *   for generators have the same name, will only use the minimum order 
instance
+   *   not use getTraceIdKeyName to control this logic, because most customers 
not want to generate multiple traceIds
+   * </pre>
+   * @return generator name
+   */
+  default String getName() {
+    return "default";
+  }
+
+  /**
+   *
+   * @return trance id key name
+   * <pre>
+   * default value is X-B3-TraceId to work with zipkin
+   * </pre>
+   */
+  String getTraceIdKeyName();
+
+  String generate();
 }
diff --git 
a/common/common-rest/src/main/resources/META-INF/services/org.apache.servicecomb.common.rest.filter.HttpServerFilter
 
b/core/src/main/resources/META-INF/services/org.apache.servicecomb.core.tracing.TraceIdGenerator
similarity index 85%
copy from 
common/common-rest/src/main/resources/META-INF/services/org.apache.servicecomb.common.rest.filter.HttpServerFilter
copy to 
core/src/main/resources/META-INF/services/org.apache.servicecomb.core.tracing.TraceIdGenerator
index 91e25af..3a57a7d 100644
--- 
a/common/common-rest/src/main/resources/META-INF/services/org.apache.servicecomb.common.rest.filter.HttpServerFilter
+++ 
b/core/src/main/resources/META-INF/services/org.apache.servicecomb.core.tracing.TraceIdGenerator
@@ -15,5 +15,4 @@
 # limitations under the License.
 #
 
-org.apache.servicecomb.common.rest.filter.inner.ServerRestArgsFilter
-org.apache.servicecomb.common.rest.filter.tracing.TracingFilter
\ No newline at end of file
+org.apache.servicecomb.core.tracing.BraveTraceIdGenerator
\ No newline at end of file
diff --git a/core/src/test/java/org/apache/servicecomb/core/TestInvocation.java 
b/core/src/test/java/org/apache/servicecomb/core/TestInvocation.java
index 0efc783..c0e98b7 100644
--- a/core/src/test/java/org/apache/servicecomb/core/TestInvocation.java
+++ b/core/src/test/java/org/apache/servicecomb/core/TestInvocation.java
@@ -16,14 +16,21 @@
  */
 package org.apache.servicecomb.core;
 
+import java.util.Arrays;
+
 import javax.xml.ws.Holder;
 
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.core.event.InvocationFinishEvent;
 import org.apache.servicecomb.core.event.InvocationStartEvent;
 import org.apache.servicecomb.core.provider.consumer.ReferenceConfig;
+import org.apache.servicecomb.core.tracing.BraveTraceIdGenerator;
+import org.apache.servicecomb.core.tracing.TraceIdGenerator;
 import org.apache.servicecomb.foundation.common.event.EventManager;
+import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
+import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
 import org.apache.servicecomb.swagger.invocation.Response;
+import org.hamcrest.Matchers;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
@@ -32,6 +39,7 @@ import org.junit.Test;
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.Subscribe;
 
+import mockit.Expectations;
 import mockit.Mock;
 import mockit.MockUp;
 import mockit.Mocked;
@@ -144,4 +152,90 @@ public class TestInvocation {
     Assert.assertSame(invocation.getHandlerContext(), 
invocation.getLocalContext());
     Assert.assertEquals(1, (int) invocation.getLocalContext("k"));
   }
+
+  @Test
+  public void traceId_fromContext(@Mocked ReferenceConfig referenceConfig) {
+    Invocation invocation = new Invocation(referenceConfig, operationMeta, 
swaggerArguments);
+    invocation.addContext(Const.TRACE_ID_NAME, "abc");
+
+    invocation.onStart();
+
+    Assert.assertEquals("abc", invocation.getTraceId());
+    Assert.assertEquals("abc", invocation.getTraceId(Const.TRACE_ID_NAME));
+  }
+
+  @Test
+  public void traceId_consumerCreateTraceId(@Mocked ReferenceConfig 
referenceConfig) {
+    TraceIdGenerator generator = 
SPIServiceUtils.getTargetService(TraceIdGenerator.class, 
BraveTraceIdGenerator.class);
+    new Expectations(generator) {
+      {
+        generator.generate();
+        result = "abc";
+      }
+    };
+    Invocation invocation = new Invocation(referenceConfig, operationMeta, 
swaggerArguments);
+
+    invocation.onStart();
+
+    Assert.assertEquals("abc", invocation.getTraceId());
+    Assert.assertEquals("abc", invocation.getTraceId(Const.TRACE_ID_NAME));
+  }
+
+  @Test
+  public void traceId_fromRequest(@Mocked Endpoint endpoint, @Mocked 
HttpServletRequestEx requestEx) {
+    new Expectations() {
+      {
+        requestEx.getHeader(Const.TRACE_ID_NAME);
+        result = "abc";
+      }
+    };
+    Invocation invocation = new Invocation(endpoint, operationMeta, 
swaggerArguments);
+
+    invocation.onStart(requestEx);
+
+    Assert.assertEquals("abc", invocation.getTraceId());
+    Assert.assertEquals("abc", invocation.getTraceId(Const.TRACE_ID_NAME));
+  }
+
+  @Test
+  public void traceId_producerCreateTraceId(@Mocked Endpoint endpoint, @Mocked 
HttpServletRequestEx requestEx) {
+    TraceIdGenerator generator = 
SPIServiceUtils.getTargetService(TraceIdGenerator.class, 
BraveTraceIdGenerator.class);
+    new Expectations(generator) {
+      {
+        generator.generate();
+        result = "abc";
+      }
+    };
+    Invocation invocation = new Invocation(endpoint, operationMeta, 
swaggerArguments);
+
+    invocation.onStart(requestEx);
+
+    Assert.assertEquals("abc", invocation.getTraceId());
+    Assert.assertEquals("abc", invocation.getTraceId(Const.TRACE_ID_NAME));
+  }
+
+  @Test
+  public void traceIdGeneratorInit(@Mocked TraceIdGenerator gen1, @Mocked 
TraceIdGenerator gen2,
+      @Mocked TraceIdGenerator gen3, @Mocked TraceIdGenerator gen4) {
+    new Expectations(SPIServiceUtils.class) {
+      {
+        gen1.getName();
+        result = "zipkin";
+
+        gen3.getName();
+        result = "apm";
+
+        gen2.getName();
+        result = "zipkin";
+
+        gen4.getName();
+        result = "apm";
+
+        SPIServiceUtils.getOrLoadSortedService(TraceIdGenerator.class);
+        result = Arrays.asList(gen1, gen3, gen2, gen4);
+      }
+    };
+
+    Assert.assertThat(Invocation.loadTraceIdGenerators(), 
Matchers.contains(gen1, gen3));
+  }
 }
diff --git 
a/core/src/test/java/org/apache/servicecomb/core/tracing/BraveTraceIdGeneratorTest.java
 
b/core/src/test/java/org/apache/servicecomb/core/tracing/BraveTraceIdGeneratorTest.java
index d9c0bea..8963cd2 100644
--- 
a/core/src/test/java/org/apache/servicecomb/core/tracing/BraveTraceIdGeneratorTest.java
+++ 
b/core/src/test/java/org/apache/servicecomb/core/tracing/BraveTraceIdGeneratorTest.java
@@ -26,10 +26,10 @@ public class BraveTraceIdGeneratorTest {
 
   @Test
   public void generateStringId() {
-    TraceIdGenerator traceIdGenerator = BraveTraceIdGenerator.INSTANCE;
-    assertNotEquals(traceIdGenerator.generateStringId(), 
traceIdGenerator.generateStringId());
+    TraceIdGenerator traceIdGenerator = new BraveTraceIdGenerator();
+    assertNotEquals(traceIdGenerator.generate(), traceIdGenerator.generate());
 
-    String traceId = traceIdGenerator.generateStringId();
+    String traceId = traceIdGenerator.generate();
     try {
       Long.parseLong(traceId, 16);
     } catch (NumberFormatException e) {
diff --git 
a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java
 
b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java
index 9cbfc86..ea8a0bc 100644
--- 
a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java
+++ 
b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java
@@ -20,12 +20,15 @@ package org.apache.servicecomb.foundation.common.utils;
 import java.lang.reflect.Method;
 import java.util.AbstractMap.SimpleEntry;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.ServiceLoader;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import org.slf4j.Logger;
@@ -129,6 +132,18 @@ public final class SPIServiceUtils {
     return services.get(0);
   }
 
+  public static <T> Collection<T> getPriorityHighestServices(Function<T, 
String> keyFunc, Class<T> serviceType) {
+    List<T> services = getOrLoadSortedService(serviceType);
+    if (services.isEmpty()) {
+      LOGGER.info("Can not find SPI service for {}", serviceType.getName());
+      return null;
+    }
+
+    Map<String, T> map = new HashMap<>();
+    services.forEach(instance -> map.putIfAbsent(keyFunc.apply(instance), 
instance));
+    return map.values();
+  }
+
   @SuppressWarnings("unchecked")
   public static <T, IMPL> IMPL getTargetService(Class<T> serviceType, 
Class<IMPL> implType) {
     List<T> services = getOrLoadSortedService(serviceType);
diff --git 
a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java
 
b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java
index 2bb136b..6b26ff9 100644
--- 
a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java
+++ 
b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java
@@ -93,4 +93,65 @@ public class TestSPIServiceUtils {
   public void getPriorityHighestService_null() {
     
Assert.assertNull(SPIServiceUtils.getPriorityHighestService(SPIServiceDef0.class));
   }
+
+  interface PriorityIntf {
+    String getName();
+
+    int getOrder();
+  }
+
+  public class PriorityImpl implements PriorityIntf {
+    private final String name;
+
+    private final int order;
+
+    public PriorityImpl(String name, int order) {
+      this.name = name;
+      this.order = order;
+    }
+
+    @Override
+    public String getName() {
+      return name;
+    }
+
+    @Override
+    public int getOrder() {
+      return order;
+    }
+
+    @Override
+    public String toString() {
+      return "PriorityImpl{" +
+          "name='" + name + '\'' +
+          ", order=" + order +
+          '}';
+    }
+  }
+
+  @Test
+  public void getPriorityHighestServices() {
+    Map<String, PriorityIntf> instances = new LinkedHashMap<>();
+    instances.putIfAbsent("1", new PriorityImpl("n1", 0));
+    instances.putIfAbsent("2", new PriorityImpl("n1", -1));
+    instances.putIfAbsent("3", new PriorityImpl("n1", 1));
+    instances.putIfAbsent("4", new PriorityImpl("n2", 0));
+    instances.putIfAbsent("5", new PriorityImpl("n2", -1));
+    instances.putIfAbsent("6", new PriorityImpl("n2", 1));
+
+    ServiceLoader<PriorityIntf> serviceLoader = 
ServiceLoader.load(PriorityIntf.class);
+    Deencapsulation.setField(serviceLoader, "providers", instances);
+    new Expectations(ServiceLoader.class) {
+      {
+        ServiceLoader.load(PriorityIntf.class);
+        result = serviceLoader;
+      }
+    };
+
+    Assert.assertThat(SPIServiceUtils.getPriorityHighestServices(inst -> 
inst.getName(), PriorityIntf.class),
+        Matchers.containsInAnyOrder(instances.get("2"), instances.get("5")));
+
+    Map<Class<?>, List<Object>> cache = 
Deencapsulation.getField(SPIServiceUtils.class, "cache");
+    cache.clear();
+  }
 }

Reply via email to