Add validation to metric names, slight refactoring

[afparsons]

I was going through some old forks of mine and realized I had made some changes on a branch.

This may have also served as a fix for #3.

I can confirm that this code does indeed work in a production application.

Frankly, I don't entirely remember exactly what my changes achieve, since it has been fifteen months since I looked at this. I think I added some validation and a few other conveniences.

Please merge if you think these changes are useful! Let me know if you need me to explain any parts in greater detail.

[sm-Fifteen]

Woah, you basically rewrote the whole thing! It's certainly a lot more proper than what I had before, and I can already see a number of benefits to some of these changes, like performing validation while creating ServerTimingMetricName instead of when initializing the middleware. I'll be sure to give it a try before merging.

Any reason why you used attrs instead of dataclasses for your data structures?

PS: Sorry if I've been leaving the issues and PRs to languish for so long, I've really neglected my personnal Github projects in recent years.

[afparsons]

Any reason why you used attrs instead of dataclasses for your data structures?

I couldn't say 😅 Like I wrote earlier, I haven't looked at this in fifteen months. As a quick guess, I assume I wanted to leverage the field validation facilities attrs provides. Let me think on it.

It may be a good idea to replace attrs in favor of a standard library implementation in order to eliminate the dependency.

[afparsons]

I began removing the dependency on attrs. I'll finalize this pull request soon.

[afparsons]

Hi @sm-Fifteen! Take a look when you get the chance.

[sm-Fifteen]

@afparsons: Alright, sorry for taking so long to come back to this, I'm looking at it now.

• I notice you have two separate structures, which you convert between using server_timing_metrics_to_dict(), and that strikes me as unneccessary? You even reimplement some methods across both, so I'm not entirely sure why you don't simply use the same one?
• You have description be a field in _ServerTimingMetricName, but you should probably have it live in ServerTimingMetric. Despite the name, "description" isn't actually meant to be a longer description of what the metric name stands for (which would be a waste of bandwidth). It's never spelled out out lout in the spec, but the examples use customView, dc;desc=atl to indicate that some request was routed through the "atl" datacenter, for instance.
  • I would actually change the doc to read "A text value for the Server-Timing metric." instead of "A description for the Server-Timing metric."
  • In fact, I'm pretty sure I used to have a yappimem;desc="1234k" ServerTiming metric to let users keep track of how close the profiler was getting to the limit (I really don't know why I didn't keep it in).
• It occured to me while reviewing your code that quoted strings still need to escape double-quotes and backslashes, which I myself forgot to do. It's just a re.sub(r'(["\\])', r'\\\1', desc_text) substitution, but now that it's a properly structured project, I guess it would be a sin to forget it.

That aside, it looks pretty good!

[afparsons]

Woah, I totally forgot about this pull request. Maybe I'll get around to it. Sorry for the long wait!

[sm-Fifteen]

Don't worry about it. I've been in a continuous cycle of forgetting and remembering about this repo for the past 4 years, I understand how it's like.