Add Distributed Tracing support #177
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Moritz Lang <[email protected]>
Signed-off-by: Moritz Lang <[email protected]>
Signed-off-by: Moritz Lang <[email protected]>
Signed-off-by: Moritz Lang <[email protected]>
Signed-off-by: Moritz Lang <[email protected]>
Signed-off-by: Moritz Lang <[email protected]>
slashmo
force-pushed
the
feature/distributed-tracing
branch
from
6c6fb74 to
5606d42
Compare
czechboy0
reviewed
Aug 6, 2025
| @@ -161,16 +176,48 @@ public final actor ValkeyConnection: ValkeyClientProtocol, Sendable { | |||
|
|
|||
| @inlinable | |||
| func _execute<Command: ValkeyCommand>(command: Command) async throws -> RESPToken { | |||
| #if DistributedTracingSupport | |||
| let span = startSpan(Command.name, ofKind: .client) | |||
| defer { span.end() } | |||
There was a problem hiding this comment.
If you wanted to get rid of the unstructured span, could also do something like:
func _execute<Command: ValkeyCommand>(command: Command) async throws -> RESPToken {
#if DistributedTracingSupport
try await _executeWithTracing(command: command)
#else
try await _execute0(command: command)
#endif
}
func _execute0<Command: ValkeyCommand>(command: Command) async throws -> RESPToken {
// existing execute logic
}
#if DistributedTracingSupport
func _executeWithTracing<Command: ValkeyCommand>(command: Command) async throws -> RESPToken {
try await withSpan { span in
// update the span
do {
try await _execute0(command: command)
} catch {
// extract info from the error into the span
}
}
#endifThere was a problem hiding this comment.
Thanks for the suggestion. This is similar to what I originally went with. I eventually switched to the more imperative API because I thought it was easier to read. I'm happy to explore withSpan again though.
There was a problem hiding this comment.
I'll let Adam/Fabian steer you here, I'm also happy with either.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #177 +/- ##
=======================================
Coverage ? 41.42%
=======================================
Files ? 83
Lines ? 13899
Branches ? 0
=======================================
Hits ? 5757
Misses ? 8142
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The API breakage check rightfully complains about the added protocol requirement extension ValkeyCommand {
public static var name: String { String(describing: Self.self) }
} |
Signed-off-by: Moritz Lang <[email protected]>
Signed-off-by: Moritz Lang <[email protected]>
slashmo
force-pushed
the
feature/distributed-tracing
branch
from
a67131f to
c57a3b5
Compare
API breakages are fine |
Signed-off-by: Moritz Lang <[email protected]>
adam-fowler
reviewed
Aug 6, 2025
There was a problem hiding this comment.
A couple of initial comments.
It's a shame I haven't got the benchmarks running for non-members of the repo. It fails on add the comment to the PR. I imagine this will add allocations. Can you run the benchmarks locally (run main and compare this PR with main) and post the results as a comment, cheers.
| @@ -40,6 +44,8 @@ public final actor ValkeyConnection: ValkeyClientProtocol, Sendable { | |||
| @usableFromInline | |||
| let channelHandler: ValkeyChannelHandler | |||
| let configuration: ValkeyConnectionConfiguration | |||
| @usableFromInline | |||
| let address: (hostOrSocketPath: String, port: Int?)? | |||
There was a problem hiding this comment.
If port isn't set you can default to 6379
| @@ -189,8 +236,42 @@ public final actor ValkeyConnection: ValkeyClientProtocol, Sendable { | |||
| public func execute<each Command: ValkeyCommand>( | |||
| _ commands: repeat each Command | |||
| ) async -> sending (repeat Result<(each Command).Response, Error>) { | |||
| #if DistributedTracingSupport | |||
| let span = startSpan("MULTI", ofKind: .client) | |||
There was a problem hiding this comment.
Why MULTI? This implies you are running the Valkey command MULTI which starts a transaction.
Is it worthwhile building a string out of all of the commands eg SET,LPUSH,INCR
There was a problem hiding this comment.
I was under the impression that we're running a MULTI followed by the commands here. If that's not the case I believe the correct thing to do is to create a span for each command, instead of one that groups them.
adam-fowler
reviewed
Aug 6, 2025
There was a problem hiding this comment.
I wrote a TestTracer that allowed you to run tests in parallel in Hummingbird. https://github.com/hummingbird-project/hummingbird/blob/main/Tests/HummingbirdTests/TracingTests.swift. It used Task Local variables to separate tracers for each test.
You could probably use a Testing trait to set it up for each test.
|
Here's the benchmark diff between swift-driver version: 1.127.10 Apple Swift version 6.2 (swiftlang-6.2.0.14.8 clang-1700.3.14.6)
Target: arm64-apple-macosx15.0===============================================================
Threshold deviations for ValkeyBenchmarks:Client: GET benchmark
===============================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Malloc (total) (K, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 74 │ 81 │ 10 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 77 │ 83 │ 8 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p75 │ 78 │ 85 │ 8 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
=========================================================================================================
Threshold deviations for ValkeyBenchmarks:Client: GET benchmark | parallel 20 | 20 concurrent connections
=========================================================================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Malloc (total) (K, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p75 │ 117 │ 125 │ 6 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
=========================================================================================================
Threshold deviations for ValkeyBenchmarks:Client: GET benchmark | parallel 50 | 20 concurrent connections
=========================================================================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Malloc (total) (K, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 5 │ 7 │ 30 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 8 │ 11 │ 45 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p75 │ 11 │ 18 │ 59 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Instructions (M, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 247 │ 269 │ 8 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 249 │ 271 │ 9 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p75 │ 250 │ 272 │ 9 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
===================================================================
Threshold deviations for ValkeyBenchmarks:Connection: GET benchmark
===================================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Throughput (# / s) (#, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 29 │ 27 │ 6 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 29 │ 27 │ 6 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p75 │ 29 │ 27 │ 6 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Time (wall clock) (ms, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 34 │ 37 │ 6 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 35 │ 37 │ 6 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p75 │ 35 │ 38 │ 8 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Time (total CPU) (ms, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 29 │ 31 │ 6 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 29 │ 31 │ 6 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p75 │ 30 │ 32 │ 9 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Instructions (M, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 225 │ 247 │ 9 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 225 │ 247 │ 9 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p75 │ 225 │ 247 │ 9 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
========================================================================
Threshold deviations for ValkeyBenchmarks:Connection: Pipeline benchmark
========================================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Throughput (# / s) (#, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 17 │ 16 │ 5 │ 5 │
├──────────────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┼─────────────────┤
│ p50 │ 17 │ 16 │ 5 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Malloc (total) (K, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p25 │ 14 │ 26 │ 83 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
====================================================================
Threshold deviations for ValkeyBenchmarks:HashSlot – {user}.whatever
====================================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Throughput (# / s) (#, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p75 │ 57 │ 53 │ 7 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Time (wall clock) (ms, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p75 │ 18 │ 19 │ 7 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
=====================================================================================
Threshold deviations for ValkeyBenchmarks:ValkeyCommandEncoder – Command with 7 words
=====================================================================================
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Time (wall clock) (μs, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p75 │ 420 │ 441 │ 5 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
╒══════════════════════════════════════════╤═════════════════╤═════════════════╤═════════════════╤═════════════════╕
│ Time (total CPU) (μs, %) │ main │ pr │ Difference % │ Threshold % │
╞══════════════════════════════════════════╪═════════════════╪═════════════════╪═════════════════╪═════════════════╡
│ p75 │ 421 │ 443 │ 5 │ 5 │
╘══════════════════════════════════════════╧═════════════════╧═════════════════╧═════════════════╧═════════════════╛
New baseline 'pr' is WORSE than the 'main' baseline thresholds.
error: benchmarkThresholdRegression |
Signed-off-by: Moritz Lang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
To make the most of Distributed Tracing, users may want to view operations performed by their database clients as part of a larger request to gain more insight into their distributed systems.
Modifications
DistributedTracingSupporttrait, enabled by defaultdb.operation.namespan attributeopen-telemetryexample to showcase Distributed Tracing supportWhen implementing the span and span attributes I closely followed the OpenTelemetry semantic conventions for Redis and general database client spans:
I believe that this provides the highest chance of the span being recognized as a database client operation by various observability backends.
Result
Users can now use Distributed Tracing spans automatically created by the library to more deeply observe their systems. They can also see an example set up of a Hummingbird server, the Valkey client, and Swift OTel to guide their Distributed Tracing usage.
Closes #176