Design discussion: improving the lib name/version story

[mgravell]

Current status

• redis supports the concept of connection-level metadata for library name/version
• sending this metadata can be disabled entirely in the library via ConfigurationOptions.SetLibraryName (a bool - on/off); it is enabled by default
• the library used by this library comes from:
  • the name set via ConfigurationOptions.LibraryName
  • if that is null/whitespace, then from the DefaultOptionsProvider.LibraryName
    • which itself defaults to "SE.Redis"
• everything from the above is composed with any suffix values, discussed below
• the version used by this library comes from the assembly build metadata, and cannot currently be externally influenced
• the SetLibraryName option can be parsed from configuration strings, but the LibraryName cannot
• everything so far effectively is fixed at the point of connection creation

Suffixes

It is often useful to know how a connection is being used in reality, which is more than just the library. At the moment, the AddLibraryNameSuffix API allows for this, allowing tokens to be appended to the effective library name. This is de-duplicated and sorted, for consistency; the expectation is that some use-case (for example, IDistributedCache) can suggest their own token ("DC") at any point, and if that results in a new value, it gets added to the set of values. For example, "SE.Redis" might become "SE-Redis-DC". The version is not impacted.

Versions

A draft PR exists to suggest making the library version editable. This has been held back as the current proposal involves an API break.

The problem

SE.Redis is often used as an implementation detail of other libraries; this most notably includes (but is not limited to) NRedisStack, which:

• has an API that touches the CLIENT API for this scenario
• actively uses that feature to attempt to stomp itself as the library in some scenarios

It is understood that the intent here is to provide useful NRedisStack metadata, including version. However, this results in the two libraries fighting - SE.Redis thinks it owns the client name, and may overstomp that NRedisStack value at any time; likewise, NRedisStack has no direct visibility of that, and has to hack in some unreliable heuristics to attempt to set the client metadata at the appropriate time. This is especially brittle in scenarios involving cluster and / or reconnects, and is discussed here. It is also noted that this feature bypasses both SetClientName and the suffix feature.

Proposal

(perhaps read the counter-proposal first)

Consider a new API that provides a managed and reasonable approach to library metadata, influenced by the existing suffix feature.

Example; consider the notorious ANOtherLib package, version 4.21, running via SE.Redis 2.9.1:

IConnectionMultiplexer conn = ...
//
// client metadata; name=`SE.Redis`, version=`2.9.1` 
//
// (time happens)
// ...
// some code involving ANOtherLib
conn.AddLibrary("ANOtherLib", Utils.MemoizedLibVersion());
//
// client metadata; name=`SE.Redis/ANOtherLib`, version=`2.9.1/4.21` 

• Subsequent calls add ANOtherLib would be a no-op (we would ignore version changes; that is explicitly not supported). This is effectively a map (dictionary) of additional library pairs.
• The library deals with de-duplicating and tracking concerns, and issuing any required updates to each connection.
• Library names are sorted alphabetically after SE.Redis (which isn't an "additional" library); versions are added in the same order as their libraries (so: AddLibrary("B","X") followed by AddLibrary("A", "Y") would reliably give the output SE.Redis/A/B paired with 2.9.1/Y/X)
• I suspect this also obviates the need to directly support changing lib-ver (Options: support changing lib-ver #2547)
• it may suggest that DefaultConfigurationOptions needs a change, to allow options-providers to suggest library a name/version pair (or pairs?) rather than the existing limited option
• I'm open to the idea of SE.Redis dealing with "read the assembly version from the calling code", if that has utility

(the specific details, for example the use of /, are subject to change based on testing against target server versions)

Mixing this with suffixes

If suffixes are being used, this would be further composed, i.e. SE.Redis/A/B-DC; there is no corresponding version tag for data added as suffixes, and there is no suggestion to add one.

Observations

This API does not allow an external client to assume total control of the client metadata, i.e. to say "this connection is ANOtherLib version 4.2.1". This is very much by design, to avoid multiple libraries (including SE.Redis) arguing and getting into a mess.

The existing suffix API has an inherent limitation insofar as it cannot influence RESP2 pub/sub connections; this is a server limitation. I do not propose any changes there: this API would have the same limitation on RESP2.

Counter-proposal

"Keep is simple, stupid"

Just allow random code to over-stomp the library at runtime and assume total ownership / blame; for example:

conn.SetLibrary("ANOtherLib", Utils.MemoizedLibVersion());
//
// client metadata; name=`ANOtherLib`, version=`4.21` 

This is very much "last caller wins"; we would short-circuit no-op calls, making it effectively free to call.

• pro: it is very simple
• con: it reduces our understanding; for example, we lose visibility of what SE.Redis version (arguably, "the actual client library") is being used
  • in most/all real-world cases it would still be possible to infer SE.Redis from the specific name used (for example, NRedisStack), but we would not know the SE.Redis version
• pro: having an API to do this formally is perhaps better than a library abusing Execute to achieve the same goal - we would simply bypass the chaos monkeys, while also respecting the suffix feature and SetClientLibrary flag
• con: if there are multiple different libraries constantly saying "me" via this API, then that could be pathologically bad
  • possible defensive mitigation: min duration between calls being effective?

[philon-msft]

+1 vote from me. Having MS.Az.SERedis version in addition to SE.Redis version would be very helpful. And I don't think this will have any impact on how we parse lib info in the service, nor how we report on it.
I would request a DefaultOptionsProvider.LibraryVersion so extension packages like MS.Az.SERedis can passively expose their version without needing to make a call. I'm less excited about automatically reading assembly version - simpler to leave it up to each package to decide what to report for their version.