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.

Sign up for GitHub

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

Revert "Implement Armeria instrumentation context propagation." #1078

Merged
merged 1 commit into from
Aug 21, 2020
Merged

Revert "Implement Armeria instrumentation context propagation." #1078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 2 additions & 11 deletions ...ntelemetry/instrumentation/auto/armeria/v1_0/ArmeriaNoDuplicateInstrumentationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,10 @@ class ArmeriaNoDuplicateInstrumentationTest extends AbstractArmeriaTest implemen
}

def childSetupSpec() {
backend.before()
frontend.before()
server.before()
}

def childCleanupSpec() {
backend.after()
frontend.after()
}

// Because our unit tests include io.grpc.Context in the bootstrap classloader, there does not
// seem to be a simple way of testing this yet.
@Override
boolean supportsContext() {
return false
server.after()
}
}
13 changes: 2 additions & 11 deletions ...uto/src/test/groovy/io/opentelemetry/instrumentation/auto/armeria/v1_0/ArmeriaTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,10 @@ class ArmeriaTest extends AbstractArmeriaTest implements AgentTestTrait {
}

def childSetupSpec() {
backend.before()
frontend.before()
server.before()
}

def childCleanupSpec() {
backend.after()
frontend.after()
}

// Because our unit tests include io.grpc.Context in the bootstrap classloader, there does not
// seem to be a simple way of testing this yet.
@Override
boolean supportsContext() {
return false
server.after()
}
}
44 changes: 0 additions & 44 deletions ...umentation/armeria-1.0/library/src/main/java/io/grpc/override/ContextStorageOverride.java
View file
Open in desktop

This file was deleted.

106 changes: 0 additions & 106 deletions ...a/io/opentelemetry/instrumentation/armeria/v1_0/common/ArmeriaContextStorageOverride.java
View file
Open in desktop

This file was deleted.

6 changes: 2 additions & 4 deletions .../library/src/test/groovy/io/opentelemetry/instrumentation/armeria/v1_0/ArmeriaTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@ class ArmeriaTest extends AbstractArmeriaTest implements InstrumentationTestTrai
}

def childSetupSpec() {
backend.before()
frontend.before()
server.before()
}

def cleanupSpec() {
backend.after()
frontend.after()
server.after()
}
}
137 changes: 4 additions & 133 deletions .../src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_0/AbstractArmeriaTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package io.opentelemetry.instrumentation.armeria.v1_0

import static io.opentelemetry.trace.Span.Kind.CLIENT
import static io.opentelemetry.trace.Span.Kind.SERVER
import static org.junit.Assume.assumeTrue

import com.linecorp.armeria.client.WebClient
import com.linecorp.armeria.client.WebClientBuilder
Expand All @@ -30,7 +29,6 @@ import com.linecorp.armeria.server.ServerBuilder
import com.linecorp.armeria.testing.junit4.server.ServerRule
import io.opentelemetry.auto.test.InstrumentationSpecification
import io.opentelemetry.trace.attributes.SemanticAttributes
import java.util.concurrent.CompletableFuture
import spock.lang.Shared
import spock.lang.Unroll

Expand All @@ -44,41 +42,18 @@ abstract class AbstractArmeriaTest extends InstrumentationSpecification {
// We cannot annotate with @ClassRule since then Armeria will be class loaded before bytecode
// instrumentation is set up by the Spock trait.
@Shared
protected ServerRule backend = new ServerRule() {
protected ServerRule server = new ServerRule() {
@Override
protected void configure(ServerBuilder sb) throws Exception {
sb = configureServer(sb)
sb.service("/hello", { ctx, req -> HttpResponse.of(HttpStatus.OK) })
}
}

// We cannot annotate with @ClassRule since then Armeria will be class loaded before bytecode
// instrumentation is set up by the Spock trait.
@Shared
protected ServerRule frontend = new ServerRule() {
@Override
protected void configure(ServerBuilder sb) throws Exception {
sb = configureServer(sb)
def backendClient = configureClient(WebClient.builder(backend.httpUri())).build()

sb.service("/exact", { ctx, req -> HttpResponse.of(HttpStatus.OK) })
sb.service("/items/{itemsId}", { ctx, req -> HttpResponse.of(HttpStatus.OK) })
sb.service("/httperror", { ctx, req -> HttpResponse.of(HttpStatus.NOT_IMPLEMENTED) })
sb.service("/exception", { ctx, req -> throw new IllegalStateException("illegal") })
sb.service("/async", { ctx, req ->
def executor = ctx.eventLoop()
CompletableFuture<HttpResponse> resp = backendClient.get("/hello").aggregate(executor)
.thenComposeAsync({ unused ->
backendClient.get("/hello").aggregate()
}, executor)
.thenApplyAsync({ unused -> HttpResponse.of(HttpStatus.OK)}, executor)

return HttpResponse.from(resp)
})
}
}

def client = configureClient(WebClient.builder(frontend.httpUri())).build()
def client = configureClient(WebClient.builder(server.httpUri())).build()

def "HTTP #method #path"() {
when:
Expand All @@ -98,7 +73,7 @@ abstract class AbstractArmeriaTest extends InstrumentationSpecification {
// TODO(anuraaga): peer name shouldn't be set to IP
"${SemanticAttributes.NET_PEER_NAME.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${frontend.httpUri()}${path}"
"${SemanticAttributes.HTTP_URL.key()}" "${server.httpUri()}${path}"
"${SemanticAttributes.HTTP_METHOD.key()}" method.name()
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" code
}
Expand All @@ -114,7 +89,7 @@ abstract class AbstractArmeriaTest extends InstrumentationSpecification {
attributes {
"${SemanticAttributes.NET_PEER_IP.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${frontend.httpUri()}${path}"
"${SemanticAttributes.HTTP_URL.key()}" "${server.httpUri()}${path}"
"${SemanticAttributes.HTTP_METHOD.key()}" method.name()
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" code
"${SemanticAttributes.HTTP_FLAVOR.key()}" "h2c"
Expand All @@ -133,108 +108,4 @@ abstract class AbstractArmeriaTest extends InstrumentationSpecification {
"/httperror" | "/httperror" | HttpMethod.GET | 501
"/exception" | "/exception" | HttpMethod.GET | 500
}

def "context propagated by executor"() {
when:
assumeTrue(supportsContext())
def response = client.get("/async").aggregate().join()

then:
response.status().code() == 200
assertTraces(1) {
trace(0, 6) {
span(0) {
operationName("HTTP GET")
spanKind CLIENT
parent()
attributes {
"${SemanticAttributes.NET_PEER_IP.key()}" "127.0.0.1"
// TODO(anuraaga): peer name shouldn't be set to IP
"${SemanticAttributes.NET_PEER_NAME.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${frontend.httpUri()}/async"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
}
}
span(1) {
operationName("/async")
spanKind SERVER
childOf span(0)
attributes {
"${SemanticAttributes.NET_PEER_IP.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${frontend.httpUri()}/async"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_FLAVOR.key()}" "h2c"
"${SemanticAttributes.HTTP_USER_AGENT.key()}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
}
}
span(2) {
operationName("HTTP GET")
spanKind CLIENT
childOf span(1)
attributes {
"${SemanticAttributes.NET_PEER_IP.key()}" "127.0.0.1"
// TODO(anuraaga): peer name shouldn't be set to IP
"${SemanticAttributes.NET_PEER_NAME.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${backend.httpUri()}/hello"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
}
}
span(3) {
operationName("/hello")
spanKind SERVER
childOf span(2)
attributes {
"${SemanticAttributes.NET_PEER_IP.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${backend.httpUri()}/hello"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_FLAVOR.key()}" "h2c"
"${SemanticAttributes.HTTP_USER_AGENT.key()}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
}
}
span(4) {
operationName("HTTP GET")
spanKind CLIENT
childOf span(1)
attributes {
"${SemanticAttributes.NET_PEER_IP.key()}" "127.0.0.1"
// TODO(anuraaga): peer name shouldn't be set to IP
"${SemanticAttributes.NET_PEER_NAME.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${backend.httpUri()}/hello"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
}
}
span(5) {
operationName("/hello")
spanKind SERVER
childOf span(4)
attributes {
"${SemanticAttributes.NET_PEER_IP.key()}" "127.0.0.1"
"${SemanticAttributes.NET_PEER_PORT.key()}" Long
"${SemanticAttributes.HTTP_URL.key()}" "${backend.httpUri()}/hello"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_FLAVOR.key()}" "h2c"
"${SemanticAttributes.HTTP_USER_AGENT.key()}" String
"${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
}
}
}
}
}

boolean supportsContext() {
return true
}
}