-
Notifications
You must be signed in to change notification settings - Fork 1k
Description
Produced from working with copilot:
Javadoc Review and Suggestions
Goal: Improve consistency, clarity, and reduce verbosity while maintaining essential information
Overview
This review covers javadoc for all public methods and classes across 21 HTTP library instrumentation modules. The suggestions aim to unify similar patterns and reduce verbosity by 15-30% while preserving all important information.
Modules Reviewed
- instrumentation/apache-httpclient/apache-httpclient-4.3/library - Apache HttpClient 4.3
- instrumentation/apache-httpclient/apache-httpclient-5.2/library - Apache HttpClient 5.2
- instrumentation/armeria/armeria-1.3/library - Armeria 1.3
- instrumentation/helidon/helidon-4.0/library - Helidon 4.0
- instrumentation/java-http-client/library - Java HTTP Client
- instrumentation/javalin/javalin-5.0/library - Javalin 5.0
- instrumentation/jetty/jetty-httpclient/jetty-httpclient-9.2/library - Jetty HttpClient 9.2
- instrumentation/jetty/jetty-httpclient/jetty-httpclient-12.0/library - Jetty HttpClient 12.0
- instrumentation/ktor/ktor-1.0/library - Ktor 1.0
- instrumentation/ktor/ktor-2.0/library - Ktor 2.0
- instrumentation/ktor/ktor-3.0/library - Ktor 3.0
- instrumentation/netty/netty-4.1/library - Netty 4.1
- instrumentation/okhttp/okhttp-3.0/library - OkHttp 3.0
- instrumentation/ratpack/ratpack-1.7/library - Ratpack 1.7
- instrumentation/restlet/restlet-1.1/library - Restlet 1.1
- instrumentation/restlet/restlet-2.0/library - Restlet 2.0
- instrumentation/servlet/servlet-3.0/library - Servlet 3.0
- instrumentation/spring/spring-web/spring-web-3.1/library - Spring Web 3.1
- instrumentation/spring/spring-webflux/spring-webflux-5.3/library - Spring WebFlux 5.3
- instrumentation/spring/spring-webmvc/spring-webmvc-5.3/library - Spring WebMVC 5.3
- instrumentation/spring/spring-webmvc/spring-webmvc-6.0/library - Spring WebMVC 6.0
Common Patterns
These modules follow consistent patterns with the following public API classes:
- [Library]Telemetry - Main entrypoint class with factory methods
- [Library]TelemetryBuilder - Builder class for configuring instrumentation
- Common methods:
create(OpenTelemetry)- Factory method returning configured telemetry instancebuilder(OpenTelemetry)- Factory method returning builderaddAttributesExtractor(AttributesExtractor)- Configure custom attribute extractionsetCapturedRequestHeaders(Collection<String>)- Configure request headers to capturesetCapturedResponseHeaders(Collection<String>)- Configure response headers to capturesetKnownMethods(Collection<String>)- Configure recognized HTTP methods- Various client/server/filter creation methods specific to each library
Note: This review excludes classes in .internal packages.
Class-Level Javadoc
*Telemetry Classes
Examples: ApacheHttpClientTelemetry, JettyClientTelemetry, ArmeriaClientTelemetry, etc.
Current pattern:
/** Entrypoint for instrumenting Apache HTTP Client. */
/** Entrypoint for instrumenting Jetty client. */
/** Entrypoint for instrumenting Armeria clients. */Suggested:
/** Entrypoint for instrumenting [Library Name] [clients/servers]. */Status: ✅ Current is good. Consistent, concise, clear. No changes needed.
*TelemetryBuilder Classes
Current pattern:
/** A builder for {@link ApacheHttpClientTelemetry}. */
/** A builder of {@link JettyClientTelemetry}. */Suggested (unified):
/** Builder for {@link [TelemetryClass]}. */Change: Use "for" consistently instead of mixing "for" and "of". Remove article "A".
Factory Methods
create(OpenTelemetry)
Current pattern:
/** Returns a new {@link ApacheHttpClientTelemetry} configured with the given {@link OpenTelemetry}. */Suggested:
/** Returns a new instance configured with the given {@link OpenTelemetry} instance. */Rationale: Removes redundancy (class name is obvious from context), maintains clarity.
builder(OpenTelemetry)
Current patterns:
/** Returns a new {@link ApacheHttpClientTelemetryBuilder} configured with the given {@link OpenTelemetry}. */
// Some are missing "Returns a new"
public static ArmeriaClientTelemetryBuilder builder(OpenTelemetry openTelemetry)Suggested:
/** Returns a builder configured with the given {@link OpenTelemetry} instance. */Rationale: More concise, removes redundant class name and "a new" (obvious from return type).
Builder Configuration Methods
addAttributesExtractor(AttributesExtractor)
Current patterns:
/** Adds an additional {@link AttributesExtractor} to invoke to set attributes to instrumented items. The {@link AttributesExtractor} will be executed after all default extractors. */
/** Adds an extra {@link AttributesExtractor} to invoke to set attributes to instrumented items. The {@link AttributesExtractor} will be executed after all default extractors. */Suggested:
/**
* Adds an {@link AttributesExtractor} to extract attributes from requests and responses.
* Executed after all default extractors.
*/Changes:
- Remove "additional"/"extra" (obvious from "adds")
- Simplify "to invoke to set attributes to instrumented items" → "to extract attributes from requests and responses" (clearer about what it actually does)
- Split into two sentences for better readability
setCapturedRequestHeaders(Collection<String>)
Current:
/**
* Configures the HTTP request headers that will be captured as span attributes.
*
* @param requestHeaders A list of HTTP header names.
*/Suggested:
/**
* Configures HTTP request headers to capture as span attributes.
*
* @param requestHeaders HTTP header names to capture.
*/Changes:
- Remove "the" and "that will be"
- Tighten @param description: remove "A list of"
setCapturedResponseHeaders(Collection<String>)
Current:
/**
* Configures the HTTP response headers that will be captured as span attributes.
*
* @param responseHeaders A list of HTTP header names.
*/Suggested:
/**
* Configures HTTP response headers to capture as span attributes.
*
* @param responseHeaders HTTP header names to capture.
*/setKnownMethods(Collection<String>)
Current:
/**
* Configures the instrumentation to recognize an alternative set of HTTP request methods.
*
* <p>By default, this instrumentation defines "known" methods as the ones listed in <a
* href="https://www.rfc-editor.org/rfc/rfc9110.html#name-methods">RFC9110</a> and the PATCH
* method defined in <a href="https://www.rfc-editor.org/rfc/rfc5789.html">RFC5789</a>.
*
* <p>Note: calling this method <b>overrides</b> the default known method sets completely; it does
* not supplement it.
*
* @param knownMethods A set of recognized HTTP request methods.
* @see HttpClientAttributesExtractorBuilder#setKnownMethods(Collection)
*/Suggested:
/**
* Configures recognized HTTP request methods.
*
* <p>By default, recognizes methods from <a
* href="https://www.rfc-editor.org/rfc/rfc9110.html#name-methods">RFC9110</a> and PATCH
* from <a href="https://www.rfc-editor.org/rfc/rfc5789.html">RFC5789</a>.
*
* <p><b>Note:</b> This <b>overrides</b> defaults completely; it does not supplement them.
*
* @param knownMethods HTTP request methods to recognize.
* @see HttpClientAttributesExtractorBuilder#setKnownMethods(Collection)
*/Changes:
- Tighten first sentence
- Simplify default description
- Improve note formatting and conciseness
- Remove article from @param
setSpanNameExtractorCustomizer(UnaryOperator<SpanNameExtractor>)
Current:
/** Sets a customizer that receives the default {@link SpanNameExtractor} and returns a customized one. */Suggested:
/** Customizes the {@link SpanNameExtractor} by transforming the default instance. */Rationale: Simpler, removes redundancy ("receives... and returns" → "transforming").
setSpanStatusExtractorCustomizer(UnaryOperator<SpanStatusExtractor>)
Current:
/** Sets a customizer that receives the default {@link SpanStatusExtractor} and returns a customized one. */Suggested:
/** Customizes the {@link SpanStatusExtractor} by transforming the default instance. */build()
Current patterns:
/** Returns a new {@link ApacheHttpClientTelemetry} configured with this {@link ApacheHttpClientTelemetryBuilder}. */
/** Returns a new {@link JettyClientTelemetry} with the settings of this {@link JettyClientTelemetryBuilder}. */Suggested:
/** Returns a new instance with the configured settings. */Rationale: Extremely concise, obvious from context. Return type is clear from method signature.
Client/Server Creation Methods
HTTP Client Creation Methods
createHttpClient()
Used in: ApacheHttpClient, JettyClient, etc.
Current:
/** Returns a new {@link CloseableHttpClient} with tracing configured. */Suggested:
/** Returns an instrumented HTTP client. */Rationale: Use generic description since return type varies across implementations.
createHttpClientBuilder()
Used in: ApacheHttpClient
Current:
/** Returns a new {@link HttpClientBuilder} to create a client with tracing configured. */Suggested:
/** Returns a builder for creating an instrumented HTTP client. */Change: Cleaner phrasing, removes "to create a client".
createCallFactory(OkHttpClient)
Used in: OkHttp
Current:
/**
* Construct a new OpenTelemetry tracing-enabled {@link okhttp3.Call.Factory} using the provided
* {@link OkHttpClient} instance.
*
* <p>Using this method will result in proper propagation and span parenting, for both {@linkplain
* Call#execute() synchronous} and {@linkplain Call#enqueue(Callback) asynchronous} usages.
*
* @param baseClient An instance of OkHttpClient configured as desired.
* @return a {@link Call.Factory} for creating new {@link Call} instances.
*/Suggested:
/**
* Returns an instrumented {@link okhttp3.Call.Factory} wrapping the provided client.
*
* <p>Supports both {@linkplain Call#execute() synchronous} and
* {@linkplain Call#enqueue(Callback) asynchronous} calls with proper context propagation.
*
* @param baseClient the OkHttpClient to wrap
* @return an instrumented Call.Factory
*/Changes:
createHttpClient(HttpClient)
Used in: JavaHttpClient
Current:
/**
* Construct a new OpenTelemetry tracing-enabled {@link HttpClient} using the provided {@link
* HttpClient} instance.
*
* @param client An instance of HttpClient configured as desired.
* @return a tracing-enabled {@link HttpClient}.
*/Suggested:
/**
* Returns an instrumented {@link HttpClient} wrapping the provided client.
*
* @param client the HttpClient to wrap
* @return an instrumented HttpClient
*/instrument(HttpClient)
Used in: RatpackClient
Current:
/** Returns instrumented instance of {@link HttpClient} with OpenTelemetry. */Suggested:
/** Returns an instrumented wrapper for the given {@link HttpClient}. */Rationale: Clearer, avoids awkward "instrumented instance with" phrasing.
Server/Filter Creation Methods
createFilter()
Used in: JavaHttpServer, Servlet, Restlet, SpringWebMvc
Current patterns:
/** Returns a new {@link Filter} for telemetry usage */
/** Returns a new {@link Filter} for producing telemetry. */
/** Returns a new {@link Filter} that generates telemetry for received HTTP requests. */Suggested:
/** Returns a {@link Filter} that instruments HTTP requests. */Changes:
- Unified across all implementations
- Remove "new" (obvious from "returns")
- Remove "received" (obvious for server filters)
createServletFilter()
Used in: SpringWebMvc
Current:
/** Returns a new {@link Filter} that generates telemetry for received HTTP requests. */Suggested:
/** Returns a {@link Filter} that instruments HTTP requests. */createWebFilter() / createWebFilterAndRegisterReactorHook()
Used in: SpringWebflux
Current:
/**
* Returns an OpenTelemetry telemetry producing {@link WebFilter} that can be registered with
* Spring Webflux to instrument HTTP server requests.
*
* @return OpenTelemetry telemetry producing {@link WebFilter}
*/Suggested:
/** Returns a {@link WebFilter} that instruments HTTP server requests. */For the reactor hook variant:
/**
* Returns a {@link WebFilter} that instruments HTTP server requests.
* Also registers the Reactor context propagation hook for reactive pipelines.
*/Changes:
- Remove verbose "OpenTelemetry telemetry producing" redundancy
- Remove unnecessary registration details (obvious from method name)
- Remove @return tag for simple single-line cases
addFilter(List<ExchangeFilterFunction>)
Used in: SpringWebfluxClient
Current:
/**
* Adds the OpenTelemetry telemetry producing {@link ExchangeFilterFunction} to the provided list
* of filter functions.
*
* @param exchangeFilterFunctions existing filter functions
*/Suggested:
/**
* Adds an instrumented {@link ExchangeFilterFunction} to the provided list.
*
* @param exchangeFilterFunctions existing filter functions
*/newDecorator()
Used in: Armeria
Current:
/**
* Returns a new {@link HttpClient} decorator for use with methods like {@link
* com.linecorp.armeria.client.ClientBuilder#decorator(Function)}.
*/Suggested:
/** Returns a decorator for instrumenting Armeria clients. */Rationale: Much more concise, removes obvious usage details.
createInterceptor()
Used in: SpringWeb
Current:
/**
* Returns a new {@link ClientHttpRequestInterceptor} that can be used with {@link
* RestTemplate#getInterceptors()}. For example:
*
* <pre>{@code
* restTemplate.getInterceptors().add(SpringWebTelemetry.create(openTelemetry).createInterceptor());
* }</pre>
*/Suggested:
/**
* Returns an interceptor for instrumenting {@link RestTemplate} requests.
*
* <pre>{@code
* restTemplate.getInterceptors().add(SpringWebTelemetry.create(openTelemetry).createInterceptor());
* }</pre>
*/Changes:
- Simplify description
- Keep example code (helpful)
Handler Creation Methods (Netty)
createRequestHandler() / createResponseHandler() / createCombinedHandler()
Current:
/**
* Returns a new {@link ChannelOutboundHandlerAdapter} that generates telemetry for outgoing HTTP
* requests. Must be paired with {@link #createResponseHandler()}.
*/Suggested:
/**
* Returns a handler that instruments outgoing HTTP requests.
* Must be paired with {@link #createResponseHandler()}.
*/Rationale: Use "handler" generically instead of full type name for brevity.
Special Methods
setChannelContext(Channel, Context)
Used in: NettyClientTelemetry
Current:
/**
* Propagate the {@link Context} to the {@link Channel}. This MUST be called before each HTTP
* request executed on a {@link Channel}.
*/Suggested:
/**
* Propagates the {@link Context} to the {@link Channel}.
* Must be called before each HTTP request on the channel.
*/Changes:
- Remove unnecessary capitalization of "MUST"
- Simplify "executed on a" to "on the"
- Use present tense for consistency
configure(HttpContext)
Used in: JavaHttpServer
Current:
/** Configures the {@link HttpContext} with OpenTelemetry. */Suggested:
/** Configures the {@link HttpContext} to produce telemetry. */Rationale: More specific about what "configures with OpenTelemetry" means.
configureRegistry(RegistrySpec)
Used in: RatpackServer
Current:
/** Configures the {@link RegistrySpec} with OpenTelemetry. */Suggested:
/** Configures the {@link RegistrySpec} to produce telemetry. */Summary of Key Changes
1. Remove redundant class names
Factory methods don't need to repeat the class name (obvious from context and return type).
2. Use "for" consistently
In builder class-level docs, use "for" not "of" (Builder for X not A builder of X).
3. Simplify verbs
- "to set" not "to invoke to set"
- "capture" not "will be captured"
- "Returns" not "Construct a new"
4. Tighten @param descriptions
Remove articles and unnecessary words: "A list of HTTP header names" → "HTTP header names to capture"
5. Use generic type names
When return types vary across implementations, use generic descriptions ("HTTP client" instead of specific class name).
6. Remove "new"
"Returns a new" → "Returns a" (obvious from "Returns").
7. Simplify customizer methods
Use "transforms/customizes" instead of "receives... and returns".
8. Unify filter/handler descriptions
Use consistent pattern across all server instrumentations.
9. Remove verbose redundancy
"OpenTelemetry telemetry producing" → "instruments"
10. Use "instrumented" consistently
For wrapped clients and filters that collect telemetry (not just tracing, but also metrics and other signals).
Impact
These changes reduce verbosity by 15-30% while maintaining all essential information and improving clarity. The unified patterns make the API more consistent and easier to understand across different instrumentation modules.