From 1afa86e063d775dc52fdfad53ed2deb17c1e2a80 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Tue, 3 Aug 2021 23:32:14 +0200 Subject: [PATCH] Add OpenFeign support. (#1632) --- CHANGELOG.md | 1 + buildSrc/src/main/java/Config.kt | 2 + sentry-openfeign/api/sentry-openfeign.api | 15 ++ sentry-openfeign/build.gradle.kts | 76 +++++++ .../io/sentry/openfeign/SentryCapability.java | 30 +++ .../sentry/openfeign/SentryFeignClient.java | 119 ++++++++++ .../sentry/openfeign/SentryFeignClientTest.kt | 208 ++++++++++++++++++ .../org.mockito.plugins.MockMaker | 1 + settings.gradle.kts | 1 + 9 files changed, 453 insertions(+) create mode 100644 sentry-openfeign/api/sentry-openfeign.api create mode 100644 sentry-openfeign/build.gradle.kts create mode 100644 sentry-openfeign/src/main/java/io/sentry/openfeign/SentryCapability.java create mode 100644 sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java create mode 100644 sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt create mode 100644 sentry-openfeign/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b78ff0ebf..cdb4c9956e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased * Feat: Spring WebClient integration (#1621) +* Feat: OpenFeign integration (#1632) ## 5.1.0-beta.9 diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 72e8d8d22d..62f76e9006 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -83,6 +83,8 @@ object Config { val fragment = "androidx.fragment:fragment-ktx:1.3.5" val reactorCore = "io.projectreactor:reactor-core:3.4.6" + + val feignCore = "io.github.openfeign:feign-core:11.6" } object AnnotationProcessors { diff --git a/sentry-openfeign/api/sentry-openfeign.api b/sentry-openfeign/api/sentry-openfeign.api new file mode 100644 index 0000000000..f1f64a6994 --- /dev/null +++ b/sentry-openfeign/api/sentry-openfeign.api @@ -0,0 +1,15 @@ +public final class io/sentry/openfeign/SentryCapability : feign/Capability { + public fun ()V + public fun (Lio/sentry/IHub;Lio/sentry/openfeign/SentryFeignClient$BeforeSpanCallback;)V + public fun enrich (Lfeign/Client;)Lfeign/Client; +} + +public final class io/sentry/openfeign/SentryFeignClient : feign/Client { + public fun (Lfeign/Client;Lio/sentry/IHub;Lio/sentry/openfeign/SentryFeignClient$BeforeSpanCallback;)V + public fun execute (Lfeign/Request;Lfeign/Request$Options;)Lfeign/Response; +} + +public abstract interface class io/sentry/openfeign/SentryFeignClient$BeforeSpanCallback { + public abstract fun execute (Lio/sentry/ISpan;Lfeign/Request;Lfeign/Response;)Lio/sentry/ISpan; +} + diff --git a/sentry-openfeign/build.gradle.kts b/sentry-openfeign/build.gradle.kts new file mode 100644 index 0000000000..dc7556e40e --- /dev/null +++ b/sentry-openfeign/build.gradle.kts @@ -0,0 +1,76 @@ +import net.ltgt.gradle.errorprone.errorprone +import org.jetbrains.kotlin.gradle.tasks.KotlinCompile + +plugins { + `java-library` + kotlin("jvm") + jacoco + id(Config.QualityPlugins.errorProne) + id(Config.QualityPlugins.gradleVersions) + id(Config.BuildPlugins.buildConfig) version Config.BuildPlugins.buildConfigVersion +} + +configure { + sourceCompatibility = JavaVersion.VERSION_1_8 + targetCompatibility = JavaVersion.VERSION_1_8 +} + +tasks.withType().configureEach { + kotlinOptions.jvmTarget = JavaVersion.VERSION_1_8.toString() +} + +dependencies { + api(project(":sentry")) + implementation(Config.Libs.feignCore) + + compileOnly(Config.CompileOnly.nopen) + errorprone(Config.CompileOnly.nopenChecker) + errorprone(Config.CompileOnly.errorprone) + errorprone(Config.CompileOnly.errorProneNullAway) + errorproneJavac(Config.CompileOnly.errorProneJavac8) + compileOnly(Config.CompileOnly.jetbrainsAnnotations) + + // tests + testImplementation(project(":sentry-test-support")) + testImplementation(kotlin(Config.kotlinStdLib)) + testImplementation(Config.TestLibs.kotlinTestJunit) + testImplementation(Config.TestLibs.mockitoKotlin) + testImplementation(Config.TestLibs.awaitility) + testImplementation(Config.TestLibs.mockWebserver) +} + +configure { + test { + java.srcDir("src/test/java") + } +} + +jacoco { + toolVersion = Config.QualityPlugins.Jacoco.version +} + +tasks.jacocoTestReport { + reports { + xml.isEnabled = true + html.isEnabled = false + } +} + +tasks { + jacocoTestCoverageVerification { + violationRules { + rule { limit { minimum = Config.QualityPlugins.Jacoco.minimumCoverage } } + } + } + check { + dependsOn(jacocoTestCoverageVerification) + dependsOn(jacocoTestReport) + } +} + +tasks.withType().configureEach { + options.errorprone { + check("NullAway", net.ltgt.gradle.errorprone.CheckSeverity.ERROR) + option("NullAway:AnnotatedPackages", "io.sentry") + } +} diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryCapability.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryCapability.java new file mode 100644 index 0000000000..0466f1a9a0 --- /dev/null +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryCapability.java @@ -0,0 +1,30 @@ +package io.sentry.openfeign; + +import feign.Capability; +import feign.Client; +import io.sentry.HubAdapter; +import io.sentry.IHub; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** Adds Sentry tracing capability to Feign clients. */ +public final class SentryCapability implements Capability { + + private final @NotNull IHub hub; + private final @Nullable SentryFeignClient.BeforeSpanCallback beforeSpan; + + public SentryCapability( + final @NotNull IHub hub, final @Nullable SentryFeignClient.BeforeSpanCallback beforeSpan) { + this.hub = hub; + this.beforeSpan = beforeSpan; + } + + public SentryCapability() { + this(HubAdapter.getInstance(), null); + } + + @Override + public @NotNull Client enrich(final @NotNull Client client) { + return new SentryFeignClient(client, hub, beforeSpan); + } +} diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java new file mode 100644 index 0000000000..09387e566b --- /dev/null +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -0,0 +1,119 @@ +package io.sentry.openfeign; + +import feign.Client; +import feign.Request; +import feign.Response; +import io.sentry.Breadcrumb; +import io.sentry.IHub; +import io.sentry.ISpan; +import io.sentry.SentryTraceHeader; +import io.sentry.SpanStatus; +import io.sentry.util.Objects; +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** A Feign client that creates a span around each executed HTTP call. */ +public final class SentryFeignClient implements Client { + private final @NotNull Client delegate; + private final @NotNull IHub hub; + private final @Nullable BeforeSpanCallback beforeSpan; + + public SentryFeignClient( + final @NotNull Client delegate, + final @NotNull IHub hub, + final @Nullable BeforeSpanCallback beforeSpan) { + this.delegate = Objects.requireNonNull(delegate, "delegate is required"); + this.hub = Objects.requireNonNull(hub, "hub is required"); + this.beforeSpan = beforeSpan; + } + + @Override + public Response execute(final @NotNull Request request, final @NotNull Request.Options options) + throws IOException { + Response response = null; + try { + final ISpan activeSpan = hub.getSpan(); + if (activeSpan == null) { + return delegate.execute(request, options); + } + + ISpan span = activeSpan.startChild("http.client"); + span.setDescription(request.httpMethod().name() + " " + request.url()); + + final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); + final RequestWrapper requestWrapper = new RequestWrapper(request); + requestWrapper.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); + try { + response = delegate.execute(requestWrapper.build(), options); + // handles both success and error responses + span.setStatus(SpanStatus.fromHttpStatusCode(response.status())); + return response; + } catch (Exception e) { + // handles cases like connection errors + span.setThrowable(e); + span.setStatus(SpanStatus.INTERNAL_ERROR); + throw e; + } finally { + if (beforeSpan != null) { + span = beforeSpan.execute(span, request, response); + } + if (span != null) { + span.finish(); + } + } + } finally { + addBreadcrumb(request, response); + } + } + + private void addBreadcrumb(final @NotNull Request request, final @Nullable Response response) { + final Breadcrumb breadcrumb = + Breadcrumb.http( + request.url(), + request.httpMethod().name(), + response != null ? response.status() : null); + breadcrumb.setData("request_body_size", request.body() != null ? request.body().length : 0); + if (response != null && response.body().length() != null) { + breadcrumb.setData("response_body_size", response.body().length()); + } + hub.addBreadcrumb(breadcrumb); + } + + static final class RequestWrapper { + private final @NotNull Request delegate; + + private final @NotNull Map> headers; + + RequestWrapper(final @NotNull Request delegate) { + this.delegate = delegate; + this.headers = new LinkedHashMap<>(delegate.headers()); + } + + public void header(final @NotNull String name, final @NotNull String value) { + if (!headers.containsKey(name)) { + headers.put(name, Collections.singletonList(value)); + } + } + + @NotNull + Request build() { + return Request.create( + delegate.httpMethod(), + delegate.url(), + headers, + delegate.body(), + delegate.charset(), + delegate.requestTemplate()); + } + } + + public interface BeforeSpanCallback { + @Nullable + ISpan execute(@NotNull ISpan span, @NotNull Request request, @Nullable Response response); + } +} diff --git a/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt new file mode 100644 index 0000000000..268ef04d12 --- /dev/null +++ b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt @@ -0,0 +1,208 @@ +package io.sentry.openfeign + +import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.check +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever +import feign.Client +import feign.Feign +import feign.FeignException +import feign.RequestLine +import io.sentry.Breadcrumb +import io.sentry.IHub +import io.sentry.SentryOptions +import io.sentry.SentryTraceHeader +import io.sentry.SentryTracer +import io.sentry.SpanStatus +import io.sentry.TransactionContext +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import kotlin.test.fail +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer + +class SentryFeignClientTest { + + class Fixture { + val hub = mock() + val server = MockWebServer() + val sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) + + init { + whenever(hub.options).thenReturn(SentryOptions()) + } + + fun getSut( + isSpanActive: Boolean = true, + httpStatusCode: Int = 201, + responseBody: String = "success", + networkError: Boolean = false, + beforeSpan: SentryFeignClient.BeforeSpanCallback? = null + ): MockApi { + if (isSpanActive) { + whenever(hub.span).thenReturn(sentryTracer) + } + server.enqueue(MockResponse() + .setBody(responseBody) + .setResponseCode(httpStatusCode)) + server.start() + + return if (!networkError) { + Feign.builder() + .addCapability(SentryCapability(hub, beforeSpan)) + } else { + val mockClient = mock() + whenever(mockClient.execute(any(), any())).thenThrow(RuntimeException::class.java) + Feign.builder() + .client(SentryFeignClient(mockClient, hub, beforeSpan)) + }.target(MockApi::class.java, server.url("/").toUrl().toString()) + } + } + + private val fixture = Fixture() + + @Test + fun `when there is an active span, adds sentry trace header to the request`() { + val sut = fixture.getSut() + sut.getOk() + val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + } + + @Test + fun `when there is no active span, does not add sentry trace header to the request`() { + val sut = fixture.getSut(isSpanActive = false) + sut.getOk() + val recorderRequest = fixture.server.takeRequest() + assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + } + + @Test + fun `does not overwrite response body`() { + val sut = fixture.getSut() + val response = sut.getOk() + assertEquals("success", response) + } + + @Test + fun `creates a span around the request`() { + val sut = fixture.getSut() + sut.getOk() + assertEquals(1, fixture.sentryTracer.children.size) + val httpClientSpan = fixture.sentryTracer.children.first() + assertEquals("http.client", httpClientSpan.operation) + assertEquals("GET ${fixture.server.url("/status/200")}", httpClientSpan.description) + assertEquals(SpanStatus.OK, httpClientSpan.status) + assertTrue(httpClientSpan.isFinished) + } + + @Test + fun `maps http status code to SpanStatus`() { + val sut = fixture.getSut(httpStatusCode = 400) + try { + sut.getOk() + } catch (e: FeignException) { + val httpClientSpan = fixture.sentryTracer.children.first() + assertEquals(SpanStatus.INVALID_ARGUMENT, httpClientSpan.status) + } + } + + @Test + fun `does not map unmapped http status code to SpanStatus`() { + val sut = fixture.getSut(httpStatusCode = 502) + try { + sut.getOk() + } catch (e: FeignException) { + val httpClientSpan = fixture.sentryTracer.children.first() + assertNull(httpClientSpan.status) + } + } + + @Test + fun `adds breadcrumb when http calls succeeds`() { + val sut = fixture.getSut(responseBody = "response body") + sut.postWithBody("request-body") + verify(fixture.hub).addBreadcrumb(check { + assertEquals("http", it.type) + assertEquals(13, it.data["response_body_size"]) + assertEquals(12, it.data["request_body_size"]) + }) + } + + @SuppressWarnings("SwallowedException") + @Test + fun `adds breadcrumb when http calls results in exception`() { + val sut = fixture.getSut(networkError = true) + + try { + sut.getOk() + fail() + } catch (e: Exception) { + // ignore me + } + verify(fixture.hub).addBreadcrumb(check { + assertEquals("http", it.type) + }) + } + + @SuppressWarnings("SwallowedException") + @Test + fun `sets status and throwable when call results in in exception`() { + val sut = fixture.getSut(networkError = true) + try { + sut.getOk() + fail() + } catch (e: Exception) { + // ignore + } + val httpClientSpan = fixture.sentryTracer.children.first() + assertEquals(SpanStatus.INTERNAL_ERROR, httpClientSpan.status) + assertTrue(httpClientSpan.throwable is Exception) + } + + @Test + fun `customizer modifies span`() { + val sut = fixture.getSut { span, _, _ -> + span.description = "overwritten description" + span + } + sut.getOk() + assertEquals(1, fixture.sentryTracer.children.size) + val httpClientSpan = fixture.sentryTracer.children.first() + assertEquals("overwritten description", httpClientSpan.description) + } + + @Test + fun `customizer receives request and response`() { + val sut = fixture.getSut { span, request, response -> + assertEquals(request.url(), request.url()) + assertEquals(request.httpMethod().name, request.httpMethod().name) + assertNotNull(response) { + assertEquals(201, it.status()) + } + span + } + sut.getOk() + } + + @Test + fun `customizer can drop the span`() { + val sut = fixture.getSut { _, _, _ -> null } + sut.getOk() + val httpClientSpan = fixture.sentryTracer.children.first() + assertFalse(httpClientSpan.isFinished) + } + + interface MockApi { + @RequestLine("GET /status/200") + fun getOk(): String + + @RequestLine("POST /post-with-body") + fun postWithBody(body: String) + } +} diff --git a/sentry-openfeign/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/sentry-openfeign/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 0000000000..1f0955d450 --- /dev/null +++ b/sentry-openfeign/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline diff --git a/settings.gradle.kts b/settings.gradle.kts index d6fe87420f..cb292307bb 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -19,6 +19,7 @@ include( "sentry-spring", "sentry-spring-boot-starter", "sentry-bom", + "sentry-openfeign", "sentry-samples:sentry-samples-android", "sentry-samples:sentry-samples-console", "sentry-samples:sentry-samples-jul",