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
Mateusz Rzeszutek committed Sep 24, 2022
1 parent 49af556 commit 714ba91
Show file tree
Hide file tree
Showing 38 changed files with 120 additions and 237 deletions.
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
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");
}
}
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
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
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
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 714ba91

Please sign in to comment.