Skip to content

Commit

Permalink
Update more tests affected by tracing agent
Browse files Browse the repository at this point in the history
The responses for these tests could change if java agent is running

GitOrigin-RevId: 37f654960b5a31ae0b3a9fb9dc7b0a8f080278c1
  • Loading branch information
rainecp authored and svc-squareup-copybara committed Jul 22, 2024
1 parent c4105aa commit 9a9c0b4
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal class ClientLoggingInterceptorTest {
assertThat(client.ping("test", AppRequest(200)).execute().code()).isEqualTo(200)

assertThat(logCollector.takeMessage(ClientLoggingInterceptor::class))
.matches("Outgoing request: .*, headers=\\{X-b3-traceid=test}")
.matches("Outgoing request: .*, headers=\\{X-b3-traceid=test.*")
}

class TestModule : KAbstractModule() {
Expand Down
9 changes: 6 additions & 3 deletions misk/src/test/kotlin/misk/web/JettyServiceMetricsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ internal class JettyServiceMetricsTest {
val labels = ConnectionMetrics.forPort("http", 0) // It's the configured port not the actual
assertThat(connectionMetrics.acceptedConnections.labels(*labels).get()).isEqualTo(1.0)
assertThat(connectionMetrics.activeConnections.labels(*labels).get()).isEqualTo(1.0)
assertThat(connectionMetrics.bytesReceived.labels(*labels).get()).isEqualTo(130.0)
// There could be additional bytesReceived (e.g. if tests are run with tracing or java agent)
assertThat(connectionMetrics.bytesReceived.labels(*labels).get()).isGreaterThanOrEqualTo(130.0)
assertThat(connectionMetrics.bytesSent.labels(*labels).get()).isEqualTo(141.0)
assertThat(connectionMetrics.messagesReceived.labels(*labels).get()).isEqualTo(1.0)
assertThat(connectionMetrics.messagesSent.labels(*labels).get()).isEqualTo(1.0)
Expand All @@ -73,7 +74,8 @@ internal class JettyServiceMetricsTest {
// Active connections should have dropped to zero, all other metrics should remain the same
assertThat(connectionMetrics.acceptedConnections.labels(*labels).get()).isEqualTo(1.0)
assertThat(connectionMetrics.activeConnections.labels(*labels).get()).isEqualTo(0.0)
assertThat(connectionMetrics.bytesReceived.labels(*labels).get()).isEqualTo(130.0)
// There could be additional bytesReceived (e.g. if tests are run with tracing or java agent)
assertThat(connectionMetrics.bytesReceived.labels(*labels).get()).isGreaterThanOrEqualTo(130.0)
assertThat(connectionMetrics.bytesSent.labels(*labels).get()).isEqualTo(141.0)
assertThat(connectionMetrics.messagesReceived.labels(*labels).get()).isEqualTo(1.0)
assertThat(connectionMetrics.messagesSent.labels(*labels).get()).isEqualTo(1.0)
Expand All @@ -82,7 +84,8 @@ internal class JettyServiceMetricsTest {
connectionMetricsCollector.refreshMetrics()
assertThat(connectionMetrics.acceptedConnections.labels(*labels).get()).isEqualTo(1.0)
assertThat(connectionMetrics.activeConnections.labels(*labels).get()).isEqualTo(0.0)
assertThat(connectionMetrics.bytesReceived.labels(*labels).get()).isEqualTo(130.0)
// There could be additional bytesReceived (e.g. if tests are run with tracing or java agent)
assertThat(connectionMetrics.bytesReceived.labels(*labels).get()).isGreaterThanOrEqualTo(130.0)
assertThat(connectionMetrics.bytesSent.labels(*labels).get()).isEqualTo(141.0)
assertThat(connectionMetrics.messagesReceived.labels(*labels).get()).isEqualTo(1.0)
assertThat(connectionMetrics.messagesSent.labels(*labels).get()).isEqualTo(1.0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.junit.jupiter.api.Test
import wisp.logging.LogCollector
import java.util.concurrent.TimeUnit
import jakarta.inject.Inject
import kotlin.test.assertEquals

@MiskTest(startService = true)
internal class RequestLoggingInterceptorTest {
Expand Down Expand Up @@ -60,11 +61,13 @@ internal class RequestLoggingInterceptorTest {
"caller"
).isSuccessful
).isTrue()
assertThat(logCollector.takeMessages(RequestLoggingInterceptor::class)).containsExactly(
"RateLimitingIncludesBodyRequestLoggingAction principal=caller time=100.0 ms code=200 " +
"request=[hello] requestHeaders={accept-encoding=[gzip], connection=[keep-alive]} " +
"response=echo: hello responseHeaders={}"
var messages = logCollector.takeMessages(RequestLoggingInterceptor::class)
assertEquals(1, messages.size)
assertThat(messages[0]).contains(
"RateLimitingIncludesBodyRequestLoggingAction principal=caller time=100.0 ms code=200 " +
"request=[hello] requestHeaders={accept-encoding=[gzip], connection=[keep-alive]"
)
assertThat(messages[0]).contains("response=echo: hello responseHeaders={}")

// Setting to low value to show that even though it is less than the bodySampling value in the
// LogRequestResponse, because the LogRateLimiter does not acquire a bucket, the request and
Expand All @@ -87,13 +90,14 @@ internal class RequestLoggingInterceptorTest {
"caller"
).isSuccessful
).isTrue()
assertThat(logCollector.takeMessages(RequestLoggingInterceptor::class)).containsExactly(
messages = logCollector.takeMessages(RequestLoggingInterceptor::class)
assertEquals(1, messages.size)
assertThat(messages[0]).contains(
"RateLimitingIncludesBodyRequestLoggingAction principal=caller time=100.0 ms " +
"code=200 request=[hello3] " +
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]} " +
"response=echo: hello3 " +
"responseHeaders={}"
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]"
)
assertThat(messages[0]).contains("response=echo: hello3 responseHeaders={}")

fakeTicker.advance(1, TimeUnit.SECONDS)

Expand Down Expand Up @@ -146,10 +150,12 @@ internal class RequestLoggingInterceptorTest {
assertThat(invoke("/call/exceptionThrowingRequestLogging/fail", "caller").code)
.isEqualTo(500)
val messages = logCollector.takeMessages(RequestLoggingInterceptor::class)
assertThat(messages).containsExactly(
assertEquals(1, messages.size)
// There could be additional headers (e.g. if tests are run with tracing or java agent)
assertThat(messages[0]).contains(
"ExceptionThrowingRequestLoggingAction principal=caller time=100.0 ms failed " +
"request=[fail] " +
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]}"
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]"
)
}

Expand Down Expand Up @@ -216,13 +222,14 @@ internal class RequestLoggingInterceptorTest {
)
.isTrue()
val messages = logCollector.takeMessages(RequestLoggingInterceptor::class)
assertThat(messages).containsExactly(
assertEquals(1, messages.size)
assertThat(messages[0]).contains(
"RequestLoggingActionWithHeaders principal=unknown time=100.0 ms code=200 " +
"request=[hello] " +
"requestHeaders={accept=[*/*], accept-encoding=[gzip], connection=[keep-alive], " +
"content-length=[5], content-type=[application/json;charset=UTF-8]} " +
"response=echo: hello responseHeaders={}"
"content-length=[5], content-type=[application/json;charset=UTF-8]"
)
assertThat(messages[0]).contains("response=echo: hello responseHeaders={}")
assertThat(messages[0]).doesNotContain(headerToNotLog)
assertThat(messages[0]).doesNotContain(headerToNotLog.lowercase())
assertThat(messages[0]).doesNotContain(headerValueToNotLog)
Expand All @@ -240,10 +247,11 @@ internal class RequestLoggingInterceptorTest {
)
.isFalse()
val messages = logCollector.takeMessages(RequestLoggingInterceptor::class)
assertThat(messages).containsExactly(
assertEquals(1, messages.size)
assertThat(messages[0]).contains(
"RequestLoggingActionWithHeaders principal=unknown time=100.0 ms failed request=[fail] " +
"requestHeaders={accept=[*/*], accept-encoding=[gzip], connection=[keep-alive], " +
"content-length=[4], content-type=[application/json;charset=UTF-8]}"
"content-length=[4], content-type=[application/json;charset=UTF-8]"
)
assertThat(messages[0]).doesNotContain(headerToNotLog)
assertThat(messages[0]).doesNotContain(headerToNotLog.lowercase())
Expand All @@ -258,11 +266,12 @@ internal class RequestLoggingInterceptorTest {

// Even though this endpoint is configured with low rate limiting and body sampling in its annotation,
// the RequestLoggingConfig injected will override it to no rate limiting and 1.0 sampling
assertThat(messages).containsExactly(
assertEquals(1, messages.size)
assertThat(messages[0]).contains(
"ConfigOverrideAction principal=caller time=100.0 ms code=200 request=[foo] " +
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]} " +
"response=echo: foo responseHeaders={}"
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]"
)
assertThat(messages[0]).contains("response=echo: foo responseHeaders={}")
}
}

Expand All @@ -273,10 +282,13 @@ internal class RequestLoggingInterceptorTest {

// Note that the [DontSayDumbTransformer] is registered earlier and thus ran _before_ the
// [EvanHatingTransformer] which added "dumb" to the response, so it doesn't get applied here.
assertThat(messages).containsExactly(
assertEquals(1, messages.size)
assertThat(messages[0]).contains(
"LogEverythingAction principal=caller time=100.0 ms code=200 request=[Quokka] " +
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]} " +
"response=echo: Quokka (the happiest, most bestest animal) responseHeaders={}"
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]"
)
assertThat(messages[0]).contains(
"response=echo: Quokka (the happiest, most bestest animal) responseHeaders={}"
)
}

Expand All @@ -293,10 +305,13 @@ internal class RequestLoggingInterceptorTest {
val interceptorLogs = events
.filter { it.loggerName == RequestLoggingInterceptor::class.qualifiedName }
.map { it.message }
assertThat(interceptorLogs).containsExactly(
assertEquals(1, interceptorLogs.size)
assertThat(interceptorLogs[0]).contains(
"LogEverythingAction principal=caller time=100.0 ms code=200 " +
"request=[Oppenheimer-the-bestest] " +
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]} " +
"requestHeaders={accept-encoding=[gzip], connection=[keep-alive]"
)
assertThat(interceptorLogs[0]).contains(
"response=echo: Oppenheimer-the-most bestest responseHeaders={}"
)
}
Expand Down

0 comments on commit 9a9c0b4

Please sign in to comment.