Skip to content

Commit

Permalink
Remove deprecated http.host and http.server_name attributes (open…
Browse files Browse the repository at this point in the history 
…-telemetry#6709)

... and make sure the `TemporaryMetricsView` follows the current spec
  • Loading branch information
@LironKS
Mateusz Rzeszutek authored and LironKS committed Dec 4, 2022
1 parent 2a08ee5 commit 938f00b
Show file tree
Hide file tree
Showing 37 changed files with 120 additions and 231 deletions.
7 changes: 0 additions & 7 deletions ...io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
String forwardedProto = forwardedProto(request);
String value = forwardedProto != null ? forwardedProto : getter.scheme(request);
internalSet(attributes, SemanticAttributes.HTTP_SCHEME, value);
internalSet(attributes, SemanticAttributes.HTTP_HOST, host(request));
internalSet(attributes, SemanticAttributes.HTTP_TARGET, getter.target(request));
internalSet(attributes, SemanticAttributes.HTTP_ROUTE, getter.route(request));
internalSet(attributes, SemanticAttributes.HTTP_SERVER_NAME, getter.serverName(request));
internalSet(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp(request));
}

Expand All @@ -95,11 +93,6 @@ public void onEnd(
internalSet(attributes, SemanticAttributes.HTTP_ROUTE, httpRouteHolderGetter.apply(context));
}

@Nullable
private String host(REQUEST request) {
return firstHeaderValue(getter.requestHeader(request, "host"));
}

@Nullable
private String forwardedProto(REQUEST request) {
// try Forwarded
Expand Down
8 changes: 7 additions & 1 deletion ...va/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesGetter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ public interface HttpServerAttributesGetter<REQUEST, RESPONSE>
* The primary server name of the matched virtual host. This should be obtained via configuration,
* not from the Host header. If no such configuration can be obtained, this method should return
* {@code null}.
*
* @deprecated This method is deprecated and will be removed in the next release.
*/
@Nullable
String serverName(REQUEST request);
@Deprecated
default String serverName(REQUEST request) {
throw new UnsupportedOperationException(
"This method is deprecated and will be removed in the next release");
}
}
8 changes: 5 additions & 3 deletions ...ain/java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ private static Set<AttributeKey> buildDurationClientView() {
Set<AttributeKey> view = new HashSet<>(durationAlwaysInclude);
view.add(SemanticAttributes.NET_PEER_NAME);
view.add(SemanticAttributes.NET_PEER_PORT);
view.add(AttributeKey.stringKey("net.peer.sock.addr"));
return view;
}

Expand All @@ -51,7 +52,8 @@ private static Set<AttributeKey> buildDurationServerView() {
// - we prefer http.route (which is scrubbed) over http.target (which is not scrubbed).
Set<AttributeKey> view = new HashSet<>(durationAlwaysInclude);
view.add(SemanticAttributes.HTTP_SCHEME);
view.add(SemanticAttributes.HTTP_HOST);
view.add(SemanticAttributes.NET_HOST_NAME);
view.add(SemanticAttributes.NET_HOST_PORT);
view.add(SemanticAttributes.HTTP_ROUTE);
return view;
}
Expand All @@ -61,10 +63,10 @@ private static Set<AttributeKey> buildActiveRequestsView() {
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#attributes
Set<AttributeKey> view = new HashSet<>();
view.add(SemanticAttributes.HTTP_METHOD);
view.add(SemanticAttributes.HTTP_HOST);
view.add(SemanticAttributes.HTTP_SCHEME);
view.add(SemanticAttributes.HTTP_FLAVOR);
view.add(SemanticAttributes.HTTP_SERVER_NAME);
view.add(SemanticAttributes.NET_HOST_NAME);
// TODO: net host port?
return view;
}

Expand Down
28 changes: 17 additions & 11 deletions ...st/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

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

import static io.opentelemetry.api.common.AttributeKey.stringKey;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;

Expand Down Expand Up @@ -35,7 +36,6 @@ void collectsMetrics() {
Attributes.builder()
.put("http.method", "GET")
.put("http.url", "https://localhost:1234/")
.put("http.host", "host")
.put("http.target", "/")
.put("http.scheme", "https")
.put("net.peer.name", "localhost")
Expand All @@ -46,9 +46,12 @@ void collectsMetrics() {
Attributes responseAttributes =
Attributes.builder()
.put("http.flavor", "2.0")
.put("http.server_name", "server")
.put("http.status_code", 200)
.put("http.response_content_length", 200)
.put("net.sock.family", "inet")
.put("net.peer.sock.addr", "1.2.3.4")
.put("net.peer.sock.name", "somehost20")
.put("net.peer.sock.port", 8080)
.build();

Context parent =
Expand Down Expand Up @@ -84,11 +87,12 @@ void collectsMetrics() {
point
.hasSum(150 /* millis */)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
equalTo(SemanticAttributes.NET_PEER_PORT, 1234),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200))
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
equalTo(SemanticAttributes.NET_PEER_PORT, 1234),
equalTo(stringKey("net.peer.sock.addr"), "1.2.3.4"))
.hasExemplarsSatisfying(
exemplar ->
exemplar
Expand All @@ -105,11 +109,12 @@ void collectsMetrics() {
point
.hasSum(100 /* bytes */)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
equalTo(SemanticAttributes.NET_PEER_PORT, 1234),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200)))),
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
equalTo(SemanticAttributes.NET_PEER_PORT, 1234),
equalTo(stringKey("net.peer.sock.addr"), "1.2.3.4")))),
metric ->
assertThat(metric)
.hasName("http.client.response.size")
Expand All @@ -121,11 +126,12 @@ void collectsMetrics() {
point
.hasSum(200 /* bytes */)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
equalTo(SemanticAttributes.NET_PEER_PORT, 1234),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200)))));
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
equalTo(SemanticAttributes.NET_PEER_PORT, 1234),
equalTo(stringKey("net.peer.sock.addr"), "1.2.3.4")))));

listener.onEnd(context2, responseAttributes, nanos(300));

Expand Down
11 changes: 0 additions & 11 deletions ...pentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ public String scheme(Map<String, String> request) {
return request.get("scheme");
}

@Override
public String serverName(Map<String, String> request) {
return request.get("serverName");
}

@Override
public List<String> requestHeader(Map<String, String> request, String name) {
String values = request.get("header." + name);
Expand Down Expand Up @@ -89,7 +84,6 @@ void normal() {
request.put("header.content-length", "10");
request.put("flavor", "http/2");
request.put("route", "/repositories/{id}");
request.put("serverName", "server");
request.put("header.user-agent", "okhttp 3.x");
request.put("header.host", "github.com");
request.put("header.forwarded", "for=1.1.1.1;proto=https");
Expand All @@ -116,11 +110,9 @@ void normal() {
entry(SemanticAttributes.HTTP_FLAVOR, "http/2"),
entry(SemanticAttributes.HTTP_METHOD, "POST"),
entry(SemanticAttributes.HTTP_SCHEME, "https"),
entry(SemanticAttributes.HTTP_HOST, "github.com"),
entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"),
entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"),
entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"),
entry(SemanticAttributes.HTTP_SERVER_NAME, "server"),
entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"),
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
Expand All @@ -131,17 +123,14 @@ void normal() {
.containsOnly(
entry(SemanticAttributes.HTTP_METHOD, "POST"),
entry(SemanticAttributes.HTTP_SCHEME, "https"),
entry(SemanticAttributes.HTTP_HOST, "github.com"),
entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"),
entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"),
entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{repoId}"),
entry(SemanticAttributes.HTTP_SERVER_NAME, "server"),
entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"),
entry(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH, 10L),
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
asList("123", "456")),
entry(SemanticAttributes.HTTP_SERVER_NAME, "server"),
entry(SemanticAttributes.HTTP_FLAVOR, "http/2"),
entry(SemanticAttributes.HTTP_STATUS_CODE, 202L),
entry(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH, 20L),
Expand Down
54 changes: 33 additions & 21 deletions ...st/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HttpFlavorValues.HTTP_2_0;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
Expand Down Expand Up @@ -34,19 +36,23 @@ void collectsMetrics() {
Attributes requestAttributes =
Attributes.builder()
.put("http.method", "GET")
.put("http.host", "host")
.put("http.flavor", HTTP_2_0)
.put("http.target", "/")
.put("http.scheme", "https")
.put("net.transport", IP_TCP)
.put("net.host.name", "localhost")
.put("net.host.port", 1234)
.put("http.request_content_length", 100)
.put("net.sock.family", "inet")
.put("net.peer.sock.addr", "1.2.3.4")
.put("net.peer.sock.port", 8080)
.put("net.host.sock.addr", "4.3.2.1")
.put("net.host.sock.port", 9090)
.build();

Attributes responseAttributes =
Attributes.builder()
.put("http.flavor", "2.0")
.put("http.server_name", "server")
.put("http.status_code", 200)
.put("http.request_content_length", 100)
.put("http.response_content_length", 200)
.build();

Expand Down Expand Up @@ -81,9 +87,10 @@ void collectsMetrics() {
point
.hasValue(1)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.HTTP_HOST, "host"),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_SCHEME, "https"))
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.HTTP_FLAVOR, HTTP_2_0),
equalTo(SemanticAttributes.NET_HOST_NAME, "localhost"))
.hasExemplarsSatisfying(
exemplar ->
exemplar
Expand All @@ -105,9 +112,10 @@ void collectsMetrics() {
point
.hasValue(2)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.HTTP_HOST, "host"),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_SCHEME, "https"))
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.HTTP_FLAVOR, HTTP_2_0),
equalTo(SemanticAttributes.NET_HOST_NAME, "localhost"))
.hasExemplarsSatisfying(
exemplar ->
exemplar
Expand All @@ -128,9 +136,10 @@ void collectsMetrics() {
point
.hasValue(1)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.HTTP_HOST, "host"),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_SCHEME, "https"))
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.HTTP_FLAVOR, HTTP_2_0),
equalTo(SemanticAttributes.NET_HOST_NAME, "localhost"))
.hasExemplarsSatisfying(
exemplar ->
exemplar
Expand All @@ -147,11 +156,12 @@ void collectsMetrics() {
point
.hasSum(150 /* millis */)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.HTTP_HOST, "host"),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0"))
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0"),
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.NET_HOST_NAME, "localhost"),
equalTo(SemanticAttributes.NET_HOST_PORT, 1234))
.hasExemplarsSatisfying(
exemplar ->
exemplar
Expand All @@ -168,11 +178,12 @@ void collectsMetrics() {
point
.hasSum(100 /* bytes */)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.HTTP_HOST, "host"),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0")))),
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0"),
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.NET_HOST_NAME, "localhost"),
equalTo(SemanticAttributes.NET_HOST_PORT, 1234)))),
metric ->
assertThat(metric)
.hasName("http.server.response.size")
Expand All @@ -184,11 +195,12 @@ void collectsMetrics() {
point
.hasSum(200 /* bytes */)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.HTTP_HOST, "host"),
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0")))));
equalTo(SemanticAttributes.HTTP_FLAVOR, "2.0"),
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.NET_HOST_NAME, "localhost"),
equalTo(SemanticAttributes.NET_HOST_PORT, 1234)))));

listener.onEnd(context2, responseAttributes, nanos(300));

Expand Down Expand Up @@ -237,7 +249,7 @@ void collectsHttpRouteFromEndAttributes() {
OperationListener listener = HttpServerMetrics.get().create(meterProvider.get("test"));

Attributes requestAttributes =
Attributes.builder().put("http.host", "host").put("http.scheme", "https").build();
Attributes.builder().put("net.host.name", "host").put("http.scheme", "https").build();

Attributes responseAttributes = Attributes.builder().put("http.route", "/test/{id}").build();

Expand All @@ -262,7 +274,7 @@ void collectsHttpRouteFromEndAttributes() {
.hasSum(100 /* millis */)
.hasAttributesSatisfying(
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
equalTo(SemanticAttributes.HTTP_HOST, "host"),
equalTo(SemanticAttributes.NET_HOST_NAME, "host"),
equalTo(
SemanticAttributes.HTTP_ROUTE, "/test/{id}")))));
}
Expand Down
Loading

0 comments on commit 938f00b

Please sign in to comment.