apply

Author: trask

.github/agents/knowledge/javaagent-singletons-patterns.md

@@ -3,26 +3,32 @@
 ## Quick Reference
 
 - Use when: reviewing `*Singletons` holder classes and their callers
-- Review focus: private fields, accessor naming, eager initialization, static-import call sites
+- Review focus: field/accessor naming, eager initialization, static-import call sites
 
 Javaagent modules keep shared `Instrumenter` instances and related collaborators in a dedicated
 `Singletons` holder class such as `MyLibrarySingletons`.
 
 ## Rules
 
-- Keep exported singleton-holder fields `private`. Do not expose package-private or public
-  `static` fields from `*Singletons` classes.
 - Initialize shared collaborators at class-load time, either with `static final` field
   initializers or in a `static {}` block.
 - Use `GlobalOpenTelemetry.get()` to obtain the `OpenTelemetry` instance.
 - The instrumentation name string should match the Gradle module path:
   `"io.opentelemetry.<module-name>"`.
-- For exported collaborators, use lower camel case field names and give the accessor method the
-  exact same name as the field. Do not prefix these accessors with `get`.
+- For exported collaborators, keep the field `private`, use a lower camel case field name, and
+  give the accessor method the exact same name as the field. Do not prefix these accessors with
+  `get`.
   - `instrumenter` -> `instrumenter()`
   - `helper` -> `helper()`
   - `setter` -> `setter()`
-- Callers should static import singleton accessor methods and invoke them unqualified.
+- For exported uppercase constant-like fields that represent stable identifiers, immutable
+  descriptors, or semantic keys/handles such as `VirtualField` and `ContextKey`, it is acceptable
+  to expose them as `public static final` fields with no accessor.
+  - `CONTEXT` stays `CONTEXT`
+  - `REQUEST_INFO` stays `REQUEST_INFO`
+  - `RESPONSE_STATUS` stays `RESPONSE_STATUS`
+- Callers should static import the exported singleton member and use it unqualified:
+  accessor methods for lower camel collaborators, fields for uppercase constant-like members.
 - Keep verb-named helper methods as verbs when they perform work instead of returning a stored
   field. This naming rule applies to field accessors.
 
@@ -48,6 +54,18 @@ public final class MyLibrarySingletons {
 }
 ```
 
+Uppercase field exception:
+
+```java
+public final class MyLibrarySingletons {
+
+  public static final VirtualField<Request, Context> REQUEST_CONTEXT =
+      VirtualField.find(Request.class, Context.class);
+
+  private MyLibrarySingletons() {}
+}
+```
+
 Caller:
 
 ```java
@@ -63,12 +81,26 @@ class MyInstrumentation implements TypeInstrumentation {
 }
 ```
 
+Caller for uppercase field:
+
+```java
+import static io.opentelemetry.javaagent.instrumentation.example.MyLibrarySingletons.REQUEST_CONTEXT;
+
+class MyInstrumentation implements TypeInstrumentation {
+  void doSomething(Request request, Context context) {
+    REQUEST_CONTEXT.set(request, context);
+  }
+}
+```
+
 ## What to Flag in Review
 
-- Exposed singleton-holder fields such as `public static final Instrumenter ...`.
+- Exposed lower camel collaborator fields such as `public static final Instrumenter ...`.
+- Private + accessor wrappers around uppercase semantic keys/handles when a direct `public static
+  final` field would be clearer and matches the naming guidance.
 - Accessor methods named `getInstrumenter()`, `getHelper()`, `getSetter()`, and similar when they
   simply return a backing field.
-- Call sites that qualify singleton field-accessor calls with the holder class instead of static
-  importing the accessor method.
+- Call sites that qualify singleton member usage with the holder class instead of static importing
+  the accessor or field.
 - Mismatches between a field name and its accessor, such as `private static final Helper helper;`
   with `public static Helper getHelper()`.

instrumentation/apache-dubbo-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachedubbo/v2_7/DubboSingletons.java

@@ -12,8 +12,8 @@
 import org.apache.dubbo.rpc.Filter;
 
 public final class DubboSingletons {
-  public static final Filter CLIENT_FILTER;
-  public static final Filter SERVER_FILTER;
+  private static final Filter clientFilter;
+  private static final Filter serverFilter;
 
   static {
     DubboTelemetry telemetry =
@@ -22,8 +22,16 @@ public final class DubboSingletons {
                 ServicePeerAttributesExtractor.create(
                     new DubboClientNetworkAttributesGetter(), GlobalOpenTelemetry.get()))
             .build();
-    CLIENT_FILTER = telemetry.newClientFilter();
-    SERVER_FILTER = telemetry.newServerFilter();
+    clientFilter = telemetry.newClientFilter();
+    serverFilter = telemetry.newServerFilter();
+  }
+
+  public static Filter clientFilter() {
+    return clientFilter;
+  }
+
+  public static Filter serverFilter() {
+    return serverFilter;
   }
 
   private DubboSingletons() {}

instrumentation/apache-dubbo-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachedubbo/v2_7/OpenTelemetryClientFilter.java

@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.instrumentation.apachedubbo.v2_7;
 
+import static io.opentelemetry.javaagent.instrumentation.apachedubbo.v2_7.DubboSingletons.clientFilter;
+
 import org.apache.dubbo.common.extension.Activate;
 import org.apache.dubbo.rpc.Filter;
 import org.apache.dubbo.rpc.Invocation;
@@ -19,7 +21,7 @@ public final class OpenTelemetryClientFilter implements Filter {
   private final Filter delegate;
 
   public OpenTelemetryClientFilter() {
-    delegate = DubboSingletons.CLIENT_FILTER;
+    delegate = clientFilter();
   }
 
   @Override

instrumentation/apache-dubbo-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachedubbo/v2_7/OpenTelemetryServerFilter.java

@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.instrumentation.apachedubbo.v2_7;
 
+import static io.opentelemetry.javaagent.instrumentation.apachedubbo.v2_7.DubboSingletons.serverFilter;
+
 import org.apache.dubbo.common.extension.Activate;
 import org.apache.dubbo.rpc.Filter;
 import org.apache.dubbo.rpc.Invocation;
@@ -19,7 +21,7 @@ public final class OpenTelemetryServerFilter implements Filter {
   private final Filter delegate;
 
   public OpenTelemetryServerFilter() {
-    delegate = DubboSingletons.SERVER_FILTER;
+    delegate = serverFilter();
   }
 
   @Override

instrumentation/armeria/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaServerBuilderInstrumentation.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
 
+import static io.opentelemetry.javaagent.instrumentation.armeria.v1_3.ArmeriaSingletons.serverDecorator;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -35,7 +36,7 @@ public static class BuildAdvice {
 
     @Advice.OnMethodEnter
     public static void onEnter(@Advice.This ServerBuilder builder) {
-      builder.decorator(ArmeriaSingletons.SERVER_DECORATOR);
+      builder.decorator(serverDecorator());
     }
   }
 }

instrumentation/armeria/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaSingletons.java

@@ -20,9 +20,9 @@
 // Holds singleton references to decorators to match against during suppression.
 // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/903
 public final class ArmeriaSingletons {
-  public static final Function<HttpClient, HttpClient> CLIENT_DECORATOR;
+  private static final Function<HttpClient, HttpClient> clientDecorator;
 
-  public static final Function<HttpService, HttpService> SERVER_DECORATOR;
+  private static final Function<HttpService, HttpService> serverDecorator;
 
   static {
     CommonConfig config = AgentCommonConfig.get();
@@ -41,10 +41,18 @@ public final class ArmeriaSingletons {
         .configure(config);
     ArmeriaServerTelemetry serverTelemetry = serverBuilder.build();
 
-    CLIENT_DECORATOR = clientTelemetry.createDecorator();
+    clientDecorator = clientTelemetry.createDecorator();
     Function<HttpService, HttpService> libraryDecorator =
         serverTelemetry.createDecorator().compose(ResponseCustomizingDecorator::new);
-    SERVER_DECORATOR = service -> new ServerDecorator(service, libraryDecorator.apply(service));
+    serverDecorator = service -> new ServerDecorator(service, libraryDecorator.apply(service));
+  }
+
+  public static Function<HttpClient, HttpClient> clientDecorator() {
+    return clientDecorator;
+  }
+
+  public static Function<HttpService, HttpService> serverDecorator() {
+    return serverDecorator;
   }
 
   private ArmeriaSingletons() {}

instrumentation/armeria/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaWebClientBuilderInstrumentation.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
 
+import static io.opentelemetry.javaagent.instrumentation.armeria.v1_3.ArmeriaSingletons.clientDecorator;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -35,7 +36,7 @@ public static class BuildAdvice {
 
     @Advice.OnMethodEnter
     public static void build(@Advice.This WebClientBuilder builder) {
-      builder.decorator(ArmeriaSingletons.CLIENT_DECORATOR);
+      builder.decorator(clientDecorator());
     }
   }
 }

instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java

@@ -58,7 +58,7 @@ public String getDbQueryText(ElasticsearchRestRequest request) {
         && epDefinition.isSearchEndpoint()
         && httpEntity != null
         && httpEntity.isRepeatable()) {
-      // Retrieve HTTP body for search-type Elasticsearch requests when CAPTURE_SEARCH_QUERY is
+      // Retrieve HTTP body for search-type Elasticsearch requests when captureSearchQuery is
       // enabled.
       try (BufferedReader reader =
           new BufferedReader(new InputStreamReader(httpEntity.getContent(), UTF_8))) {

instrumentation/grpc-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_6/GrpcClientBuilderBuildInstrumentation.java

@@ -8,6 +8,7 @@
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass;
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static io.opentelemetry.javaagent.instrumentation.grpc.v1_6.GrpcSingletons.MANAGED_CHANNEL_BUILDER_INSTRUMENTED;
+import static io.opentelemetry.javaagent.instrumentation.grpc.v1_6.GrpcSingletons.clientInterceptor;
 import static net.bytebuddy.matcher.ElementMatchers.declaresField;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
@@ -45,7 +46,7 @@ public static void addInterceptor(
         @Advice.This ManagedChannelBuilder<?> builder,
         @Advice.FieldValue("interceptors") List<ClientInterceptor> interceptors) {
       if (!Boolean.TRUE.equals(MANAGED_CHANNEL_BUILDER_INSTRUMENTED.get(builder))) {
-        interceptors.add(0, GrpcSingletons.CLIENT_INTERCEPTOR);
+        interceptors.add(0, clientInterceptor());
         MANAGED_CHANNEL_BUILDER_INSTRUMENTED.set(builder, true);
       }
     }

instrumentation/grpc-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_6/GrpcContextInstrumentation.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.grpc.v1_6;
 
+import static io.opentelemetry.javaagent.instrumentation.grpc.v1_6.GrpcSingletons.storage;
 import static net.bytebuddy.matcher.ElementMatchers.isStatic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.returns;
@@ -37,7 +38,7 @@ public static class ContextBridgeAdvice {
     @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class, suppress = Throwable.class)
     @Nullable
     public static Context.Storage onEnter() {
-      return GrpcSingletons.getStorage();
+      return storage();
     }
 
     @AssignReturned.ToReturned