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

Remove HttpServerTest#extraAttributes() method #5176

Merged
Show file tree
Hide file tree
Changes from 4 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 @@ -6,7 +6,6 @@
import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpServerTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes

abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<Object> implements AgentTestTrait {

Expand All @@ -33,9 +32,7 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<Object>

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes
[]
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,13 @@ abstract class AbstractArmeriaHttpServerTest extends HttpServerTest<Server> {
}

@Override
List<AttributeKey<?>> extraAttributes() {
[
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
Set<AttributeKey<?>> extra = [
SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH,
SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH,
SemanticAttributes.HTTP_SERVER_NAME,
SemanticAttributes.NET_PEER_NAME,
SemanticAttributes.NET_TRANSPORT
SemanticAttributes.HTTP_SERVER_NAME
]
super.httpAttributes(endpoint) + extra
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,11 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
}

@Override
List<AttributeKey<?>> extraAttributes() {
[
SemanticAttributes.HTTP_SERVER_NAME,
SemanticAttributes.NET_PEER_NAME,
SemanticAttributes.NET_TRANSPORT
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,11 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
}

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

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,11 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
}

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

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,16 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
}

@Override
List<AttributeKey<?>> extraAttributes() {
[
SemanticAttributes.HTTP_SERVER_NAME,
SemanticAttributes.NET_TRANSPORT
]
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes.add(SemanticAttributes.HTTP_SERVER_NAME)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint) {
"HTTP GET"
}

@Override
Expand Down Expand Up @@ -114,7 +119,7 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
case EXCEPTION:
throw new Exception(endpoint.body)
case INDEXED_CHILD:
INDEXED_CHILD.collectSpanAttributes {name -> request.getParameter(name) }
INDEXED_CHILD.collectSpanAttributes { name -> request.getParameter(name) }
response.status = endpoint.status
response.writer.print(endpoint.body)
break
Expand All @@ -140,16 +145,4 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
}
}
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint) {
"HTTP GET"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,16 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
}

@Override
List<AttributeKey<?>> extraAttributes() {
[
SemanticAttributes.HTTP_SERVER_NAME,
SemanticAttributes.NET_TRANSPORT
]
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes.addAll(SemanticAttributes.HTTP_SERVER_NAME)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint) {
"HTTP GET"
}

@Override
Expand Down Expand Up @@ -115,7 +120,7 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
case EXCEPTION:
throw new Exception(endpoint.body)
case INDEXED_CHILD:
INDEXED_CHILD.collectSpanAttributes {name -> request.getParameter(name) }
INDEXED_CHILD.collectSpanAttributes { name -> request.getParameter(name) }
response.status = endpoint.status
response.writer.print(endpoint.body)
break
Expand All @@ -141,16 +146,4 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
}
}
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint) {
"HTTP GET"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ class KtorHttpServerTest extends HttpServerTest<ApplicationEngine> implements Li
true
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.NET_PEER_PORT)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint) {
def route = expectedHttpRoute(endpoint)
Expand All @@ -54,12 +61,4 @@ class KtorHttpServerTest extends HttpServerTest<ApplicationEngine> implements Li
return super.expectedHttpRoute(endpoint)
}
}

@Override
List<AttributeKey<?>> extraAttributes() {
[
SemanticAttributes.NET_PEER_NAME,
SemanticAttributes.NET_TRANSPORT
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@

package io.opentelemetry.javaagent.instrumentation.netty.v3_8.server;

import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP;

import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.v3_8.HttpRequestAndChannel;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import javax.annotation.Nullable;
import org.jboss.netty.channel.socket.DatagramChannel;
import org.jboss.netty.handler.codec.http.HttpResponse;

final class NettyNetServerAttributesExtractor
Expand All @@ -18,7 +22,7 @@ final class NettyNetServerAttributesExtractor
@Override
@Nullable
public String transport(HttpRequestAndChannel requestAndChannel) {
return null;
return requestAndChannel.channel() instanceof DatagramChannel ? IP_UDP : IP_TCP;
}

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

package io.opentelemetry.javaagent.instrumentation.netty.common.server;

import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP;

import io.netty.channel.socket.DatagramChannel;
import io.netty.handler.codec.http.HttpResponse;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
Expand All @@ -18,7 +22,7 @@ final class NettyNetServerAttributesExtractor
@Override
@Nullable
public String transport(HttpRequestAndChannel requestAndChannel) {
return null;
return requestAndChannel.channel() instanceof DatagramChannel ? IP_UDP : IP_TCP;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ 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 play.BuiltInComponents
import play.Mode
import play.mvc.Controller
Expand Down Expand Up @@ -103,9 +102,7 @@ class PlayServerTest extends HttpServerTest<Server> implements AgentTestTrait {

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes
[]
}

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

package io.opentelemetry.instrumentation.ratpack.server

import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.test.InstrumentationSpecification
import io.opentelemetry.instrumentation.test.utils.PortUtils
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
Expand Down Expand Up @@ -81,10 +80,6 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification {

abstract boolean hasHandlerSpan()

List<AttributeKey<?>> extraAttributes() {
[]
}

def "test bindings for #path"() {
when:
def resp = client.get(path).aggregate().join()
Expand All @@ -93,44 +88,26 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification {
resp.status().code() == 200
resp.contentUtf8() == route

def extraAttributes = extraAttributes()

assertTraces(1) {
trace(0, 1 + (hasHandlerSpan() ? 1 : 0)) {
span(0) {
name "/$route"
kind SERVER
hasNoParent()
attributes {
if (extraAttributes.contains(SemanticAttributes.NET_TRANSPORT)) {
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
}
"$SemanticAttributes.NET_TRANSPORT" IP_TCP
// net.peer.name resolves to "127.0.0.1" on windows which is same as net.peer.ip so then not captured
"$SemanticAttributes.NET_PEER_NAME" { it == null || it == "localhost" }
"$SemanticAttributes.NET_PEER_IP" { it == null || it == "127.0.0.1" }
"$SemanticAttributes.NET_PEER_PORT" Long

"$SemanticAttributes.HTTP_METHOD" "GET"
"$SemanticAttributes.HTTP_STATUS_CODE" 200
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String

if (extraAttributes.contains(SemanticAttributes.HTTP_URL)) {
"$SemanticAttributes.HTTP_URL" "http://localhost:${app.bindPort}/${path}"
} else {
"$SemanticAttributes.HTTP_SCHEME" "http"
"$SemanticAttributes.HTTP_HOST" "localhost:${app.bindPort}"
"$SemanticAttributes.HTTP_TARGET" "/$path"
}

if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) {
"$SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH" Long
}
if (extraAttributes.contains(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH)) {
"$SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH" Long
}
if (extraAttributes.contains(SemanticAttributes.HTTP_SERVER_NAME)) {
"$SemanticAttributes.HTTP_SERVER_NAME" String
}
"$SemanticAttributes.HTTP_SCHEME" "http"
"$SemanticAttributes.HTTP_HOST" "localhost:${app.bindPort}"
"$SemanticAttributes.HTTP_TARGET" "/$path"
"$SemanticAttributes.HTTP_ROUTE" "/$route"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@

package io.opentelemetry.instrumentation.ratpack.server

import io.opentelemetry.api.common.AttributeKey
import io.opentelemetry.instrumentation.ratpack.RatpackTracing
import io.opentelemetry.instrumentation.test.LibraryTestTrait
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import ratpack.server.RatpackServerSpec

class RatpackAsyncHttpServerTest extends AbstractRatpackAsyncHttpServerTest implements LibraryTestTrait {
Expand All @@ -26,12 +24,4 @@ class RatpackAsyncHttpServerTest extends AbstractRatpackAsyncHttpServerTest impl
boolean hasHandlerSpan(ServerEndpoint endpoint) {
false
}

@Override
List<AttributeKey<?>> extraAttributes() {
return [
SemanticAttributes.HTTP_ROUTE,
SemanticAttributes.NET_TRANSPORT
]
}
}
Loading