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

Deprecate CapturedHttpHeaders and replace it with builder methods #5533

Merged
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 @@ -7,7 +7,6 @@

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeaders;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
Expand Down Expand Up @@ -43,9 +42,7 @@ public class InstrumenterBenchmark {
"benchmark",
HttpSpanNameExtractor.create(ConstantHttpAttributesGetter.INSTANCE))
.addAttributesExtractor(
HttpClientAttributesExtractor.builder(ConstantHttpAttributesGetter.INSTANCE)
.captureHttpHeaders(CapturedHttpHeaders.empty())
.build())
HttpClientAttributesExtractor.create(ConstantHttpAttributesGetter.INSTANCE))
.addAttributesExtractor(
NetServerAttributesExtractor.create(new ConstantNetAttributesGetter()))
.newInstrumenter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,16 @@
* attribute key. The HTTP response header values will be captured under the {@code
* http.response.header.<name>} attribute key. The {@code <name>} part in the attribute key is the
* normalized header name: lowercase, with dashes replaced by underscores.
*
* @deprecated This class should no longer be used directly. Use the {@link
* HttpClientAttributesExtractorBuilder#setCapturedRequestHeaders(List)}, {@link
* HttpClientAttributesExtractorBuilder#setCapturedResponseHeaders(List)}, {@link
* HttpServerAttributesExtractorBuilder#setCapturedRequestHeaders(List)} and {@link
* HttpServerAttributesExtractorBuilder#setCapturedResponseHeaders(List)} methods instead.
*/
@Deprecated
@AutoValue
@AutoValue.CopyAnnotations
public abstract class CapturedHttpHeaders {

private static final CapturedHttpHeaders EMPTY = create(emptyList(), emptyList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* return {@code null} from the protected attribute methods, but implement as many as possible for
* best compliance with the OpenTelemetry specification.
*/
@SuppressWarnings("deprecation") // suppress CapturedHttpHeaders deprecation
public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
extends HttpCommonAttributesExtractor<
REQUEST, RESPONSE, HttpClientAttributesGetter<REQUEST, RESPONSE>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
package io.opentelemetry.instrumentation.api.instrumenter.http;

import io.opentelemetry.instrumentation.api.config.Config;
import java.util.List;

/** A builder of {@link HttpClientAttributesExtractor}. */
@SuppressWarnings("deprecation") // suppress CapturedHttpHeaders deprecation
public final class HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> {

final HttpClientAttributesGetter<REQUEST, RESPONSE> getter;
Expand All @@ -22,13 +24,40 @@ public final class HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> {
*
* @param capturedHttpHeaders A configuration object specifying which HTTP request and response
* headers should be captured as span attributes.
* @deprecated Use {@link #setCapturedRequestHeaders(List)} and {@link
* #setCapturedResponseHeaders(List)} instead.
*/
@Deprecated
public HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> captureHttpHeaders(
CapturedHttpHeaders capturedHttpHeaders) {
this.capturedHttpHeaders = capturedHttpHeaders;
return this;
}

/**
* Configures the HTTP request headers that will be captured as span attributes.
*
* @param requestHeaders A list of HTTP header names.
*/
public HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> setCapturedRequestHeaders(
List<String> requestHeaders) {
this.capturedHttpHeaders =
CapturedHttpHeaders.create(requestHeaders, capturedHttpHeaders.responseHeaders());
return this;
}

/**
* Configures the HTTP response headers that will be captured as span attributes.
*
* @param responseHeaders A list of HTTP header names.
*/
public HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> setCapturedResponseHeaders(
List<String> responseHeaders) {
this.capturedHttpHeaders =
CapturedHttpHeaders.create(capturedHttpHeaders.requestHeaders(), responseHeaders);
return this;
}

/**
* Returns a new {@link HttpClientAttributesExtractor} with the settings of this {@link
* HttpClientAttributesExtractorBuilder}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#common-attributes">HTTP
* attributes</a> that are common to client and server instrumentations.
*/
@SuppressWarnings("deprecation") // suppress CapturedHttpHeaders deprecation
abstract class HttpCommonAttributesExtractor<
REQUEST, RESPONSE, GETTER extends HttpCommonAttributesGetter<REQUEST, RESPONSE>>
implements AttributesExtractor<REQUEST, RESPONSE> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* return {@code null} from the protected attribute methods, but implement as many as possible for
* best compliance with the OpenTelemetry specification.
*/
@SuppressWarnings("deprecation") // suppress CapturedHttpHeaders deprecation
public final class HttpServerAttributesExtractor<REQUEST, RESPONSE>
extends HttpCommonAttributesExtractor<
REQUEST, RESPONSE, HttpServerAttributesGetter<REQUEST, RESPONSE>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
package io.opentelemetry.instrumentation.api.instrumenter.http;

import io.opentelemetry.instrumentation.api.config.Config;
import java.util.List;

/** A builder of {@link HttpServerAttributesExtractor}. */
@SuppressWarnings("deprecation") // suppress CapturedHttpHeaders deprecation
public final class HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> {

final HttpServerAttributesGetter<REQUEST, RESPONSE> getter;
Expand All @@ -22,13 +24,40 @@ public final class HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> {
*
* @param capturedHttpHeaders A configuration object specifying which HTTP request and response
* headers should be captured as span attributes.
* @deprecated Use {@link #setCapturedRequestHeaders(List)} and {@link
* #setCapturedResponseHeaders(List)} instead.
*/
@Deprecated
public HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> captureHttpHeaders(
CapturedHttpHeaders capturedHttpHeaders) {
this.capturedHttpHeaders = capturedHttpHeaders;
return this;
}

/**
* Configures the HTTP request headers that will be captured as span attributes.
*
* @param requestHeaders A list of HTTP header names.
*/
public HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> setCapturedRequestHeaders(
List<String> requestHeaders) {
this.capturedHttpHeaders =
CapturedHttpHeaders.create(requestHeaders, capturedHttpHeaders.responseHeaders());
return this;
}

/**
* Configures the HTTP response headers that will be captured as span attributes.
*
* @param responseHeaders A list of HTTP header names.
*/
public HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> setCapturedResponseHeaders(
List<String> responseHeaders) {
this.capturedHttpHeaders =
CapturedHttpHeaders.create(capturedHttpHeaders.requestHeaders(), responseHeaders);
return this;
}

/**
* Returns a new {@link HttpServerAttributesExtractor} with the settings of this {@link
* HttpServerAttributesExtractorBuilder}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,8 @@ void normal() {

HttpClientAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
HttpClientAttributesExtractor.builder(new TestHttpClientAttributesGetter())
.captureHttpHeaders(
CapturedHttpHeaders.create(
singletonList("Custom-Request-Header"),
singletonList("Custom-Response-Header")))
.setCapturedRequestHeaders(singletonList("Custom-Request-Header"))
.setCapturedResponseHeaders(singletonList("Custom-Response-Header"))
.build();

AttributesBuilder attributes = Attributes.builder();
Expand Down Expand Up @@ -151,7 +149,8 @@ void invalidStatusCode() {

HttpClientAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
HttpClientAttributesExtractor.builder(new TestHttpClientAttributesGetter())
.captureHttpHeaders(CapturedHttpHeaders.empty())
.setCapturedRequestHeaders(emptyList())
.setCapturedResponseHeaders(emptyList())
.build();

AttributesBuilder attributes = Attributes.builder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.function.Function;
import org.junit.jupiter.api.Test;

@SuppressWarnings("deprecation") // suppress CapturedHttpHeaders deprecation
class HttpServerAttributesExtractorTest {

static class TestHttpServerAttributesExtractor
Expand Down Expand Up @@ -185,7 +186,8 @@ void extractClientIpFromX_Forwarded_For() {

HttpServerAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
HttpServerAttributesExtractor.builder(new TestHttpServerAttributesExtractor())
.captureHttpHeaders(CapturedHttpHeaders.empty())
.setCapturedRequestHeaders(emptyList())
.setCapturedResponseHeaders(emptyList())
.build();

AttributesBuilder attributes = Attributes.builder();
Expand All @@ -205,7 +207,8 @@ void extractClientIpFromX_Forwarded_Proto() {

HttpServerAttributesExtractor<Map<String, String>, Map<String, String>> extractor =
HttpServerAttributesExtractor.builder(new TestHttpServerAttributesExtractor())
.captureHttpHeaders(CapturedHttpHeaders.empty())
.setCapturedRequestHeaders(emptyList())
.setCapturedResponseHeaders(emptyList())
.build();

AttributesBuilder attributes = Attributes.builder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
import javax.annotation.Nullable;
import org.apache.http.HttpResponse;

final class ApacheHttpClientHttpAttributesGetter
enum ApacheHttpClientHttpAttributesGetter
implements HttpClientAttributesGetter<ApacheHttpClientRequest, HttpResponse> {
INSTANCE;

@Override
public String method(ApacheHttpClientRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
package io.opentelemetry.instrumentation.apachehttpclient.v4_3;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeaders;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractorBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor;
Expand All @@ -28,7 +27,9 @@ public final class ApacheHttpClientTracingBuilder {

private final List<AttributesExtractor<? super ApacheHttpClientRequest, ? super HttpResponse>>
additionalExtractors = new ArrayList<>();
private CapturedHttpHeaders capturedHttpHeaders = CapturedHttpHeaders.client(Config.get());
private final HttpClientAttributesExtractorBuilder<ApacheHttpClientRequest, HttpResponse>
httpAttributesExtractorBuilder =
HttpClientAttributesExtractor.builder(ApacheHttpClientHttpAttributesGetter.INSTANCE);

ApacheHttpClientTracingBuilder(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
Expand All @@ -49,12 +50,37 @@ public ApacheHttpClientTracingBuilder addAttributeExtractor(
* Configure the instrumentation to capture chosen HTTP request and response headers as span
* attributes.
*
* @param capturedHttpHeaders An instance of {@link CapturedHttpHeaders} containing the configured
* HTTP request and response names.
* @param capturedHttpHeaders An instance of {@link
* io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeaders} containing the
* configured HTTP request and response names.
* @deprecated Use {@link #setCapturedRequestHeaders(List)} and {@link
* #setCapturedResponseHeaders(List)} instead.
*/
@Deprecated
public ApacheHttpClientTracingBuilder captureHttpHeaders(
CapturedHttpHeaders capturedHttpHeaders) {
this.capturedHttpHeaders = capturedHttpHeaders;
io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeaders
capturedHttpHeaders) {
httpAttributesExtractorBuilder.captureHttpHeaders(capturedHttpHeaders);
return this;
}

/**
* Configures the HTTP request headers that will be captured as span attributes.
*
* @param requestHeaders A list of HTTP header names.
*/
public ApacheHttpClientTracingBuilder setCapturedRequestHeaders(List<String> requestHeaders) {
httpAttributesExtractorBuilder.setCapturedRequestHeaders(requestHeaders);
return this;
}

/**
* Configures the HTTP response headers that will be captured as span attributes.
*
* @param responseHeaders A list of HTTP header names.
*/
public ApacheHttpClientTracingBuilder setCapturedResponseHeaders(List<String> responseHeaders) {
httpAttributesExtractorBuilder.setCapturedResponseHeaders(responseHeaders);
return this;
}

Expand All @@ -64,7 +90,7 @@ public ApacheHttpClientTracingBuilder captureHttpHeaders(
*/
public ApacheHttpClientTracing build() {
ApacheHttpClientHttpAttributesGetter httpAttributesGetter =
new ApacheHttpClientHttpAttributesGetter();
ApacheHttpClientHttpAttributesGetter.INSTANCE;
ApacheHttpClientNetAttributesGetter netAttributesGetter =
new ApacheHttpClientNetAttributesGetter();

Expand All @@ -74,10 +100,7 @@ public ApacheHttpClientTracing build() {
INSTRUMENTATION_NAME,
HttpSpanNameExtractor.create(httpAttributesGetter))
.setSpanStatusExtractor(HttpSpanStatusExtractor.create(httpAttributesGetter))
.addAttributesExtractor(
HttpClientAttributesExtractor.builder(httpAttributesGetter)
.captureHttpHeaders(capturedHttpHeaders)
.build())
.addAttributesExtractor(httpAttributesExtractorBuilder.build())
.addAttributesExtractor(NetClientAttributesExtractor.create(netAttributesGetter))
.addAttributesExtractors(additionalExtractors)
// We manually inject because we need to inject internal requests for redirects.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
import java.util.List;
import javax.annotation.Nullable;

final class ArmeriaHttpClientAttributesGetter
enum ArmeriaHttpClientAttributesGetter
implements HttpClientAttributesGetter<RequestContext, RequestLog> {
INSTANCE;

@Override
public String method(RequestContext ctx) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
import java.util.List;
import javax.annotation.Nullable;

final class ArmeriaHttpServerAttributesGetter
enum ArmeriaHttpServerAttributesGetter
implements HttpServerAttributesGetter<RequestContext, RequestLog> {
INSTANCE;

@Override
public String method(RequestContext ctx) {
Expand Down
Loading