stats: add utilities to create stats from a vector of tokens, mixing dynamic and symbolic elements.

[jmarantz]

Description: Creating joined stat names is a bit awkward due to the way the memory management needs to work, and having dynamic components to the name adds further complexity. This PR adds some utility helper methods to make this much easier and terse to express.

There's one subtle semantic change in the grpc status, which is to use a Dynamic stat name rather than building a locally controlled mutex-protected map from strings to symbolic stat names.

Risk Level: low -- this is mostly a refactoring
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a

[jmarantz]

@snowp @mattklein123 @danzh2010 ptal

Snow/Matt: curious what you think about this pattern of putting this helper function in Stats::Utility rather than making it a method of Scope. The downside is it's more cumbersome as a static method at call-sites. The upside is that there are several derivations of Scope and it's annoying to have to duplicate the code. I think Snow ran into this too.

One alternative which I've used a lot in the past, but is less common in Envoy, is to have a shared helper class between the interface and the implementations with common methods factored out there.

The other question is whether we should discourage extensive use of dynamic StatNames as they are more expensive, and you could argue this makes it too easy. Curious to hear your thoughts.

If I do go through with this, there are about 15 or so more places in the code where we could do some simplifications of creating stats based on this. Wouldn't all need to get done at once though.

[mattklein123]

At a high level this looks like a good improvement to me!

/wait

[snowp]

I agree that this is an improvement, I think we can document the pitfalls of using dynamic names instead of complicating the API intentionally.

I think any attempt to make this more explicit (e.g. no implicit cast from string_view) still makes it easy to mess up unless docs are consulted - unless we make it kinda ridiculous scary sounding, e.g. ,

Utility::counterFromElements(scope, {prefix_, DynamicStatDangerReadDocs(dynamic_value)}, tags);

which I don't think is worth it

[jmarantz]

I pushed a version where it's not quite as verbose as what @snowp suggested, but I did add a type-wrapper so you have to use Stats::Utility::Dynamic(str) when you want to compose a joined stat-name that includes a dynamic string. Otherwise i think it won't be at all clear at the call-sites that dynamics are being added.

Still working on cleaning up the other non-test uses of join().

[jmarantz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s), but failed to run 2 pipeline(s).

[jmarantz]

This is ready for review; thanks!

[snowp]

This is a massive improvement IMO, thanks for doing this! A couple comments

[mattklein123]

Thanks this is a great improvement. One small perf suggestion.

[mattklein123]

Thanks!

[jmarantz]

I need another review due to a commit (969c26a) to undo a suggestion about how to write the constructor, which apparently did not work at some call-site that I didn't manually re-test.

Addressed all comments.