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
Changes from 1 commit
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
Prev Previous commit
fix all AttributesExtractors
Mateusz Rzeszutek committed Feb 2, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit b2b40c640f7df2c9b7ac624a8b1dd23c4e41e0f4
Original file line number Diff line number Diff line change
@@ -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;
@@ -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()) {
@@ -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) {}
Original file line number Diff line number Diff line change
@@ -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

@@ -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()

@@ -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")
@@ -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()

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

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

then:
0 * builder.put(*_)
@@ -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()

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

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

then:
0 * builder.put(*_)
@@ -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()

@@ -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")
@@ -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()

@@ -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")
@@ -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()

@@ -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"])
@@ -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()

@@ -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
@@ -15,20 +15,18 @@

/**
* 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> {

// TODO: use new methods everywhere

/**
* Extracts attributes from the {@link Context} and the {@link REQUEST} into the {@link
* AttributesBuilder} at the beginning of a request.
@@ -40,9 +38,10 @@ default void onStart(AttributesBuilder attributes, Context parentContext, REQUES
/**
* 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 Use {@link #onStart(AttributesBuilder, Context, Object)} instead.
// @Deprecated
@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.");
@@ -64,10 +63,10 @@ default void onEnd(
/**
* 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.
*/
// * @deprecated Use {@link #onEnd(AttributesBuilder, Context, Object, Object, Throwable)}
// instead.
// @Deprecated
@Deprecated
default void onEnd(
AttributesBuilder attributes,
REQUEST request,
Original file line number Diff line number Diff line change
@@ -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>
@@ -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) {}
Original file line number Diff line number Diff line change
@@ -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;
@@ -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) {
Original file line number Diff line number Diff line change
@@ -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;
@@ -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());
@@ -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) {}
Original file line number Diff line number Diff line change
@@ -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;
@@ -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));
@@ -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) {}
Original file line number Diff line number Diff line change
@@ -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;
@@ -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));
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.api.instrumenter.messaging;

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;
@@ -24,7 +25,7 @@ public abstract class MessagingAttributesExtractor<REQUEST, RESPONSE>
public static final String TEMP_DESTINATION_NAME = "(temporary)";

@Override
public final void onStart(AttributesBuilder attributes, REQUEST request) {
public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
set(attributes, SemanticAttributes.MESSAGING_SYSTEM, system(request));
set(attributes, SemanticAttributes.MESSAGING_DESTINATION_KIND, destinationKind(request));
boolean isTemporaryDestination = temporaryDestination(request);
@@ -55,6 +56,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) {
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.api.instrumenter.net;

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;
@@ -34,11 +35,12 @@ private NetClientAttributesExtractor(NetClientAttributesGetter<REQUEST, RESPONSE
}

@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) {
Loading