Skip to content

Commit 379318f

Browse files
steveraosteverao
committed
Address review comments
1 parent e2c19d0 commit 379318f

6 files changed

Lines changed: 41 additions & 31 deletions

File tree

‎instrumentation/apache-dubbo-2.7/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/DubboRequest.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package io.opentelemetry.instrumentation.apachedubbo.v2_7;
77

88
import com.google.auto.value.AutoValue;
9+
import io.opentelemetry.instrumentation.apachedubbo.v2_7.internal.DubboRegistryUtil;
910
import java.net.InetSocketAddress;
1011
import javax.annotation.Nullable;
1112
import org.apache.dubbo.common.URL;
@@ -26,7 +27,12 @@ static DubboRequest create(RpcInvocation invocation, RpcContext context) {
2627
context.getLocalAddress());
2728
}
2829

29-
public abstract RpcInvocation invocation();
30+
abstract RpcInvocation invocation();
31+
32+
@Nullable
33+
public String registryAddress() {
34+
return DubboRegistryUtil.extractRegistryAddress(invocation());
35+
}
3036

3137
public abstract RpcContext context();
3238

‎instrumentation/apache-dubbo-2.7/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/internal/DubboClientNetworkAttributesGetter.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package io.opentelemetry.instrumentation.apachedubbo.v2_7.internal;
77

88
import static io.opentelemetry.instrumentation.apachedubbo.v2_7.internal.DubboRegistryUtil.buildServiceTarget;
9-
import static io.opentelemetry.instrumentation.apachedubbo.v2_7.internal.DubboRegistryUtil.extractRegistryAddress;
109

1110
import io.opentelemetry.instrumentation.apachedubbo.v2_7.DubboRequest;
1211
import io.opentelemetry.instrumentation.api.semconv.network.NetworkAttributesGetter;
@@ -25,7 +24,7 @@ public final class DubboClientNetworkAttributesGetter
2524
@Nullable
2625
@Override
2726
public String getServerAddress(DubboRequest request) {
28-
String registryAddress = extractRegistryAddress(request.invocation());
27+
String registryAddress = request.registryAddress();
2928
if (registryAddress != null) {
3029
return registryAddress + "/" + buildServiceTarget(request.url());
3130
}
@@ -35,7 +34,7 @@ public String getServerAddress(DubboRequest request) {
3534
@Nullable
3635
@Override
3736
public Integer getServerPort(DubboRequest request) {
38-
if (extractRegistryAddress(request.invocation()) != null) {
37+
if (request.registryAddress() != null) {
3938
return null;
4039
}
4140
return request.url().getPort();

‎instrumentation/apache-dubbo-2.7/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/internal/DubboRegistryUtil.java‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,21 @@ public final class DubboRegistryUtil {
3737

3838
/**
3939
* Used by {@link RegistryCapturingInvoker} while the cluster delegate runs (including into
40-
* consumer protocol filters).
40+
* consumer protocol filters). Returns the previous value so callers can restore it.
4141
*/
42-
static void pushCapturedRegistryAddress(String address) {
42+
@Nullable
43+
static String pushCapturedRegistryAddress(String address) {
44+
String previous = CAPTURED_REGISTRY_ADDRESS.get();
4345
CAPTURED_REGISTRY_ADDRESS.set(address);
46+
return previous;
4447
}
4548

46-
public static void clearCapturedRegistryAddress() {
47-
CAPTURED_REGISTRY_ADDRESS.remove();
49+
public static void restoreCapturedRegistryAddress(@Nullable String previous) {
50+
if (previous == null) {
51+
CAPTURED_REGISTRY_ADDRESS.remove();
52+
} else {
53+
CAPTURED_REGISTRY_ADDRESS.set(previous);
54+
}
4855
}
4956

5057
@Nullable
@@ -70,17 +77,6 @@ public static String extractRegistryAddress(RpcInvocation invocation) {
7077
return null;
7178
}
7279

73-
/**
74-
* Resolves {@code protocol://host:port} from a registry-backed directory (for example {@code
75-
* RegistryDirectory}), using {@code getRegistry()} when present and otherwise the {@code
76-
* registry} field. Called once per consumer refer when {@link RegistryCapturingClusterWrapper}
77-
* wraps the cluster invoker.
78-
*/
79-
@Nullable
80-
public static String tryExtractRegistryAddressFromDirectory(Directory<?> directory) {
81-
return extractRegistryAddressFromDirectory(directory);
82-
}
83-
8480
public static String buildServiceTarget(URL url) {
8581
String interfaceName = url.getServiceInterface();
8682
if (interfaceName == null || interfaceName.isEmpty()) {
@@ -125,8 +121,14 @@ private static Directory<?> getDirectory(Invoker<?> invoker) {
125121
}
126122
}
127123

124+
/**
125+
* Resolves {@code protocol://host:port} from a registry-backed directory (for example {@code
126+
* RegistryDirectory}), using {@code getRegistry()} when present and otherwise the {@code
127+
* registry} field. Called once per consumer refer when {@link RegistryCapturingClusterWrapper}
128+
* wraps the cluster invoker.
129+
*/
128130
@Nullable
129-
private static String extractRegistryAddressFromDirectory(Directory<?> directory) {
131+
public static String tryExtractRegistryAddressFromDirectory(Directory<?> directory) {
130132
MethodHandle getRegistry =
131133
findAccessor(directory.getClass(), "getRegistry", "registry", REGISTRY_ACCESSOR_CACHE);
132134
if (getRegistry == null) {
@@ -183,7 +185,7 @@ private static MethodHandle resolveMethod(Class<?> clazz, String name) {
183185
Method m = clazz.getMethod(name);
184186
m.setAccessible(true);
185187
return LOOKUP.unreflect(m);
186-
} catch (NoSuchMethodException | IllegalAccessException e) {
188+
} catch (NoSuchMethodException | IllegalAccessException ignored) {
187189
// ignore
188190
}
189191
return null;
@@ -196,7 +198,7 @@ private static MethodHandle resolveField(Class<?> clazz, String name) {
196198
Field f = c.getDeclaredField(name);
197199
f.setAccessible(true);
198200
return LOOKUP.unreflectGetter(f);
199-
} catch (NoSuchFieldException | IllegalAccessException e) {
201+
} catch (NoSuchFieldException | IllegalAccessException ignored) {
200202
// ignore
201203
}
202204
}

‎instrumentation/apache-dubbo-2.7/library-autoconfigure/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/internal/RegistryCapturingInvoker.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ public void destroy() {
4949

5050
@Override
5151
public Result invoke(Invocation invocation) {
52-
DubboRegistryUtil.pushCapturedRegistryAddress(registryAddress);
52+
String previous = DubboRegistryUtil.pushCapturedRegistryAddress(registryAddress);
5353
try {
5454
return delegate.invoke(invocation);
5555
} finally {
56-
DubboRegistryUtil.clearCapturedRegistryAddress();
56+
DubboRegistryUtil.restoreCapturedRegistryAddress(previous);
5757
}
5858
}
5959
}

‎instrumentation/apache-dubbo-2.7/library-autoconfigure/src/test/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/internal/DubboRegistryUtilTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class DubboRegistryUtilTest {
3030

3131
@AfterEach
3232
void tearDown() {
33-
DubboRegistryUtil.clearCapturedRegistryAddress();
33+
DubboRegistryUtil.restoreCapturedRegistryAddress(null);
3434
}
3535

3636
@ParameterizedTest
@@ -76,7 +76,7 @@ void extractRegistryAddressFieldFallback() throws Exception {
7676
.isEqualTo("zookeeper://10.0.0.1:2181");
7777
}
7878

79-
@SuppressWarnings("all")
79+
@SuppressWarnings("UnusedVariable")
8080
abstract static class MockableRegistryDirectory implements Directory<Object> {
8181
Object registry;
8282
}

‎instrumentation/apache-dubbo-2.7/testing/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/AbstractDubboRegistryTest.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,17 +183,18 @@ void testRegistryModeServerAddress() throws ReflectiveOperationException {
183183
equalTo(SERVER_PORT, null),
184184
satisfies(
185185
NETWORK_PEER_ADDRESS,
186-
k -> assertLatestDeps(k, a -> a.isInstanceOf(String.class))),
186+
val ->
187+
assertLatestDeps(val, v -> v.isInstanceOf(String.class))),
187188
satisfies(
188189
NETWORK_PEER_PORT,
189-
k -> assertLatestDeps(k, a -> a.isInstanceOf(Long.class))),
190+
val -> assertLatestDeps(val, v -> v.isInstanceOf(Long.class))),
190191
satisfies(NETWORK_TYPE, AbstractDubboTest::assertNetworkType)),
191192
span ->
192193
span.hasName(
193194
"io.opentelemetry.instrumentation.apachedubbo.v2_7.api.HelloService/hello")
194195
.hasKind(SpanKind.SERVER)
195196
.hasParent(trace.getSpan(1))
196-
.hasAttributesSatisfying(
197+
.hasAttributesSatisfyingExactly(
197198
equalTo(RPC_SYSTEM, emitOldRpcSemconv() ? "apache_dubbo" : null),
198199
equalTo(RPC_SYSTEM_NAME, emitStableRpcSemconv() ? "dubbo" : null),
199200
equalTo(
@@ -206,7 +207,9 @@ void testRegistryModeServerAddress() throws ReflectiveOperationException {
206207
emitStableRpcSemconv()
207208
? "io.opentelemetry.instrumentation.apachedubbo.v2_7.api.HelloService/hello"
208209
: "hello"),
209-
satisfies(NETWORK_PEER_ADDRESS, k -> k.isInstanceOf(String.class)),
210-
satisfies(NETWORK_PEER_PORT, k -> k.isInstanceOf(Long.class)))));
210+
satisfies(
211+
NETWORK_PEER_ADDRESS, val -> val.isInstanceOf(String.class)),
212+
satisfies(
213+
NETWORK_PEER_PORT, val -> val.isInstanceOf(Long.class)))));
211214
}
212215
}

0 commit comments

Comments
 (0)