Skip to content

Commit

Permalink
Instrumenter API improvements (#2860)
Browse files Browse the repository at this point in the history
* Instrumenter API improvements

* Move HTTP & net classes to separate packages
* Remove `db` prefix from method names in `DbAttributesExtractor`
* Add request-only net attributes extractor (it'll be needed in spring-sleuth-otel once we decide to use Instrumenters there)

* One NetAttributesExtractor class, javadocs

* add missing @nullable

* Apply suggestions from code review

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

* spotless

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
  • Loading branch information
Mateusz Rzeszutek and trask committed May 6, 2021
1 parent a568daa commit c9a3793
Show file tree
Hide file tree
Showing 26 changed files with 276 additions and 195 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.db.DbAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetAttributesExtractor;

/**
* Extractor of {@link io.opentelemetry.api.common.Attributes} for a given request and response.
Expand All @@ -16,6 +19,7 @@
* lifecycle. It is best to populate as much as possible in {@link #onStart(AttributesBuilder,
* Object)} to have it available during sampling.
*
* @see DbAttributesExtractor
* @see HttpAttributesExtractor
* @see NetAttributesExtractor
*/
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,6 @@
@FunctionalInterface
public interface SpanNameExtractor<REQUEST> {

/**
* Returns a {@link SpanNameExtractor} which should be used for HTTP requests. HTTP attributes
* will be examined to determine the name of the span.
*/
static <REQUEST> SpanNameExtractor<REQUEST> http(
HttpAttributesExtractor<REQUEST, ?> attributesExtractor) {
return new HttpSpanNameExtractor<>(attributesExtractor);
}

/** Returns the span name. */
String extract(REQUEST request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ static <REQUEST, RESPONSE> SpanStatusExtractor<REQUEST, RESPONSE> getDefault() {
return (SpanStatusExtractor<REQUEST, RESPONSE>) DefaultSpanStatusExtractor.INSTANCE;
}

/**
* Returns the {@link SpanStatusExtractor} for HTTP requests, which will use the HTTP status code
* to determine the {@link StatusCode} if available or fallback to {@linkplain #getDefault() the
* default status} otherwise.
*/
static <REQUEST, RESPONSE> SpanStatusExtractor<REQUEST, RESPONSE> http(
HttpAttributesExtractor<REQUEST, RESPONSE> attributesExtractor) {
return new HttpSpanStatusExtractor<>(attributesExtractor);
}

/** Returns the {@link StatusCode}. */
StatusCode extract(REQUEST request, RESPONSE response, @Nullable Throwable error);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,32 @@
public abstract class DbAttributesExtractor<REQUEST> extends AttributesExtractor<REQUEST, Void> {
@Override
protected void onStart(AttributesBuilder attributes, REQUEST request) {
set(attributes, SemanticAttributes.DB_SYSTEM, dbSystem(request));
set(attributes, SemanticAttributes.DB_USER, dbUser(request));
set(attributes, SemanticAttributes.DB_NAME, dbName(request));
set(attributes, SemanticAttributes.DB_CONNECTION_STRING, dbConnectionString(request));
set(attributes, SemanticAttributes.DB_STATEMENT, dbStatement(request));
set(attributes, SemanticAttributes.DB_OPERATION, dbOperation(request));
set(attributes, SemanticAttributes.DB_SYSTEM, system(request));
set(attributes, SemanticAttributes.DB_USER, user(request));
set(attributes, SemanticAttributes.DB_NAME, name(request));
set(attributes, SemanticAttributes.DB_CONNECTION_STRING, connectionString(request));
set(attributes, SemanticAttributes.DB_STATEMENT, statement(request));
set(attributes, SemanticAttributes.DB_OPERATION, operation(request));
}

@Override
protected final void onEnd(AttributesBuilder attributes, REQUEST request, Void unused) {}

@Nullable
protected abstract String dbSystem(REQUEST request);
protected abstract String system(REQUEST request);

@Nullable
protected abstract String dbUser(REQUEST request);
protected abstract String user(REQUEST request);

@Nullable
protected abstract String dbName(REQUEST request);
protected abstract String name(REQUEST request);

@Nullable
protected abstract String dbConnectionString(REQUEST request);
protected abstract String connectionString(REQUEST request);

@Nullable
protected abstract String dbStatement(REQUEST request);
protected abstract String statement(REQUEST request);

@Nullable
protected abstract String dbOperation(REQUEST request);
protected abstract String operation(REQUEST request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
public final class DbSpanNameExtractor<REQUEST> implements SpanNameExtractor<REQUEST> {
/**
* Returns a {@link SpanNameExtractor} that constructs the span name according to DB semantic
* conventions: {@code <db.operation> <db.name><table>}.
* conventions: {@code <db.operation> <db.name>.<table>}.
*
* @see DbAttributesExtractor#dbOperation(Object) used to extract {@code <db.operation>}.
* @see DbAttributesExtractor#dbName(Object) used to extract {@code <db.name>}.
* @see SqlAttributesExtractor#dbTable(Object) used to extract {@code <db.table>}.
* @see DbAttributesExtractor#operation(Object) used to extract {@code <db.operation>}.
* @see DbAttributesExtractor#name(Object) used to extract {@code <db.name>}.
* @see SqlAttributesExtractor#table(Object) used to extract {@code <db.table>}.
*/
public static <REQUEST> SpanNameExtractor<REQUEST> create(
DbAttributesExtractor<REQUEST> attributesExtractor) {
Expand All @@ -32,8 +32,8 @@ private DbSpanNameExtractor(DbAttributesExtractor<REQUEST> attributesExtractor)

@Override
public String extract(REQUEST request) {
String operation = attributesExtractor.dbOperation(request);
String dbName = attributesExtractor.dbName(request);
String operation = attributesExtractor.operation(request);
String dbName = attributesExtractor.name(request);
if (operation == null) {
return dbName == null ? DEFAULT_SPAN_NAME : dbName;
}
Expand All @@ -58,7 +58,7 @@ public String extract(REQUEST request) {
@Nullable
private String getTableName(REQUEST request) {
if (attributesExtractor instanceof SqlAttributesExtractor) {
return ((SqlAttributesExtractor<REQUEST>) attributesExtractor).dbTable(request);
return ((SqlAttributesExtractor<REQUEST>) attributesExtractor).table(request);
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,35 +29,35 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
super.onStart(attributes, request);
AttributeKey<String> dbTable = dbTableAttribute();
if (dbTable != null) {
set(attributes, dbTable, dbTable(request));
set(attributes, dbTable, table(request));
}
}

@Nullable
@Override
protected final String dbStatement(REQUEST request) {
protected final String statement(REQUEST request) {
return sanitize(request).getFullStatement();
}

@Nullable
@Override
protected final String dbOperation(REQUEST request) {
protected final String operation(REQUEST request) {
return sanitize(request).getOperation();
}

@Nullable
protected final String dbTable(REQUEST request) {
protected final String table(REQUEST request) {
return sanitize(request).getTable();
}

private SqlStatementInfo sanitize(REQUEST request) {
// sanitized statement is cached
return SqlStatementSanitizer.sanitize(rawDbStatement(request));
return SqlStatementSanitizer.sanitize(rawStatement(request));
}

@Nullable
protected abstract AttributeKey<String> dbTableAttribute();

@Nullable
protected abstract String rawDbStatement(REQUEST request);
protected abstract String rawStatement(REQUEST request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter;
package io.opentelemetry.instrumentation.api.instrumenter.http;

import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter.http;

import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;

/**
* Extractor of the <a
* href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#name">HTTP
* span name</a>. Instrumentation of HTTP server or client frameworks should use this class to
* comply with OpenTelemetry HTTP semantic conventions.
*/
public final class HttpSpanNameExtractor<REQUEST> implements SpanNameExtractor<REQUEST> {

/**
* Returns a {@link SpanNameExtractor} which should be used for HTTP requests. HTTP attributes
* will be examined to determine the name of the span.
*/
public static <REQUEST> SpanNameExtractor<REQUEST> create(
HttpAttributesExtractor<REQUEST, ?> attributesExtractor) {
return new HttpSpanNameExtractor<>(attributesExtractor);
}

private final HttpAttributesExtractor<REQUEST, ?> attributesExtractor;

private HttpSpanNameExtractor(HttpAttributesExtractor<REQUEST, ?> attributesExtractor) {
this.attributesExtractor = attributesExtractor;
}

@Override
public String extract(REQUEST request) {
String route = attributesExtractor.route(request);
if (route != null) {
return route;
}
String method = attributesExtractor.method(request);
if (method != null) {
return "HTTP " + method;
}
return "HTTP request";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter.http;

import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor;
import io.opentelemetry.instrumentation.api.tracer.HttpStatusConverter;

/**
* Extractor of the <a
* href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status">HTTP
* span status</a>. Instrumentation of HTTP server or client frameworks should use this class to
* comply with OpenTelemetry HTTP semantic conventions.
*/
public final class HttpSpanStatusExtractor<REQUEST, RESPONSE>
implements SpanStatusExtractor<REQUEST, RESPONSE> {

/**
* Returns the {@link SpanStatusExtractor} for HTTP requests, which will use the HTTP status code
* to determine the {@link StatusCode} if available or fallback to {@linkplain #getDefault() the
* default status} otherwise.
*/
public static <REQUEST, RESPONSE> SpanStatusExtractor<REQUEST, RESPONSE> create(
HttpAttributesExtractor<REQUEST, RESPONSE> attributesExtractor) {
return new HttpSpanStatusExtractor<>(attributesExtractor);
}

private final HttpAttributesExtractor<REQUEST, RESPONSE> attributesExtractor;

private HttpSpanStatusExtractor(HttpAttributesExtractor<REQUEST, RESPONSE> attributesExtractor) {
this.attributesExtractor = attributesExtractor;
}

@Override
public StatusCode extract(REQUEST request, RESPONSE response, Throwable error) {
Long statusCode = attributesExtractor.statusCode(request, response);
if (statusCode != null) {
return HttpStatusConverter.statusFromHttpStatus(statusCode.intValue());
}
return SpanStatusExtractor.getDefault().extract(request, response, error);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter;
package io.opentelemetry.instrumentation.api.instrumenter.net;

import java.net.InetAddress;
import java.net.InetSocketAddress;
Expand All @@ -19,12 +19,17 @@
public abstract class InetSocketAddressNetAttributesExtractor<REQUEST, RESPONSE>
extends NetAttributesExtractor<REQUEST, RESPONSE> {

/**
* This method will be called twice: both when the request starts ({@code response} is always null
* then) and when the response ends. This way it is possible to capture net attributes in both
* phases of processing.
*/
@Nullable
protected abstract InetSocketAddress getAddress(REQUEST request, RESPONSE response);
protected abstract InetSocketAddress getAddress(REQUEST request, @Nullable RESPONSE response);

@Override
@Nullable
protected final String peerName(REQUEST request, RESPONSE response) {
protected final String peerName(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
if (address == null) {
return null;
Expand All @@ -37,7 +42,7 @@ protected final String peerName(REQUEST request, RESPONSE response) {

@Override
@Nullable
protected final Long peerPort(REQUEST request, RESPONSE response) {
protected final Long peerPort(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
if (address == null) {
return null;
Expand All @@ -47,7 +52,7 @@ protected final Long peerPort(REQUEST request, RESPONSE response) {

@Override
@Nullable
protected final String peerIp(REQUEST request, RESPONSE response) {
protected final String peerIp(REQUEST request, @Nullable RESPONSE response) {
InetSocketAddress address = getAddress(request, response);
if (address == null) {
return null;
Expand Down

0 comments on commit c9a3793

Please sign in to comment.