diff --git a/build.gradle.kts b/build.gradle.kts index 9183d70..a267cbd 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -39,6 +39,10 @@ dependencies { implementation(libs.ktor.server.content.negotiation) implementation(libs.ktor.server.logging) + // Metrics + implementation(libs.ktor.server.metrics.micrometer) + implementation(libs.micrometer.registry.prometheus) + // Serialization implementation(libs.ktor.server.serialization) implementation(libs.kotlinx.serialization.json) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index cc7f94e..af4a8c9 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -12,6 +12,7 @@ wire-sdk-version = "0.2.1" redis-version = "6.7.1.RELEASE" mustache-version = "0.9.14" mockk-version = "1.14.11" +micrometer-version = "1.16.0" shadow = "9.6.1" [plugins] @@ -29,6 +30,9 @@ ktor-server-test-host = { module = "io.ktor:ktor-server-test-host", version.ref ktor-server-content-negotiation = { module = "io.ktor:ktor-server-content-negotiation", version.ref = "ktor-version" } ktor-server-serialization = { module = "io.ktor:ktor-serialization-kotlinx-json", version.ref = "ktor-version" } ktor-server-logging = { module = "io.ktor:ktor-client-logging", version.ref = "ktor-version" } +# Pinned to the version koin-ktor already forces onto ktor-server-core; 3.2.3 is binary incompatible with it. +ktor-server-metrics-micrometer = { module = "io.ktor:ktor-server-metrics-micrometer", version = "3.4.0" } +micrometer-registry-prometheus = { module = "io.micrometer:micrometer-registry-prometheus", version.ref = "micrometer-version" } kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlin-serialization" } koin-ktor = { module = "io.insert-koin:koin-ktor", version.ref = "koin-version" } koin-test = { module = "io.insert-koin:koin-test", version.ref = "koin-version" } diff --git a/src/main/kotlin/com/wire/github/Application.kt b/src/main/kotlin/com/wire/github/Application.kt index 134af68..41a1e73 100644 --- a/src/main/kotlin/com/wire/github/Application.kt +++ b/src/main/kotlin/com/wire/github/Application.kt @@ -1,6 +1,7 @@ package com.wire.github import com.wire.github.config.projectModules +import com.wire.github.metrics.configureMetrics import com.wire.github.util.ENV_VAR_PORT import io.ktor.server.application.Application import io.ktor.server.engine.embeddedServer @@ -22,4 +23,5 @@ fun main() { fun Application.module() { configureRouting() + configureMetrics() } diff --git a/src/main/kotlin/com/wire/github/EventsHandler.kt b/src/main/kotlin/com/wire/github/EventsHandler.kt index 60b6703..55fcf26 100644 --- a/src/main/kotlin/com/wire/github/EventsHandler.kt +++ b/src/main/kotlin/com/wire/github/EventsHandler.kt @@ -1,5 +1,6 @@ package com.wire.github +import com.wire.github.metrics.UsageMetrics import com.wire.github.util.ENV_VAR_HOST import com.wire.github.util.SessionIdentifierGenerator import com.wire.github.util.toStorageKey @@ -16,6 +17,7 @@ class EventsHandler : WireEventsHandlerSuspending() { private val logger = LoggerFactory.getLogger(this::class.java) private val redisConnection = GlobalContext.get().get>() private val storage = redisConnection.sync() + private val usageMetrics = GlobalContext.get().get() override suspend fun onTextMessageReceived(wireMessage: WireMessage.Text) { if (wireMessage.text.equals(HELP_COMMAND, ignoreCase = true)) { @@ -24,6 +26,7 @@ class EventsHandler : WireEventsHandlerSuspending() { "conversationId: ${wireMessage.conversationId}, " + "senderId: ${wireMessage.sender}" ) + usageMetrics.onHelpCommand() val message = formatSetupInstructions( conversationId = wireMessage.conversationId, secret = storage.get(wireMessage.conversationId.toStorageKey()) @@ -50,6 +53,7 @@ class EventsHandler : WireEventsHandlerSuspending() { "Event received. Event: AppAddedToConversation, " + "conversationId: ${conversation.id}" ) + usageMetrics.onAppAddedToConversation() val message = buildString { appendLine(WELCOME_TEXT) appendLine(formatSetupInstructions(conversationId = conversation.id)) diff --git a/src/main/kotlin/com/wire/github/Routing.kt b/src/main/kotlin/com/wire/github/Routing.kt index efa1a57..0f4075e 100644 --- a/src/main/kotlin/com/wire/github/Routing.kt +++ b/src/main/kotlin/com/wire/github/Routing.kt @@ -1,5 +1,6 @@ package com.wire.github +import com.wire.github.metrics.UsageMetrics import com.wire.github.response.model.GitHubResponse import com.wire.github.util.KtxSerializer import com.wire.github.util.SignatureValidator @@ -20,6 +21,7 @@ import io.ktor.server.routing.application import io.ktor.server.routing.get import io.ktor.server.routing.post import io.ktor.server.routing.routing +import java.io.IOException import java.util.UUID import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.SerializationException @@ -35,6 +37,7 @@ fun Application.configureRouting() { val wireAppSdk = GlobalContext.get().get() val signatureValidator = GlobalContext.get().get() val templateHandler = GlobalContext.get().get() + val usageMetrics = GlobalContext.get().get() routing { trace { @@ -72,19 +75,40 @@ fun Application.configureRouting() { val payload = call.receiveText() // Validation of received signature - val isSignatureValid = signatureValidator.isValid( - conversationId = conversationId, - conversationDomain = conversationDomain, - signature = signature, - payload = payload - ) + val isSignatureValid = try { + signatureValidator.isValid( + conversationId = conversationId, + conversationDomain = conversationDomain, + signature = signature, + payload = payload + ) + } catch (exception: IOException) { + application.log.warn( + "No secret stored for conversation $conversationId@$conversationDomain, " + + "rejecting $event delivery $delivery", + exception + ) + + // A missing secret can never validate on retry, so this is a permanent + // rejection (403) rather than a server error (500) GitHub would redeliver. + return@post call.respond( + status = HttpStatusCode.Forbidden, + message = "Invalid Signature for Conversation" + ) + } if (!isSignatureValid) { + application.log.warn( + "Invalid signature for conversation $conversationId@$conversationDomain, " + + "rejecting $event delivery $delivery" + ) return@post call.respond( status = HttpStatusCode.Forbidden, message = "Invalid Signature for Conversation" ) } + usageMetrics.onWebhookEventReceived(event = event) + val response = try { KtxSerializer.json.decodeFromString(payload) } catch (exception: SerializationException) { @@ -98,18 +122,25 @@ fun Application.configureRouting() { response = response ) - messageTemplate?.let { message -> - wireAppSdk.getApplicationManager().sendMessage( - message = WireMessage.Text.create( - conversationId = QualifiedId( - id = UUID.fromString(conversationId), - domain = conversationDomain - ), - text = message - ) + if (messageTemplate == null) { + usageMetrics.onUnsupportedEvent( + event = event, + action = response.action ) + return@post call.response.status(HttpStatusCode.OK) } + wireAppSdk.getApplicationManager().sendMessage( + message = WireMessage.Text.create( + conversationId = QualifiedId( + id = UUID.fromString(conversationId), + domain = conversationDomain + ), + text = messageTemplate + ) + ) + usageMetrics.onNotificationSent(event = event) + return@post call.response.status(HttpStatusCode.OK) } } diff --git a/src/main/kotlin/com/wire/github/config/Modules.kt b/src/main/kotlin/com/wire/github/config/Modules.kt index ea20554..de9b29b 100644 --- a/src/main/kotlin/com/wire/github/config/Modules.kt +++ b/src/main/kotlin/com/wire/github/config/Modules.kt @@ -1,6 +1,7 @@ package com.wire.github.config import com.wire.github.EventsHandler +import com.wire.github.metrics.UsageMetrics import com.wire.github.util.ENV_VAR_API_HOST import com.wire.github.util.ENV_VAR_API_TOKEN import com.wire.github.util.ENV_VAR_APPLICATION_ID @@ -12,6 +13,8 @@ import com.wire.sdk.WireAppSdk import io.ktor.utils.io.core.toByteArray import io.lettuce.core.RedisClient import io.lettuce.core.api.StatefulRedisConnection +import io.micrometer.prometheusmetrics.PrometheusConfig +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry import org.koin.dsl.module val projectModules = module { @@ -24,6 +27,8 @@ val projectModules = module { single { TemplateHandler() } single { RedisClient.create(ENV_VAR_REDIS_URL) } single> { get().connect() } + single { PrometheusMeterRegistry(PrometheusConfig.DEFAULT) } + single { UsageMetrics(registry = get()) } } private fun wireAppSdk(): WireAppSdk = diff --git a/src/main/kotlin/com/wire/github/metrics/Metrics.kt b/src/main/kotlin/com/wire/github/metrics/Metrics.kt new file mode 100644 index 0000000..3c3d12c --- /dev/null +++ b/src/main/kotlin/com/wire/github/metrics/Metrics.kt @@ -0,0 +1,24 @@ +package com.wire.github.metrics + +import io.ktor.server.application.Application +import io.ktor.server.application.install +import io.ktor.server.metrics.micrometer.MicrometerMetrics +import io.ktor.server.response.respond +import io.ktor.server.routing.get +import io.ktor.server.routing.routing +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry +import org.koin.core.context.GlobalContext + +fun Application.configureMetrics() { + val prometheusRegistry = GlobalContext.get().get() + + install(plugin = MicrometerMetrics) { + registry = prometheusRegistry + } + + routing { + get("/metrics") { + call.respond(prometheusRegistry.scrape()) + } + } +} diff --git a/src/main/kotlin/com/wire/github/metrics/UsageMetrics.kt b/src/main/kotlin/com/wire/github/metrics/UsageMetrics.kt new file mode 100644 index 0000000..cff3a0e --- /dev/null +++ b/src/main/kotlin/com/wire/github/metrics/UsageMetrics.kt @@ -0,0 +1,92 @@ +package com.wire.github.metrics + +import io.micrometer.core.instrument.Counter +import io.micrometer.core.instrument.MeterRegistry + +class UsageMetrics( + private val registry: MeterRegistry +) { + private val helpCommandCounter: Counter = Counter + .builder("githubapp_help_commands_total") + .description("Number of Help commands received") + .register(registry) + + private val appAddedToConversationCounter: Counter = Counter + .builder("githubapp_added_to_conversation_total") + .description("Number of times the app is added to a conversation") + .register(registry) + + private fun webhookEventsReceivedCounter(event: String): Counter = + Counter + .builder("githubapp_webhook_events_received_total") + .description("Number of GitHub webhook deliveries accepted for processing") + .tag(TAG_EVENT, event) + .register(registry) + + private fun notificationsSentCounter(event: String): Counter = + Counter + .builder("githubapp_notifications_sent_total") + .description("Number of messages sent to a conversation for a webhook delivery") + .tag(TAG_EVENT, event) + .register(registry) + + private fun unsupportedEventsCounter( + event: String, + action: String? + ): Counter = + Counter + .builder("githubapp_unsupported_events_total") + .description("Number of webhook deliveries with no matching message template") + .tag(TAG_EVENT, event) + .tag(TAG_ACTION, action ?: NO_ACTION) + .register(registry) + + /** + * A help command was received in a conversation. + * Signals how often users come back for the setup instructions. + */ + fun onHelpCommand() { + helpCommandCounter.increment() + } + + /** + * The app was added to a conversation. + * First step of onboarding, before any webhook is configured. + */ + fun onAppAddedToConversation() { + appAddedToConversationCounter.increment() + } + + /** + * A webhook delivery passed signature validation and entered processing. + * Top of the delivery funnel. + */ + fun onWebhookEventReceived(event: String) { + webhookEventsReceivedCounter(event).increment() + } + + /** + * A webhook delivery was rendered and sent to the conversation. + * Bottom of the delivery funnel. + */ + fun onNotificationSent(event: String) { + notificationsSentCounter(event).increment() + } + + /** + * A webhook delivery had no matching message template, so nothing was sent. + * Ranks the event/action pairs worth adding templates for. + */ + fun onUnsupportedEvent( + event: String, + action: String? + ) { + unsupportedEventsCounter(event, action).increment() + } + + private companion object { + const val TAG_EVENT = "event" + const val TAG_ACTION = "action" + const val NO_ACTION = "none" + } +} diff --git a/src/test/kotlin/com/wire/github/ApplicationTest.kt b/src/test/kotlin/com/wire/github/ApplicationTest.kt index bb06502..9e7d198 100644 --- a/src/test/kotlin/com/wire/github/ApplicationTest.kt +++ b/src/test/kotlin/com/wire/github/ApplicationTest.kt @@ -1,5 +1,6 @@ package com.wire.github +import com.wire.github.metrics.UsageMetrics import com.wire.github.util.SignatureValidator import com.wire.github.util.TemplateHandler import com.wire.sdk.WireAppSdk @@ -10,19 +11,24 @@ import io.ktor.client.request.setBody import io.ktor.http.ContentType import io.ktor.http.HttpStatusCode import io.ktor.http.contentType +import io.ktor.client.statement.bodyAsText import io.ktor.server.testing.testApplication import kotlin.test.Test +import kotlin.test.assertTrue import org.junit.jupiter.api.Assertions.assertEquals import io.ktor.client.request.header import io.lettuce.core.RedisClient import io.lettuce.core.api.StatefulRedisConnection import io.lettuce.core.api.sync.RedisCommands +import io.micrometer.prometheusmetrics.PrometheusConfig +import io.micrometer.prometheusmetrics.PrometheusMeterRegistry import io.mockk.every import io.mockk.mockk import io.mockk.verify import java.util.UUID import kotlin.test.AfterTest import kotlin.test.BeforeTest +import org.koin.core.context.GlobalContext import org.koin.core.context.loadKoinModules import org.koin.core.context.startKoin import org.koin.core.context.stopKoin @@ -47,6 +53,8 @@ class ApplicationTest { single { mockRedisClient } single> { mockRedisConnection } single { mockWireAppSdk } + single { PrometheusMeterRegistry(PrometheusConfig.DEFAULT) } + single { UsageMetrics(registry = get()) } } ) } @@ -68,6 +76,146 @@ class ApplicationTest { assertEquals(HttpStatusCode.OK, response.status) } + @Test + fun `given app is added to a conversation, when GET metrics, then counter is exposed`() = + testApplication { + application { + module() + } + GlobalContext.get().get().onAppAddedToConversation() + + val response = client.get("/metrics") + + assertEquals(HttpStatusCode.OK, response.status) + assertTrue( + response.bodyAsText().contains("githubapp_added_to_conversation_total 1.0") + ) + } + + @Test + fun `given help command is received, when GET metrics, then counter is exposed`() = + testApplication { + application { + module() + } + val usageMetrics = GlobalContext.get().get() + usageMetrics.onHelpCommand() + usageMetrics.onHelpCommand() + + val response = client.get("/metrics") + + assertEquals(HttpStatusCode.OK, response.status) + assertTrue( + response.bodyAsText().contains("githubapp_help_commands_total 2.0") + ) + } + + @Test + fun `given a delivery is rendered, when GET metrics, then funnel counters are set`() { + val signatureValidator = mockk() + every { signatureValidator.isValid(any(), any(), any(), any()) } returns true + + val wireAppSdk = mockk() + every { + wireAppSdk.getApplicationManager().sendMessage(message = any()) + } returns UUID.randomUUID() + + val templateHandler = mockk() + every { + templateHandler.handleEvent(event = any(), response = any()) + } returns DUMMY_TEMPLATE + + loadKoinModules( + module { + single { signatureValidator } + single { wireAppSdk } + single { templateHandler } + } + ) + + testApplication { + application { + module() + } + + client.post("/${CONVERSATION_ID.id}/${CONVERSATION_ID.domain}") { + contentType(ContentType.Application.Json) + header("X-GitHub-Event", DUMMY_EVENT) + header("X-Hub-Signature", "sha1=$DUMMY_SIGNATURE") + header("X-GitHub-Delivery", "delivery") + setBody(DUMMY_PAYLOAD) + } + + val metrics = client.get("/metrics").bodyAsText() + + assertTrue( + metrics.contains( + """githubapp_webhook_events_received_total{event="$DUMMY_EVENT"} 1.0""" + ), + "received counter missing in:\n$metrics" + ) + assertTrue( + metrics.contains( + """githubapp_notifications_sent_total{event="$DUMMY_EVENT"} 1.0""" + ), + "sent counter missing in:\n$metrics" + ) + } + } + + @Test + fun `given no template for the event, when GET metrics, then unsupported counter is exposed`() { + val signatureValidator = mockk() + every { signatureValidator.isValid(any(), any(), any(), any()) } returns true + + val wireAppSdk = mockk(relaxed = true) + + val templateHandler = mockk() + every { + templateHandler.handleEvent(event = any(), response = any()) + } returns null + + loadKoinModules( + module { + single { signatureValidator } + single { wireAppSdk } + single { templateHandler } + } + ) + + testApplication { + application { + module() + } + + val response = client.post("/${CONVERSATION_ID.id}/${CONVERSATION_ID.domain}") { + contentType(ContentType.Application.Json) + header("X-GitHub-Event", DUMMY_EVENT) + header("X-Hub-Signature", "sha1=$DUMMY_SIGNATURE") + header("X-GitHub-Delivery", "delivery") + setBody(DUMMY_PAYLOAD) + } + + assertEquals(HttpStatusCode.OK, response.status) + verify(exactly = 0) { + wireAppSdk.getApplicationManager().sendMessage(message = any()) + } + + val metrics = client.get("/metrics").bodyAsText() + + assertTrue( + metrics.contains( + """githubapp_unsupported_events_total{action="created",event="$DUMMY_EVENT"} 1.0""" + ), + "unsupported counter missing in:\n$metrics" + ) + assertTrue( + metrics.contains("githubapp_notifications_sent_total") == false, + "nothing should have been sent, but sent counter exists in:\n$metrics" + ) + } + } + @Test fun `given received event, when pull_request is created, then validations are passing`() { val signatureValidator = mockk()