Skip to content

Implement idle timeout, additional HTTP attributes - #47

Merged
ladd merged 3 commits into
mainfrom
lvt-idle-timeout
Nov 12, 2025
Merged

Implement idle timeout, additional HTTP attributes#47
ladd merged 3 commits into
mainfrom
lvt-idle-timeout

Conversation

@ladd

@ladd ladd commented Nov 12, 2025

Copy link
Copy Markdown
  • Add an idle timeout feature for the reporter to observe when no spans have been retired in a defined interval. This simplifies implementing trace flushing during background tasks.
  • Collect additional HTTP attributes: connection reuse, proxied, DNS, TCP, TLS timings.
  • Minimum flush / idle interval changed to 0.1 seconds.

@jparise
@bachand

@bachand bachand left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @ladd !

/// Add listeners for application lifecycle events -- typically called during didFinishLaunching.
func subscribeToLifecycleEvents()

/// Called when no spans have been retied in `idleTimeoutInterval`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Called when no spans have been retied in `idleTimeoutInterval`
/// Called when no spans have been retired in `idleTimeoutInterval`

addAttribute("network.connection.type", metric.isCellular ? "cell" : "wifi")
addAttribute("network.protocol.version", Self.networkProtocolVersion(metric.networkProtocolName))

// No official semantic convention exists yet for the the properties below

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// No official semantic convention exists yet for the the properties below
// No official semantic convention exists yet for the properties below

Comment on lines +31 to +33
public var flushInterval: TimeInterval { Tracer.defaultFlushInterval }

public var idleTimeoutInterval: TimeInterval { Tracer.defaultIdleInterval }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between the NautilusTelemetryReporter and the Tracer? I'm asking to learn.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NautilusTelemetryReporter protocol is implemented in an app component and is responsible for sending telemetry batches to an OpenTelemetry collector instance. Tracer is responsible for span creation & batching -- it passes off batches to the reporter.

flushInterval = Self.defaultFlushInterval
idleTimeoutInterval = Self.defaultIdleInterval
flushTimer = FlushTimer(flushInterval: flushInterval, repeating: true) { [weak self] in self?.flushRetiredSpans() }
idleTimer = FlushTimer(flushInterval: idleTimeoutInterval, repeating: false) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me understand why this one doesn't repeat?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idle timer firing is pushed forward every time a span is retired and then fires once if no more spans are retired in that interval. This implements a behavior of "if no spans have been ended for 10 seconds, then inform the reporter". The intended use case is for the reporter to check whether the app is in the background & then flush the trace, which gives us better capture of background task & notification-driven fetches.

Internal PR coming which may make this more clear.

flushTimer.schedule(
deadline: DispatchTime.now() + flushInterval,
repeating: flushInterval,
repeating: repeating ? flushInterval : .infinity,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting .never when repeating == false. Can you help. me understand .infinity?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's from Apple's docs for the variant that takes a Double for the repeating interval:

    /// - parameter repeating: the repeat interval for the timer in seconds, or `.infinity` if the timer
    ///		should fire only once.

If you pass a DispatchTimeInterval, you can use .never, which is probably nicer to read.

XCTAssertNil(attributes["http.response.header.content-encoding"])
}

func testNilDateElapsedNanosecondAttribute() throws {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

var idleTimeoutInterval: TimeInterval { 0.1 }

func idleTimeout() {
idleExpectation.fulfill()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

var handlerCallCount = 0

let timer = FlushTimer(flushInterval: 0.1) {
let timer = FlushTimer(flushInterval: 0.1, repeating: true) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test for repeating: false?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Comment on lines +49 to +50
timer.flushInterval = 0.05
XCTAssertEqual(timer.flushInterval, 0.05)
XCTAssertEqual(timer.flushInterval, timer.minimumFlushInterval)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
timer.flushInterval = 0.05
XCTAssertEqual(timer.flushInterval, 0.05)
XCTAssertEqual(timer.flushInterval, timer.minimumFlushInterval)
let tooSmallFlushInterval = 0.05
XCTAssertNotEqual(tooSmallFlushInterval, timer.minimumFlushInterval)
timer.flushInterval = tooSmallFlushInterval
XCTAssertEqual(timer.flushInterval, timer.minimumFlushInterval)

This would be a bit more powerful and robust to changes.

@ladd
ladd merged commit 513d965 into main Nov 12, 2025
7 of 8 checks passed
@ladd
ladd deleted the lvt-idle-timeout branch November 12, 2025 21:04

@bachand bachand left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my feedback @ladd !

flushTimer.schedule(
deadline: DispatchTime.now() + flushInterval,
repeating: repeating ? flushInterval : .infinity,
repeating: repeating ? DispatchTimeInterval(flushInterval) : .never,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot clear 😅 Thanks!

Comment on lines +248 to +249
// Push the timeout ahead
idleTimer?.setupTimer()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants