Skip to content

Commit 74fd747

Browse files
committed
detect client disconnect on fetch
Add a StreamOps.onDownstreamAbort operator that runs an action when the downstream cancels before the upstream has completed, and use it on the /api/v2/fetch response entity to record an atlas.webapi.clientDisconnect counter when a client goes away mid-stream. This is observe-only: no request handling behavior changes. A mid-stream disconnect is currently invisible because the IPC access log entry is completed when the response headers are produced, not when the body has been delivered. watchTermination is not usable here since it completes successfully on a downstream cancellation and so cannot distinguish an abandoned stream from one that finished normally. The endpoint tag uses the matched request path to line up with the endpoint tag on ipc.server.call, which is derived from the Netflix-Endpoint header set by the endpointPath directives.
1 parent 17c3f5c commit 74fd747

4 files changed

Lines changed: 346 additions & 0 deletions

File tree

atlas-pekko/src/main/scala/com/netflix/atlas/pekko/StreamOps.scala

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import org.apache.pekko.stream.stage.GraphStageWithMaterializedValue
4545

4646
import java.util.concurrent.BlockingQueue
4747
import scala.concurrent.duration.FiniteDuration
48+
import scala.util.control.NonFatal
4849

4950
/**
5051
* Utility functions for commonly used operations on Pekko streams. Most of these are for
@@ -317,6 +318,72 @@ object StreamOps extends StrictLogging {
317318
Flow[T].via(new MonitorFlow[T](registry, id))
318319
}
319320

321+
/**
322+
* Performs the specified action if the downstream cancels before the upstream has
323+
* completed. When used on the source for an HTTP response entity, this indicates the
324+
* client went away before the full response could be delivered.
325+
*
326+
* Note this is distinct from `watchTermination`, which completes successfully for a
327+
* downstream cancellation and so cannot be used to tell an abandoned stream apart from
328+
* one that finished normally.
329+
*
330+
* @param action
331+
* Action to perform, passed the cause reported for the cancellation. For a normal
332+
* cancellation this will be `SubscriptionWithCancelException.NoMoreElementsNeeded`.
333+
*/
334+
def onDownstreamAbort[T](action: Throwable => Unit): Flow[T, T, NotUsed] = {
335+
Flow[T].via(new OnDownstreamAbort[T](action))
336+
}
337+
338+
private final class OnDownstreamAbort[T](action: Throwable => Unit)
339+
extends GraphStage[FlowShape[T, T]] {
340+
341+
private val in = Inlet[T]("OnDownstreamAbort.in")
342+
private val out = Outlet[T]("OnDownstreamAbort.out")
343+
344+
override val shape: FlowShape[T, T] = FlowShape(in, out)
345+
346+
override def createLogic(inheritedAttributes: Attributes): GraphStageLogic = {
347+
new GraphStageLogic(shape) with InHandler with OutHandler {
348+
349+
private var upstreamDone = false
350+
351+
override def onPush(): Unit = {
352+
push(out, grab(in))
353+
}
354+
355+
override def onPull(): Unit = {
356+
pull(in)
357+
}
358+
359+
override def onUpstreamFinish(): Unit = {
360+
upstreamDone = true
361+
completeStage()
362+
}
363+
364+
override def onUpstreamFailure(t: Throwable): Unit = {
365+
upstreamDone = true
366+
failStage(t)
367+
}
368+
369+
override def onDownstreamFinish(cause: Throwable): Unit = {
370+
// Do not let a failure in the action mask the cancellation itself. The cancellation
371+
// is propagated in a `finally` so it still happens if the action throws something
372+
// that is not caught below.
373+
try {
374+
if (!upstreamDone) action(cause)
375+
} catch {
376+
case NonFatal(e) => logger.warn("onDownstreamAbort action failed", e)
377+
} finally {
378+
cancelStage(cause)
379+
}
380+
}
381+
382+
setHandlers(in, out, this)
383+
}
384+
}
385+
}
386+
320387
private final class MonitorFlow[T](registry: Registry, id: String)
321388
extends GraphStage[FlowShape[T, T]] {
322389

atlas-pekko/src/test/scala/com/netflix/atlas/pekko/StreamOpsSuite.scala

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import org.apache.pekko.stream.Materializer
2323
import org.apache.pekko.stream.scaladsl.Keep
2424
import org.apache.pekko.stream.scaladsl.Sink
2525
import org.apache.pekko.stream.scaladsl.Source
26+
import org.apache.pekko.stream.testkit.scaladsl.TestSink
27+
import org.apache.pekko.stream.testkit.scaladsl.TestSource
2628
import com.netflix.spectator.api.DefaultRegistry
2729
import com.netflix.spectator.api.ManualClock
2830
import com.netflix.spectator.api.Registry
@@ -533,4 +535,73 @@ class StreamOpsSuite extends FunSuite {
533535
val result = Await.result(future, Duration.Inf)
534536
assertEquals(result.size, chunkSize * numChunks)
535537
}
538+
539+
//
540+
// onDownstreamAbort
541+
//
542+
// These tests drive both ends of the stream with probes so the ordering is explicit.
543+
// `upstream.expectCancellation()` only returns once the cancellation has propagated all
544+
// the way to the source, which is strictly after the stage under test has handled it.
545+
// That gives a happens-before edge without relying on timing.
546+
//
547+
548+
test("onDownstreamAbort: action runs when downstream cancels early") {
549+
val causes = new ArrayBlockingQueue[Throwable](1)
550+
val (upstream, downstream) = TestSource[Int]()
551+
.via(StreamOps.onDownstreamAbort[Int](causes.add))
552+
.toMat(TestSink[Int]())(Keep.both)
553+
.run()
554+
555+
downstream.request(1)
556+
upstream.sendNext(1)
557+
downstream.expectNext(1)
558+
downstream.cancel()
559+
560+
upstream.expectCancellation()
561+
assertEquals(causes.size(), 1)
562+
}
563+
564+
test("onDownstreamAbort: action does not run when upstream completes first") {
565+
val causes = new ArrayBlockingQueue[Throwable](1)
566+
val (upstream, downstream) = TestSource[Int]()
567+
.via(StreamOps.onDownstreamAbort[Int](causes.add))
568+
.toMat(TestSink[Int]())(Keep.both)
569+
.run()
570+
571+
downstream.request(1)
572+
upstream.sendNext(1)
573+
downstream.expectNext(1)
574+
upstream.sendComplete()
575+
downstream.expectComplete()
576+
577+
assertEquals(causes.size(), 0)
578+
}
579+
580+
test("onDownstreamAbort: action does not run when upstream fails") {
581+
val causes = new ArrayBlockingQueue[Throwable](1)
582+
val (upstream, downstream) = TestSource[Int]()
583+
.via(StreamOps.onDownstreamAbort[Int](causes.add))
584+
.toMat(TestSink[Int]())(Keep.both)
585+
.run()
586+
587+
downstream.request(1)
588+
upstream.sendError(new IllegalStateException("boom"))
589+
downstream.expectError()
590+
591+
assertEquals(causes.size(), 0)
592+
}
593+
594+
test("onDownstreamAbort: failure in the action does not mask the cancellation") {
595+
val (upstream, downstream) = TestSource[Int]()
596+
.via(StreamOps.onDownstreamAbort[Int](_ => throw new IllegalStateException("boom")))
597+
.toMat(TestSink[Int]())(Keep.both)
598+
.run()
599+
600+
downstream.request(1)
601+
upstream.sendNext(1)
602+
downstream.expectNext(1)
603+
downstream.cancel()
604+
605+
upstream.expectCancellation()
606+
}
536607
}

atlas-webapi/src/main/scala/com/netflix/atlas/webapi/FetchRequestSource.scala

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ import com.netflix.atlas.eval.graph.GraphConfig
2727
import com.netflix.atlas.eval.model.TimeSeriesMessage
2828
import com.netflix.atlas.json3.Json
2929
import com.netflix.atlas.pekko.DiagnosticMessage
30+
import com.netflix.atlas.pekko.StreamOps
31+
import com.netflix.spectator.api.Id
32+
import com.netflix.spectator.api.Registry
33+
import com.netflix.spectator.api.Spectator
3034
import org.apache.pekko.NotUsed
3135
import org.apache.pekko.actor.ActorRefFactory
3236
import org.apache.pekko.http.scaladsl.model.HttpEntity
@@ -64,6 +68,22 @@ object FetchRequestSource {
6468

6569
private val chunkSize = ApiSettings.fetchChunkSize
6670

71+
/**
72+
* Counter for clients that go away before the full response has been delivered. The
73+
* `endpoint` tag uses the matched request path so it lines up with the `endpoint` tag on
74+
* `ipc.server.call`, which comes from the `Netflix-Endpoint` header set by `endpointPath`.
75+
*/
76+
private val clientDisconnectId =
77+
Id.create("atlas.webapi.clientDisconnect").withTag("endpoint", "/api/v2/fetch")
78+
79+
// Atlas only allows `-._A-Za-z0-9^~` in tag values. A Scala object reports a simple name with
80+
// a trailing `$` and an anonymous class reports an empty one, so clean up the class name
81+
// before using it as a tag value.
82+
private def causeTag(cause: Throwable): String = {
83+
val name = cause.getClass.getSimpleName.stripSuffix("$")
84+
if (name.isEmpty) "unknown" else name
85+
}
86+
6787
/**
6888
* Create an SSE source that can be used as the entity for the HttpResponse.
6989
*/
@@ -139,7 +159,23 @@ object FetchRequestSource {
139159
* Returns an HttpResponse with an entity that is generated by the fetch source.
140160
*/
141161
def createResponse(system: ActorRefFactory, graphCfg: GraphConfig): HttpResponse = {
162+
createResponse(system, graphCfg, Spectator.globalRegistry())
163+
}
164+
165+
private[webapi] def createResponse(
166+
system: ActorRefFactory,
167+
graphCfg: GraphConfig,
168+
registry: Registry
169+
): HttpResponse = {
170+
// Detect a client that goes away part way through the response. The access log entry is
171+
// completed when the response headers are produced, so a mid-stream disconnect is not
172+
// otherwise visible.
142173
val source = apply(system, graphCfg)
174+
.via(StreamOps.onDownstreamAbort { cause =>
175+
registry
176+
.counter(clientDisconnectId.withTag("cause", causeTag(cause)))
177+
.increment()
178+
})
143179
HttpResponse(
144180
status = StatusCodes.OK,
145181
entity = HttpEntity.Chunked(MediaTypes.`text/event-stream`, source)
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/*
2+
* Copyright 2014-2026 Netflix, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.netflix.atlas.webapi
17+
18+
import com.netflix.atlas.core.model.TimeSeries
19+
import com.netflix.atlas.eval.graph.Grapher
20+
import com.netflix.spectator.api.AbstractRegistry
21+
import com.netflix.spectator.api.Clock
22+
import com.netflix.spectator.api.Counter
23+
import com.netflix.spectator.api.DefaultRegistry
24+
import com.netflix.spectator.api.DistributionSummary
25+
import com.netflix.spectator.api.Gauge
26+
import com.netflix.spectator.api.Id
27+
import com.netflix.spectator.api.Measurement
28+
import com.netflix.spectator.api.Registry
29+
import com.netflix.spectator.api.Timer
30+
import com.typesafe.config.ConfigFactory
31+
import munit.FunSuite
32+
import org.apache.pekko.actor.Actor
33+
import org.apache.pekko.actor.ActorSystem
34+
import org.apache.pekko.actor.Props
35+
import org.apache.pekko.http.scaladsl.model.HttpEntity
36+
import org.apache.pekko.http.scaladsl.model.HttpRequest
37+
import org.apache.pekko.stream.scaladsl.Sink
38+
import org.apache.pekko.stream.scaladsl.Source
39+
import org.apache.pekko.stream.testkit.scaladsl.TestSink
40+
41+
import java.util.concurrent.CountDownLatch
42+
import java.util.concurrent.TimeUnit
43+
import scala.concurrent.Await
44+
import scala.concurrent.duration.*
45+
import scala.jdk.CollectionConverters.*
46+
47+
class FetchRequestSourceSuite extends FunSuite {
48+
49+
private val config = ConfigFactory.load()
50+
private val grapher = Grapher(config)
51+
52+
/** Db actor that responds with empty data for every request. */
53+
private class EmptyDb extends Actor {
54+
55+
def receive: Receive = {
56+
case GraphApi.DataRequest(ctx, exprs, _) =>
57+
val data = exprs.map(e => e -> List.empty[TimeSeries]).toMap
58+
sender() ! GraphApi.DataResponse(ctx.step, data)
59+
}
60+
}
61+
62+
private def disconnects(registry: Registry): Double = {
63+
registry
64+
.counters()
65+
.iterator()
66+
.asScala
67+
.filter(_.id().name() == "atlas.webapi.clientDisconnect")
68+
.map(_.actualCount())
69+
.sum
70+
}
71+
72+
/**
73+
* Counter that releases a latch once it has been updated. The disconnect is recorded on a
74+
* stream thread and there is nothing upstream of the recording stage for the test to
75+
* synchronize on, so the counter itself provides the happens-before edge.
76+
*/
77+
private class SignalCounter(delegate: Counter, latch: CountDownLatch) extends Counter {
78+
79+
override def id(): Id = delegate.id()
80+
81+
override def measure(): java.lang.Iterable[Measurement] = delegate.measure()
82+
83+
override def hasExpired: Boolean = delegate.hasExpired
84+
85+
override def actualCount(): Double = delegate.actualCount()
86+
87+
override def add(amount: Double): Unit = {
88+
delegate.add(amount)
89+
latch.countDown()
90+
}
91+
}
92+
93+
/** Registry that releases `latch` when a counter named `name` is updated. */
94+
private class SignalRegistry(name: String, latch: CountDownLatch)
95+
extends AbstractRegistry(Clock.SYSTEM) {
96+
97+
private val delegate = new DefaultRegistry()
98+
99+
override protected def newCounter(id: Id): Counter = {
100+
val c = delegate.counter(id)
101+
if (id.name() == name) new SignalCounter(c, latch) else c
102+
}
103+
104+
override protected def newDistributionSummary(id: Id): DistributionSummary =
105+
delegate.distributionSummary(id)
106+
107+
override protected def newTimer(id: Id): Timer = delegate.timer(id)
108+
109+
override protected def newGauge(id: Id): Gauge = delegate.gauge(id)
110+
111+
override protected def newMaxGauge(id: Id): Gauge = delegate.maxGauge(id)
112+
}
113+
114+
private def withFixture(registry: Registry)(
115+
f: (ActorSystem, Registry, Source[HttpEntity.ChunkStreamPart, ?]) => Unit
116+
): Unit = {
117+
val system = ActorSystem(s"FetchRequestSourceSuite-${System.nanoTime()}")
118+
try {
119+
system.actorOf(Props(new EmptyDb), "db")
120+
val uri = "/api/v2/fetch?q=name,sps,:eq,:sum&s=e-6h&e=now&step=1h"
121+
val graphCfg = grapher.toGraphConfig(HttpRequest(uri = uri))
122+
val response = FetchRequestSource.createResponse(system, graphCfg, registry)
123+
response.entity match {
124+
case HttpEntity.Chunked(_, source) => f(system, registry, source)
125+
case other => fail(s"expected a chunked entity, got $other")
126+
}
127+
} finally {
128+
Await.ready(system.terminate(), 30.seconds)
129+
}
130+
}
131+
132+
test("no disconnect recorded when the full response is consumed") {
133+
withFixture(new DefaultRegistry) { (system, registry, source) =>
134+
implicit val sys: ActorSystem = system
135+
// Awaiting the sink gives a happens-before edge: the stream has fully terminated,
136+
// so the counter is stable by the time it is read.
137+
Await.result(source.runWith(Sink.ignore), 30.seconds)
138+
assertEquals(disconnects(registry), 0.0)
139+
}
140+
}
141+
142+
test("disconnect recorded when the client goes away") {
143+
val latch = new CountDownLatch(1)
144+
withFixture(new SignalRegistry("atlas.webapi.clientDisconnect", latch)) {
145+
(system, registry, source) =>
146+
implicit val sys: ActorSystem = system
147+
// Cancel without requesting anything. With no demand the terminating `close` message
148+
// cannot be pushed, so the upstream cannot have completed first and the cancellation is
149+
// unambiguously an abort. Requesting an element first would be racy: for an empty
150+
// response the close message is the only element, and the source completes as soon as
151+
// it has been pushed.
152+
val probe = source.runWith(TestSink[HttpEntity.ChunkStreamPart]())
153+
probe.cancel()
154+
155+
// Blocks until the counter has actually been updated, so the read below is ordered
156+
// after it. The timeout only bounds a failure, it is not a wait for the common case.
157+
assert(latch.await(10, TimeUnit.SECONDS), "disconnect counter was never updated")
158+
assertEquals(disconnects(registry), 1.0)
159+
}
160+
}
161+
162+
test("response entity is a chunked SSE stream") {
163+
withFixture(new DefaultRegistry) { (system, registry, source) =>
164+
implicit val sys: ActorSystem = system
165+
val parts = Await.result(source.runWith(Sink.seq), 30.seconds)
166+
assert(parts.nonEmpty)
167+
val text = parts.map(_.data().utf8String).mkString
168+
assert(text.startsWith("data: "), s"unexpected SSE payload: ${text.take(40)}")
169+
assertEquals(disconnects(registry), 0.0)
170+
}
171+
}
172+
}

0 commit comments

Comments
 (0)