code review comments

open-telemetry · Apr 19, 2022 · 21ee219 · 21ee219
1 parent 2e36472
commit 21ee219
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 15 deletions.
diff --git a/...n-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressor.java b/...n-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressor.java
@@ -78,24 +78,24 @@ public boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
 
   final class BySpanKey implements SpanSuppressor {
 
-    private final Set<SpanKey> outgoingSpanKeys;
+    private final Set<SpanKey> spanKeys;
 
-    BySpanKey(Set<SpanKey> outgoingSpanKeys) {
-      this.outgoingSpanKeys = outgoingSpanKeys;
+    BySpanKey(Set<SpanKey> spanKeys) {
+      this.spanKeys = spanKeys;
     }
 
     @Override
     public Context storeInContext(Context context, SpanKind spanKind, Span span) {
-      for (SpanKey outgoingSpanKey : outgoingSpanKeys) {
-        context = outgoingSpanKey.storeInContext(context, span);
+      for (SpanKey spanKey : spanKeys) {
+        context = spanKey.storeInContext(context, span);
       }
       return context;
     }
 
     @Override
     public boolean shouldSuppress(Context parentContext, SpanKind spanKind) {
-      for (SpanKey outgoingSpanKey : outgoingSpanKeys) {
-        if (outgoingSpanKey.fromContextOrNull(parentContext) == null) {
+      for (SpanKey spanKey : spanKeys) {
+        if (spanKey.fromContextOrNull(parentContext) == null) {
           return false;
         }
       }

diff --git a/...api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java b/...api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java
@@ -174,7 +174,8 @@ void server() {
             .newServerInstrumenter(new MapGetter());
 
     Context context = instrumenter.start(Context.root(), REQUEST);
-    assertThat(Span.fromContext(context).getSpanContext().isValid()).isTrue();
+    SpanContext spanContext = Span.fromContext(context).getSpanContext();
+    assertThat(spanContext.isValid()).isTrue();
 
     instrumenter.end(context, REQUEST, RESPONSE, null);
 
@@ -187,8 +188,8 @@ void server() {
                         span.hasName("span")
                             .hasKind(SpanKind.SERVER)
                             .hasInstrumentationScopeInfo(InstrumentationScopeInfo.create("test"))
-                            .hasTraceId(Span.fromContext(context).getSpanContext().getTraceId())
-                            .hasSpanId(Span.fromContext(context).getSpanContext().getSpanId())
+                            .hasTraceId(spanContext.getTraceId())
+                            .hasSpanId(spanContext.getSpanId())
                             .hasParentSpanId(SpanId.getInvalid())
                             .hasStatus(StatusData.unset())
                             .hasLinks(expectedSpanLink())

diff --git a/...n/client/MockHttpAttributesExtractor.java → ...HttpClientSpanKeyAttributesExtractor.java b/...n/client/MockHttpAttributesExtractor.java → ...HttpClientSpanKeyAttributesExtractor.java
@@ -19,7 +19,7 @@
  * Attributes extractor that pretends it's a {@link HttpClientAttributesExtractor} so that error
  * only CONNECT spans can be suppressed by higher level HTTP clients based on netty.
  */
-enum MockHttpAttributesExtractor
+enum HttpClientSpanKeyAttributesExtractor
     implements AttributesExtractor<NettyConnectionRequest, Channel>, SpanKeyProvider {
   INSTANCE;
 

diff --git a/...lemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java b/...lemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java
@@ -61,13 +61,13 @@ public NettyConnectionInstrumenter createConnectionInstrumenter() {
             .setTimeExtractor(new NettyConnectionTimeExtractor());
     if (!connectionTelemetryEnabled) {
       // when the connection telemetry is not enabled, netty creates CONNECT spans whenever a
-      // connection error occurs - because there is no HTTP span in that scenario, in raw netty
+      // connection error occurs - because there is no HTTP span in that scenario, if raw netty
       // connection occurs before an HTTP message is even formed
       // we don't want that span when a higher-level HTTP library (like reactor-netty or async http
       // client) is used, the connection phase is a part of the HTTP span for these
       // for that to happen, the CONNECT span will "pretend" to be a full HTTP span when connection
       // telemetry is off
-      instrumenterBuilder.addAttributesExtractor(MockHttpAttributesExtractor.INSTANCE);
+      instrumenterBuilder.addAttributesExtractor(HttpClientSpanKeyAttributesExtractor.INSTANCE);
     }
 
     Instrumenter<NettyConnectionRequest, Channel> instrumenter =

diff --git a/...lemetry-api/opentelemetry-api-1.0/testing/src/main/java/AgentSpanTestingInstrumenter.java b/...lemetry-api/opentelemetry-api-1.0/testing/src/main/java/AgentSpanTestingInstrumenter.java
@@ -30,7 +30,7 @@ public final class AgentSpanTestingInstrumenter {
   private static final Instrumenter<Request, Void> INSTRUMENTER_HTTP_SERVER =
       Instrumenter.<Request, Void>builder(
               GlobalOpenTelemetry.get(), "test", request -> request.name)
-          .addAttributesExtractor(HttpServerAttributesExtractor.INSTANCE)
+          .addAttributesExtractor(HttpServerSpanKeyAttributesExtractor.INSTANCE)
           .addContextCustomizer(
               (context, request, startAttributes) -> context.with(REQUEST_CONTEXT_KEY, request))
           .newInstrumenter(request -> request.kind);
@@ -94,7 +94,7 @@ private static SpanKey[] getSpanKeys() {
   }
 
   // simulate a real HTTP server implementation
-  private enum HttpServerAttributesExtractor
+  private enum HttpServerSpanKeyAttributesExtractor
       implements AttributesExtractor<Request, Void>, SpanKeyProvider {
     INSTANCE;