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.

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

Refactor AttributesExtractor so that it extracts route from Context #5288

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.api.annotation.support;

import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.cache.Cache;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -45,7 +46,7 @@ public static <REQUEST, RESPONSE> MethodSpanAttributesExtractor<REQUEST, RESPONS
}

@Override
public void onStart(AttributesBuilder attributes, REQUEST request) {
public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
Method method = methodExtractor.extract(request);
AttributeBindings bindings = cache.computeIfAbsent(method, this::bind);
if (!bindings.isEmpty()) {
Expand All @@ -57,6 +58,7 @@ public void onStart(AttributesBuilder attributes, REQUEST request) {
@Override
public void onEnd(
AttributesBuilder attributes,
Context context,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.api.annotation.support

import io.opentelemetry.api.common.AttributesBuilder
import io.opentelemetry.context.Context
import io.opentelemetry.instrumentation.api.cache.Cache
import spock.lang.Specification

Expand All @@ -17,6 +18,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
def "extracts attributes for method with attribute names"() {
given:
def request = new Object()
def context = Context.root()
def method = TestClass.getDeclaredMethod("method", String, String, String)
AttributesBuilder builder = Mock()

Expand All @@ -32,7 +34,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
)

when:
extractor.onStart(builder, request)
extractor.onStart(builder, context, request)

then:
1 * builder.put({ it.getKey() == "x" }, "a")
Expand All @@ -43,6 +45,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
def "does not extract attributes for empty attribute name array"() {
given:
def request = new Object()
def context = Context.root()
def method = TestClass.getDeclaredMethod("method", String, String, String)
AttributesBuilder builder = Mock()

Expand All @@ -58,7 +61,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
)

when:
extractor.onStart(builder, request)
extractor.onStart(builder, context, request)

then:
0 * builder.put(*_)
Expand All @@ -67,6 +70,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
def "does not extract attributes for method with attribute names array with fewer elements than parameters"() {
given:
def request = new Object()
def context = Context.root()
def method = TestClass.getDeclaredMethod("method", String, String, String)
AttributesBuilder builder = Mock()

Expand All @@ -82,7 +86,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
)

when:
extractor.onStart(builder, request)
extractor.onStart(builder, context, request)

then:
0 * builder.put(*_)
Expand All @@ -91,6 +95,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
def "extracts attributes for method with attribute names array with null element"() {
given:
def request = new Object()
def context = Context.root()
def method = TestClass.getDeclaredMethod("method", String, String, String)
AttributesBuilder builder = Mock()

Expand All @@ -106,7 +111,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
)

when:
extractor.onStart(builder, request)
extractor.onStart(builder, context, request)

then:
1 * builder.put({ it.getKey() == "x" }, "a")
Expand All @@ -117,6 +122,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
def "does not extracts attribute for method with null argument"() {
given:
def request = new Object()
def context = Context.root()
def method = TestClass.getDeclaredMethod("method", String, String, String)
AttributesBuilder builder = Mock()

Expand All @@ -132,7 +138,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
)

when:
extractor.onStart(builder, request)
extractor.onStart(builder, context, request)

then:
1 * builder.put({ it.getKey() == "x" }, "a")
Expand All @@ -143,6 +149,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
def "applies cached bindings"() {
given:
def request = new Object()
def context = Context.root()
def method = TestClass.getDeclaredMethod("method", String, String, String)
AttributesBuilder builder = Mock()

Expand All @@ -161,7 +168,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
)

when:
extractor.onStart(builder, request)
extractor.onStart(builder, context, request)

then:
1 * bindings.apply(_, ["a", "b", "c"])
Expand All @@ -170,6 +177,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
def "does not apply cached empty bindings"() {
given:
def request = new Object()
def context = Context.root()
def method = TestClass.getDeclaredMethod("method", String, String, String)
AttributesBuilder builder = Mock()

Expand All @@ -188,13 +196,14 @@ class MethodSpanAttributesExtractorTest extends Specification {
)

when:
extractor.onStart(builder, request)
extractor.onStart(builder, context, request)

then:
0 * bindings.apply(_, _)
}

class TestClass {
@SuppressWarnings("unused")
void method(String x, String y, String z) {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,74 @@

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.db.DbAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import javax.annotation.Nullable;

/**
* Extractor of {@link io.opentelemetry.api.common.Attributes} for a given request and response.
* Will be called {@linkplain #onStart(AttributesBuilder, Object) on start} with just the {@link
* REQUEST} and again {@linkplain #onEnd(AttributesBuilder, Object, Object, Throwable) on end} with
* both {@link REQUEST} and {@link RESPONSE} to allow populating attributes at each stage of a
* request's lifecycle. It is best to populate as much as possible in {@link
* #onStart(AttributesBuilder, Object)} to have it available during sampling.
* Will be called {@linkplain #onStart(AttributesBuilder, Context, Object) on start} with just the
* {@link REQUEST} and again {@linkplain #onEnd(AttributesBuilder, Context, Object, Object,
* Throwable) on end} with both {@link REQUEST} and {@link RESPONSE} to allow populating attributes
* at each stage of a request's lifecycle. It is best to populate as much as possible in {@link
* #onStart(AttributesBuilder, Context, Object)} to have it available during sampling.
*
* @see DbAttributesExtractor
* @see HttpClientAttributesExtractor
* @see NetServerAttributesExtractor
*/
public interface AttributesExtractor<REQUEST, RESPONSE> {

/**
* Extracts attributes from the {@link Context} and the {@link REQUEST} into the {@link
* AttributesBuilder} at the beginning of a request.
*/
default void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
onStart(attributes, request);
}

/**
* Extracts attributes from the {@link REQUEST} into the {@link AttributesBuilder} at the
* beginning of a request.
*
* @deprecated Use {@link #onStart(AttributesBuilder, Context, Object)} instead.
*/
@Deprecated
default void onStart(AttributesBuilder attributes, REQUEST request) {
throw new UnsupportedOperationException(
"This method variant is deprecated and will be removed in the next minor release.");
}

/**
* Extracts attributes from the {@link Context}, the {@link REQUEST} and either {@link RESPONSE}
* or {@code error} into the {@link AttributesBuilder} at the end of a request.
*/
void onStart(AttributesBuilder attributes, REQUEST request);
default void onEnd(
AttributesBuilder attributes,
Context context,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error) {
onEnd(attributes, request, response, error);
}

/**
* Extracts attributes from the {@link REQUEST} and either {@link RESPONSE} or {@code error} into
* the {@link AttributesBuilder} at the end of a request.
*
* @deprecated Use {@link #onEnd(AttributesBuilder, Context, Object, Object, Throwable)} instead.
*/
void onEnd(
@Deprecated
default void onEnd(
AttributesBuilder attributes,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error);
@Nullable Throwable error) {
throw new UnsupportedOperationException(
"This method variant is deprecated and will be removed in the next minor release.");
}

/**
* Sets the {@code value} with the given {@code key} to the {@link AttributesBuilder} if {@code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import javax.annotation.Nullable;

final class ConstantAttributesExtractor<REQUEST, RESPONSE, T>
Expand All @@ -21,13 +22,14 @@ final class ConstantAttributesExtractor<REQUEST, RESPONSE, T>
}

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

@Override
public void onEnd(
AttributesBuilder attributes,
Context context,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public Context start(Context parentContext, REQUEST request) {

UnsafeAttributes attributesBuilder = new UnsafeAttributes();
for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor : attributesExtractors) {
extractor.onStart(attributesBuilder, request);
extractor.onStart(attributesBuilder, parentContext, request);
}
Attributes attributes = attributesBuilder;

Expand Down Expand Up @@ -221,7 +221,7 @@ public void end(

UnsafeAttributes attributes = new UnsafeAttributes();
for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor : attributesExtractors) {
extractor.onEnd(attributes, request, response, error);
extractor.onEnd(attributes, context, request, response, error);
}
span.setAllAttributes(attributes);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static java.util.Collections.emptyMap;

import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
Expand Down Expand Up @@ -49,11 +50,12 @@ public static <REQUEST, RESPONSE> PeerServiceAttributesExtractor<REQUEST, RESPON
}

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

@Override
public void onEnd(
AttributesBuilder attributes,
Context context,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.api.instrumenter.code;

import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;
Expand All @@ -19,7 +20,7 @@ public abstract class CodeAttributesExtractor<REQUEST, RESPONSE>
implements AttributesExtractor<REQUEST, RESPONSE> {

@Override
public final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
Class<?> cls = codeClass(request);
if (cls != null) {
set(attributes, SemanticAttributes.CODE_NAMESPACE, cls.getName());
Expand All @@ -32,6 +33,7 @@ public final void onStart(AttributesBuilder attributes, REQUEST request) {
@Override
public final void onEnd(
AttributesBuilder attributes,
Context context,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.api.instrumenter.db;

import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;
Expand All @@ -21,8 +22,9 @@
*/
public abstract class DbAttributesExtractor<REQUEST, RESPONSE>
implements AttributesExtractor<REQUEST, RESPONSE> {

@Override
public void onStart(AttributesBuilder attributes, REQUEST request) {
public void onStart(AttributesBuilder attributes, Context parentContext, 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 @@ -34,6 +36,7 @@ public void onStart(AttributesBuilder attributes, REQUEST request) {
@Override
public final void onEnd(
AttributesBuilder attributes,
Context context,
REQUEST request,
@Nullable RESPONSE response,
@Nullable Throwable error) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.db.SqlStatementInfo;
import io.opentelemetry.instrumentation.api.db.SqlStatementSanitizer;
import javax.annotation.Nullable;
Expand All @@ -26,8 +27,8 @@ public abstract class SqlAttributesExtractor<REQUEST, RESPONSE>
extends DbAttributesExtractor<REQUEST, RESPONSE> {

@Override
public final void onStart(AttributesBuilder attributes, REQUEST request) {
super.onStart(attributes, request);
public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
super.onStart(attributes, parentContext, request);
AttributeKey<String> dbTable = dbTableAttribute();
if (dbTable != null) {
set(attributes, dbTable, table(request));
Expand Down
Loading