Skip to content

Conversation

slashmo
Copy link
Contributor

@slashmo slashmo commented Aug 21, 2025

Motivation

Most ValkeyCommand types in valley-swift exactly match their underlying Valkey command name. However, for some commands like nested ones the Swift type name differs from the underlying command. This means there's no simple way of accessing the underlying command's name such as using "\(type(of: Self.self))", which is required for Distributed Tracing support to derive proper span operation names based on the command being executed. See Also: #177
When running codegen for these commands we do have access to the underlying command names, so we could add them as properties to the generated Swift types.

Note

This PR is split off from #177 to decrease that PR's diff and keep it focused on just adding Distributed Tracing support.

Modifications

  • Updated the command builder to generate a command name property, to be used in Distributed Tracing spans
  • Re-run command builder to generate command name properties

Result

All ValkeyCommand conforming types now have a static name property exposing the underlying Valkey command name. This will enable proper Distributed Tracing span operation names.

@slashmo slashmo force-pushed the feature/generate-command-name branch from 4abb8de to 7e96dca Compare August 21, 2025 19:33
@slashmo slashmo requested a review from fabianfett August 21, 2025 19:34
@adam-fowler
Copy link
Collaborator

You forgot to include the ValkeyCommandsBuilder changes

@slashmo slashmo force-pushed the feature/generate-command-name branch from 7e96dca to 88369d8 Compare August 21, 2025 20:49
@slashmo
Copy link
Contributor Author

slashmo commented Aug 21, 2025

You forgot to include the ValkeyCommandsBuilder changes

Those changes were already in the PR (https://github.com/valkey-io/valkey-swift/pull/186/files#diff-2f98614f8e9a53691e3637ff419218f0aaf7554d5d9eb48f6572c3087388e002). I did however forget to add the name property to the non-generated GET command in IntegrationTests.

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 0% with 386 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@b116c42). Learn more about missing BASE report.

Files with missing lines Patch % Lines
Sources/Valkey/Commands/ServerCommands.swift 0.00% 71 Missing ⚠️
Sources/Valkey/Commands/SortedSetCommands.swift 0.00% 35 Missing ⚠️
Sources/Valkey/Commands/GenericCommands.swift 0.00% 33 Missing ⚠️
Sources/Valkey/Commands/ClusterCommands.swift 0.00% 32 Missing ⚠️
Sources/Valkey/Commands/ConnectionCommands.swift 0.00% 28 Missing ⚠️
Sources/Valkey/Commands/SentinelCommands.swift 0.00% 25 Missing ⚠️
Sources/Valkey/Commands/StreamCommands.swift 0.00% 23 Missing ⚠️
Sources/Valkey/Commands/ListCommands.swift 0.00% 22 Missing ⚠️
Sources/Valkey/Commands/ScriptingCommands.swift 0.00% 22 Missing ⚠️
Sources/Valkey/Commands/StringCommands.swift 0.00% 22 Missing ⚠️
... and 8 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #186   +/-   ##
=======================================
  Coverage        ?   40.87%           
=======================================
  Files           ?       84           
  Lines           ?    13852           
  Branches        ?        0           
=======================================
  Hits            ?     5662           
  Misses          ?     8190           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@adam-fowler adam-fowler merged commit 1bc10e3 into valkey-io:main Aug 23, 2025
11 of 13 checks passed
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.

4 participants