allow users to set sampling priority

[lucaspimentel]

Allow users to override a trace's sampling priority by setting span tags.

Updates #214.

[boydc7]

Was this intentional? I don't really care much, but I seem to recall something about not using latest vs. specific versions

[lucaspimentel]

Yeah. I was interested in using the Enum generic type constraint. I was going to go without, but noticed that Datadog.Trace.ClrProfiler.Managed was already at 7.3, anyway.

[boydc7]

Not sure where this is used, so this may just be nothing to worry about, and if so just ignore the rest, I'm assuming I'll find out later in this PR.

In the past I've also done something like this and ran into cases including "Y", "N", "T", "F", any non-1 numeric value to indicate false (-1 is popular it seems)....there are also cases with FLAG like cases (i.e. command line args that are implicitly true/false just by being present or not) where a value of "" or whitespace could/should be implied to be true vs. the default of false...again, entirely dependent on where it's used I suppose.

If it's only for our internal use, not a big deal, if it's for trying to cover those, might want to reconsider a logic flow that accounts for these in as performant a way as possible.

I'd also favor just a string comparison strategy over ToUpperInvariant-ing the source, which would be difficult to do in a switch format.

Something that in order does:

• source.StartsWith("t", StringComparison.OrdinalIgnoreCase) (or source[0] == 't' || source[0] == 'T')
• source.StartsWith("f", StringComparison.OrdinalIgnoreCase) ...
• Same for y/n
• Maybe next check if it is a known 1/0/-1 common value
• Then convert to int and compare vs. 0

[lucaspimentel]

This extension method is internal only. I'm using it to convert tags values like span.SetSpan("force.keep", "true") because Span.SetSpan() only allows string values. (Strongly-typed tags is something I would like to revisit later, but we need to support the OpenTracing convention regardless.) In the future, I plan to use the extension method to convert settings from string-typed sources like environment variables. In all cases, there will be only one correct documented value, but I wanted to give users a little margin for error by allowing more values aside from true/false when their intent is obvious. It is not meant to be exhaustive.

• I thought about allowing Y/N, T/F, and even enable/disable, but we have to draw the line somewhere. Once they are out there and people are using them, it will be harder to change or remove without breaking existing code.
• We shouldn't use StartsWith() because that would allow clearly invalid values like Yesterday or Tomorrow.
• Regarding -1, AFAIK the general rule is that 0 means false and any non-zero value means true. (not to be confused with return codes where 0 means success and non-zero values are error codes)
• The switch is effectively doing ordinal comparisons ignoring case, and in this case is much easier to read and maintain than a chain of if/else with StringComparison. I imagine that using ToUpperInvariant() once and making several case-sensitive comparisons is also slightly faster.

[boydc7]

Similar feedback here as to other PRs - we seem to just really want to keep piling the logic/responsibilities inside a single class (Span in this case) when the responsibility here is external, and could easily be separated away. I would particularly at this point start pulling this stuff out into an external tagging service(s) that handle this....also starts/moves us down a path towards making a span a dumb dto that is managed by a external composable things....

[boydc7]

huh, look at that, I wasn't aware you could constrain a type by Enum

[lucaspimentel]

It was added recently in C# 7.3.