Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert AttributesExtractor to interface #4363

Merged
merged 1 commit into from
Oct 14, 2021
Merged

Convert AttributesExtractor to interface #4363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...o/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

/** Extractor of {@link io.opentelemetry.api.common.Attributes} for a traced method. */
public final class MethodSpanAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {

private final MethodExtractor<REQUEST> methodExtractor;
private final MethodArgumentsExtractor<REQUEST> methodArgumentsExtractor;
Expand Down Expand Up @@ -46,7 +46,7 @@ public static <REQUEST, RESPONSE> MethodSpanAttributesExtractor<REQUEST, RESPONS
}

@Override
protected void onStart(AttributesBuilder attributes, REQUEST request) {
public void onStart(AttributesBuilder attributes, REQUEST request) {
Method method = methodExtractor.extract(request);
AttributeBindings bindings = cache.computeIfAbsent(method, this::bind);
if (!bindings.isEmpty()) {
Expand All @@ -56,7 +56,7 @@ protected void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
11 changes: 5 additions & 6 deletions .../src/main/java/io/opentelemetry/instrumentation/api/instrumenter/AttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@
* @see HttpClientAttributesExtractor
* @see NetServerAttributesExtractor
*/
public abstract class AttributesExtractor<REQUEST, RESPONSE> {
public interface AttributesExtractor<REQUEST, RESPONSE> {
/**
* Extracts attributes from the {@link REQUEST} into the {@link AttributesBuilder} at the
* beginning of a request.
*/
protected abstract void onStart(AttributesBuilder attributes, REQUEST request);
void onStart(AttributesBuilder attributes, REQUEST request);

/**
* Extracts attributes from the {@link REQUEST} and either {@link RESPONSE} or {@code error} into
* the {@link AttributesBuilder} at the end of a request.
*/
protected abstract void onEnd(
void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand All @@ -45,8 +45,7 @@ protected abstract void onEnd(
* Sets the {@code value} with the given {@code key} to the {@link AttributesBuilder} if {@code
* value} is not {@code null}.
*/
protected static <T> void set(
AttributesBuilder attributes, AttributeKey<T> key, @Nullable T value) {
default <T> void set(AttributesBuilder attributes, AttributeKey<T> key, @Nullable T value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since set isn't really part of the contract of AttributesExtractor, we should keep them off the method even as defaults. I'm thinking the use case of "fill attribute if available" is so common that it's fine to add the @Nullable annotation in the API and consider it "correct usage". So easiest is probably to rely on the implementation detail that put can be called directly, and I can work on that in the SDK to formalize it.

Can be another PR but we should do it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkwatson explicitly told me on Slack not to rely on null-checking of the put :) But if that gets changed in SDK/API, I will be happy to remove this method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say either rely on it, or move to an internal class - I guess one of the motivations of the change is to make this interface stable :) I don't think the set method can be part of that.

if (value != null) {
attributes.put(key, value);
}
Expand All @@ -56,7 +55,7 @@ protected static <T> void set(
* Returns an {@link AttributesExtractor} implementation that always extracts the provided
* constant value.
*/
public static <REQUEST, RESPONSE, T> AttributesExtractor<REQUEST, RESPONSE> constant(
static <REQUEST, RESPONSE, T> AttributesExtractor<REQUEST, RESPONSE> constant(
AttributeKey<T> attributeKey, T attributeValue) {
return new ConstantAttributesExtractor<>(attributeKey, attributeValue);
}
Expand Down
6 changes: 3 additions & 3 deletions ...n/java/io/opentelemetry/instrumentation/api/instrumenter/ConstantAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import org.checkerframework.checker.nullness.qual.Nullable;

final class ConstantAttributesExtractor<REQUEST, RESPONSE, T>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {

private final AttributeKey<T> attributeKey;
private final T attributeValue;
Expand All @@ -21,12 +21,12 @@ final class ConstantAttributesExtractor<REQUEST, RESPONSE, T>
}

@Override
protected void onStart(AttributesBuilder attributes, REQUEST request) {
public void onStart(AttributesBuilder attributes, REQUEST request) {
attributes.put(attributeKey, attributeValue);
}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions ...ava/io/opentelemetry/instrumentation/api/instrumenter/PeerServiceAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* comma-separated list of {@code host=name} pairs.
*/
public final class PeerServiceAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {
private static final Map<String, String> JAVAAGENT_PEER_SERVICE_MAPPING =
Config.get().getMap("otel.instrumentation.common.peer-service-mapping");

Expand All @@ -48,10 +48,10 @@ public static <REQUEST, RESPONSE> PeerServiceAttributesExtractor<REQUEST, RESPON
}

@Override
protected void onStart(AttributesBuilder attributes, REQUEST request) {}
public void onStart(AttributesBuilder attributes, REQUEST request) {}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions .../java/io/opentelemetry/instrumentation/api/instrumenter/code/CodeAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
* code attributes</a>.
*/
public abstract class CodeAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, REQUEST request) {
Class<?> cls = codeClass(request);
if (cls != null) {
set(attributes, SemanticAttributes.CODE_NAMESPACE, cls.getName());
Expand All @@ -30,7 +30,7 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions ...main/java/io/opentelemetry/instrumentation/api/instrumenter/db/DbAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
* specification.
*/
public abstract class DbAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {
@Override
protected void onStart(AttributesBuilder attributes, REQUEST request) {
public void onStart(AttributesBuilder attributes, REQUEST request) {
set(attributes, SemanticAttributes.DB_SYSTEM, system(request));
set(attributes, SemanticAttributes.DB_USER, user(request));
set(attributes, SemanticAttributes.DB_NAME, name(request));
Expand All @@ -32,7 +32,7 @@ protected void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
2 changes: 1 addition & 1 deletion ...ain/java/io/opentelemetry/instrumentation/api/instrumenter/db/SqlAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public abstract class SqlAttributesExtractor<REQUEST, RESPONSE>
extends DbAttributesExtractor<REQUEST, RESPONSE> {

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, REQUEST request) {
super.onStart(attributes, request);
AttributeKey<String> dbTable = dbTableAttribute();
if (dbTable != null) {
Expand Down
4 changes: 2 additions & 2 deletions ...io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ protected HttpClientAttributesExtractor() {
}

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, REQUEST request) {
super.onStart(attributes, request);
set(attributes, SemanticAttributes.HTTP_URL, url(request));
}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions ...io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* attributes</a> that are common to client and server instrumentations.
*/
public abstract class HttpCommonAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {

private final CapturedHttpHeaders capturedHttpHeaders;

Expand All @@ -31,7 +31,7 @@ public abstract class HttpCommonAttributesExtractor<REQUEST, RESPONSE>
}

@Override
protected void onStart(AttributesBuilder attributes, REQUEST request) {
public void onStart(AttributesBuilder attributes, REQUEST request) {
set(attributes, SemanticAttributes.HTTP_METHOD, method(request));
set(attributes, SemanticAttributes.HTTP_USER_AGENT, userAgent(request));

Expand All @@ -44,7 +44,7 @@ protected void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
4 changes: 2 additions & 2 deletions ...io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected HttpServerAttributesExtractor() {
}

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, REQUEST request) {
super.onStart(attributes, request);

set(attributes, SemanticAttributes.HTTP_FLAVOR, flavor(request));
Expand All @@ -55,7 +55,7 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions ...pentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
* best compliance with the OpenTelemetry specification.
*/
public abstract class MessagingAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {
public static final String TEMP_DESTINATION_NAME = "(temporary)";

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, REQUEST request) {
set(attributes, SemanticAttributes.MESSAGING_SYSTEM, system(request));
set(attributes, SemanticAttributes.MESSAGING_DESTINATION_KIND, destinationKind(request));
boolean isTemporaryDestination = temporaryDestination(request);
Expand Down Expand Up @@ -53,7 +53,7 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions ...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
* it is more convenient to use {@link InetSocketAddressNetClientAttributesExtractor}.
*/
public abstract class NetClientAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {}
public final void onStart(AttributesBuilder attributes, REQUEST request) {}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions ...a/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
* it is more convenient to use {@link InetSocketAddressNetServerAttributesExtractor}.
*/
public abstract class NetServerAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, REQUEST request) {
set(attributes, SemanticAttributes.NET_TRANSPORT, transport(request));

String peerIp = peerIp(request);
Expand All @@ -38,7 +38,7 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions ...in/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/RpcAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@
* specification.
*/
public abstract class RpcAttributesExtractor<REQUEST, RESPONSE>
extends AttributesExtractor<REQUEST, RESPONSE> {
implements AttributesExtractor<REQUEST, RESPONSE> {

@Override
protected final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, REQUEST request) {
set(attributes, SemanticAttributes.RPC_SYSTEM, system(request));
set(attributes, SemanticAttributes.RPC_SERVICE, service(request));
set(attributes, SemanticAttributes.RPC_METHOD, method(request));
}

@Override
protected final void onEnd(
public final void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
Expand Down
6 changes: 3 additions & 3 deletions .../test/java/io/opentelemetry/instrumentation/api/instrumenter/AttributesExtractorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@
class AttributesExtractorTest {

static class TestAttributesExtractor
extends AttributesExtractor<Map<String, String>, Map<String, String>> {
implements AttributesExtractor<Map<String, String>, Map<String, String>> {

@Override
protected void onStart(AttributesBuilder attributes, Map<String, String> request) {
public void onStart(AttributesBuilder attributes, Map<String, String> request) {
set(attributes, AttributeKey.stringKey("animal"), request.get("animal"));
set(attributes, AttributeKey.stringKey("country"), request.get("country"));
}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
Map<String, String> request,
@Nullable Map<String, String> response,
Expand Down
12 changes: 6 additions & 6 deletions ...api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,16 @@ class InstrumenterTest {
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));

static class AttributesExtractor1
extends AttributesExtractor<Map<String, String>, Map<String, String>> {
implements AttributesExtractor<Map<String, String>, Map<String, String>> {

@Override
protected void onStart(AttributesBuilder attributes, Map<String, String> request) {
public void onStart(AttributesBuilder attributes, Map<String, String> request) {
attributes.put("req1", request.get("req1"));
attributes.put("req2", request.get("req2"));
}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
Map<String, String> request,
Map<String, String> response,
Expand All @@ -93,16 +93,16 @@ protected void onEnd(
}

static class AttributesExtractor2
extends AttributesExtractor<Map<String, String>, Map<String, String>> {
implements AttributesExtractor<Map<String, String>, Map<String, String>> {

@Override
protected void onStart(AttributesBuilder attributes, Map<String, String> request) {
public void onStart(AttributesBuilder attributes, Map<String, String> request) {
attributes.put("req3", request.get("req3"));
attributes.put("req2", request.get("req2_2"));
}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
Map<String, String> request,
Map<String, String> response,
Expand Down
6 changes: 3 additions & 3 deletions ...nt/instrumentation/asynchttpclient/v2_0/AsyncHttpClientAdditionalAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
import org.checkerframework.checker.nullness.qual.Nullable;

public class AsyncHttpClientAdditionalAttributesExtractor
extends AttributesExtractor<RequestContext, Response> {
implements AttributesExtractor<RequestContext, Response> {

@Override
protected void onStart(AttributesBuilder attributes, RequestContext requestContext) {}
public void onStart(AttributesBuilder attributes, RequestContext requestContext) {}

@Override
protected void onEnd(
public void onEnd(
AttributesBuilder attributes,
RequestContext requestContext,
@Nullable Response response,
Expand Down
Loading