Skip to content

Add a client name suffix option#2002

Closed
philon-msft wants to merge 2 commits intomainfrom
philon-msft/clientname
Closed

Add a client name suffix option#2002
philon-msft wants to merge 2 commits intomainfrom
philon-msft/clientname

Conversation

@philon-msft
Copy link
Collaborator

The suffix enables a couple of scenarios:

  • A client application adds a suffix to identify its connections on the server, while still relying on the default client name logic to use its machine name or instance name as the unique root of the client name
  • An extension package adds a suffix to identify connections where it's being used

Also add to the library version tag on default client names:

  • A "lib:" token to facilitate parsing
  • A library identifier "SER" to distinguish from other libraries that may use a similar version tag

@NickCraver
Copy link
Collaborator

May have mismatched on proposals here - I was thinking name overrides being in the default options provider - for example if there's a client name fetcher there or a suffix generally appended, both would work that way. Here from a config standpoint, specifying clientname and clientname suffix feels very odd (and restrictive) - I'd rather provide the ability to modify or set the client name entirely to the lib, let me spike this on the #1987 branch to give an idea - I think this will be both cleaner and more flexible.

@NickCraver
Copy link
Collaborator

@philon-msft alrighty I made a thing over in 3abc530 - thoughts? It occurs to me what I need to do in that branch is make an off-the-wall provider and make sure all that works with the overrides, adding to list!

@philon-msft
Copy link
Collaborator Author

Thanks @NickCraver a couple thoughts:

  • An advantage of a separate 'suffix' option is that it allows extension packages to append their IDs only when it won't interfere with a user-supplied clientName (because specifying a clientName disables the suffix). If we leave it to extension to do the appending, they have no way of knowing if the root is default or user-supplied. Is there another way to support that? Or is it acceptable for extensions to always append, potentially changing a user-specified client name? I'm actually okay with the latter, because users should expect that applying an extension will change behavior.
  • Admittedly, the "lib:" token doesn't add any useful info. But it does make parsing client names on the server easier. Without the token, we'll have to parse anything inside parentheses, which could be noisy. I'm open to ideas on how explicit that parsing should be, and how to best implement it.

@philon-msft
Copy link
Collaborator Author

On further examination, I think I see where you're going with the DefaultOptionsProvider appending a suffix. It's creating the default, so it can always append whatever it wants. If the user is specifying their own client name, then that default+suffix will never be used.

@NickCraver
Copy link
Collaborator

  • An advantage of a separate 'suffix' option is that it allows extension packages to append their IDs only when it won't interfere with a user-supplied clientName (because specifying a clientName disables the suffix). If we leave it to extension to do the appending, they have no way of knowing if the root is default or user-supplied. Is there another way to support that? Or is it acceptable for extensions to always append, potentially changing a user-specified client name? I'm actually okay with the latter, because users should expect that applying an extension will change behavior.

I'd say either is acceptable, but for any meaningful usefulness we're talking about is it acceptable to append by default, which would mean changing explicitly supplied client names today. I'd lean towards that being unacceptable and likely breaking lots of use cases. I'm 100% fine changing the state if users aren't explicitly specifying anything today (total defaults) and that's the approach here.

  • Admittedly, the "lib:" token doesn't add any useful info. But it does make parsing client names on the server easier. Without the token, we'll have to parse anything inside parentheses, which could be noisy. I'm open to ideas on how explicit that parsing should be, and how to best implement it.

I agree this may be noisy, but: we're not going to get all the libs to agree on a string format anyway - in my mind there's 0% chance this isn't a regex for any kind of cross-client analysis (many old clients aren't ever getting updated, for example). My preference would be to leave the nasty bits out of every connection for a single server side convenience use case, especially if we're the only one doing it. If we're the only one using it as a practical consumer example, there's still effectively lib: parsing just for us, which is the same as SE.Redis-v just for us, the cost/payoff is equivalent there.

That's my thinking anyway - curious where ya end up on this!

@philon-msft
Copy link
Collaborator Author

@NickCraver Agreed on all points - thanks for the discussion! I'll close my PR. Or should I tweak it to only add the "SE.Redis-" in the version tag? I'd like to include that in the official 2.5 release if possible. But if you're planning to pull in #1987 for the initial 2.5 release, then no need for my PR at all.

@NickCraver
Copy link
Collaborator

@philon-msft With your input on scenarios and some Marc eyes to boot, I see no issues landing that in 2.5x, adding some tests this AM before on to house things! I'd like to talk through this Tuesday and get it on in...or if it's contentious we'll can just tweak the suffix in a small PR for the release ASAP.

@philon-msft
Copy link
Collaborator Author

Will be implemented with a different approach in #1987

@NickCraver NickCraver deleted the philon-msft/clientname branch March 7, 2022 00:32
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