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

Updates to http.server_name #5369

Merged
merged 7 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -68,6 +68,7 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
set(attributes, SemanticAttributes.HTTP_HOST, host(request));
set(attributes, SemanticAttributes.HTTP_TARGET, getter.target(request));
set(attributes, SemanticAttributes.HTTP_ROUTE, getter.route(request));
set(attributes, SemanticAttributes.HTTP_SERVER_NAME, getter.serverName(request));
set(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp(request));
}

Expand All @@ -80,7 +81,6 @@ public void onEnd(
@Nullable Throwable error) {

super.onEnd(attributes, context, request, response, error);
set(attributes, SemanticAttributes.HTTP_SERVER_NAME, getter.serverName(request, response));
set(attributes, SemanticAttributes.HTTP_ROUTE, httpRouteHolderGetter.apply(context));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

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

import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import javax.annotation.Nullable;

/**
Expand All @@ -33,14 +31,11 @@ public interface HttpServerAttributesGetter<REQUEST, RESPONSE>
@Nullable
String scheme(REQUEST request);

// Attributes which are not always available when the request is ready.

/**
* Extracts the {@code http.server_name} span attribute.
*
* <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether
* {@code response} is {@code null} or not.
* 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}.
*/
@Nullable
String serverName(REQUEST request, @Nullable RESPONSE response);
String serverName(REQUEST request);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public String scheme(Map<String, String> request) {
}

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

Expand Down Expand Up @@ -146,6 +146,7 @@ void normal() {
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 @@ -160,6 +161,7 @@ void normal() {
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(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public String scheme(HttpRequest request) {

@Override
@Nullable
public String serverName(HttpRequest request, @Nullable HttpResponse httpResponse) {
public String serverName(HttpRequest request) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ class RestCamelTest extends AgentInstrumentationSpecification implements RetryOn
"$SemanticAttributes.HTTP_METHOD" "GET"
"$SemanticAttributes.NET_PEER_IP" "127.0.0.1"
"$SemanticAttributes.NET_PEER_PORT" Long
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" "/api/{module}/unit/{unitId}"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ class TwoServicesWithDirectClientCamelTest extends AgentInstrumentationSpecifica
"$SemanticAttributes.NET_PEER_IP" "127.0.0.1"
"$SemanticAttributes.HTTP_USER_AGENT" "Jakarta Commons-HttpClient/3.1"
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH" Long
"$SemanticAttributes.HTTP_ROUTE" "/serviceTwo"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ public List<String> responseHeader(RequestContext ctx, RequestLog requestLog, St

@Override
@Nullable
public String serverName(RequestContext ctx, @Nullable RequestLog requestLog) {
public String serverName(RequestContext ctx) {
if (ctx instanceof ServiceRequestContext) {
return ((ServiceRequestContext) ctx).config().virtualHost().hostnamePattern();
return ((ServiceRequestContext) ctx).config().virtualHost().defaultHostname();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anuraaga can you ack this change?

}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ import io.dropwizard.setup.Bootstrap
import io.dropwizard.setup.Environment
import io.dropwizard.testing.ConfigOverride
import io.dropwizard.testing.DropwizardTestSupport
import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.api.trace.StatusCode
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.instrumentation.test.base.HttpServerTest
import io.opentelemetry.instrumentation.test.utils.PortUtils
import io.opentelemetry.sdk.trace.data.SpanData
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes

import javax.ws.rs.GET
import javax.ws.rs.HeaderParam
Expand Down Expand Up @@ -62,14 +60,6 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
testSupport.after()
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
Set<AttributeKey<?>> extra = [
SemanticAttributes.HTTP_SERVER_NAME
]
super.httpAttributes(endpoint) + extra
}

// this override is needed because dropwizard reports peer ip as the client ip
@Override
String peerIp(ServerEndpoint endpoint) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ package test
import grails.boot.GrailsApp
import grails.boot.config.GrailsAutoConfiguration
import groovy.transform.CompileStatic
import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.api.trace.StatusCode
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.instrumentation.test.base.HttpServerTest
import io.opentelemetry.sdk.trace.data.SpanData
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import org.springframework.boot.autoconfigure.SpringBootApplication
import org.springframework.boot.autoconfigure.web.ServerProperties
import org.springframework.context.ConfigurableApplicationContext
Expand Down Expand Up @@ -56,14 +54,6 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
}
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
Set<AttributeKey<?>> extra = [
SemanticAttributes.HTTP_SERVER_NAME
]
super.httpAttributes(endpoint) + extra
}

@Override
String expectedHttpRoute(ServerEndpoint endpoint) {
if (endpoint == PATH_PARAM) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public String scheme(HttpRequestPacket request) {

@Nullable
@Override
public String serverName(HttpRequestPacket request, @Nullable HttpResponsePacket response) {
public String serverName(HttpRequestPacket request) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* SPDX-License-Identifier: Apache-2.0
*/

import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.instrumentation.test.base.HttpServerTest
Expand Down Expand Up @@ -189,14 +188,6 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
"failing" | "throw" | 500 | { it == "failure" } | true | "failure"
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
Set<AttributeKey<?>> extra = [
SemanticAttributes.HTTP_SERVER_NAME
]
super.httpAttributes(endpoint) + extra
}

@Override
boolean hasHandlerSpan(ServerEndpoint endpoint) {
true
Expand Down Expand Up @@ -283,7 +274,6 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" TEST_USER_AGENT
"$SemanticAttributes.HTTP_CLIENT_IP" TEST_CLIENT_IP
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
// Optional
"$SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH" { it == null || it instanceof Long }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes.add(SemanticAttributes.HTTP_SERVER_NAME)
attributes
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes.addAll(SemanticAttributes.HTTP_SERVER_NAME)
attributes
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
// Optional
"$SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH" { it == null || it instanceof Long }
Expand Down Expand Up @@ -158,7 +157,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -211,7 +209,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH" Long
"$SemanticAttributes.HTTP_ROUTE" route
Expand Down Expand Up @@ -274,7 +271,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 500
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -341,7 +337,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -389,7 +384,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -469,7 +463,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 500
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -518,7 +511,6 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -166,7 +165,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -214,7 +212,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -310,7 +307,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -392,7 +388,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 500
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down Expand Up @@ -453,7 +448,6 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_STATUS_CODE" 404
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_SERVER_NAME" String
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
"$SemanticAttributes.HTTP_ROUTE" route
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ internal class KtorHttpServerAttributesGetter :
return request.origin.scheme
}

override fun serverName(request: ApplicationRequest, response: ApplicationResponse?): String? {
override fun serverName(request: ApplicationRequest): String? {
return null
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ public String route(LibertyRequest libertyRequest) {

@Override
@Nullable
public String serverName(
LibertyRequest libertyRequest, @Nullable LibertyResponse libertyResponse) {
public String serverName(LibertyRequest libertyRequest) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ public String scheme(HttpRequestAndChannel requestAndChannel) {

@Override
@Nullable
public String serverName(
HttpRequestAndChannel requestAndChannel, @Nullable HttpResponse response) {
public String serverName(HttpRequestAndChannel requestAndChannel) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ public String scheme(HttpRequestAndChannel requestAndChannel) {

@Override
@Nullable
public String serverName(
HttpRequestAndChannel requestAndChannel, @Nullable HttpResponse response) {
public String serverName(HttpRequestAndChannel requestAndChannel) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public String flavor(Request request) {

@Override
@Nullable
public String serverName(Request request, @Nullable Response response) {
public String serverName(Request request) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public String flavor(Request request) {

@Override
@Nullable
public String serverName(Request request, @Nullable Response response) {
public String serverName(Request request) {
return null;
}

Expand Down
Loading