Support question mark in wildcard

[reyang]

Previously we only support * in wildcards, not ?.
This PR added ? support.

"Indirectly" related to open-telemetry/opentelemetry-specification#2325 (this PR only touches the provider/source part, not the View part).

[cijothomas]

1. Need changelog entry
2. I suppose we didn't have unit test for tracing's AddSource with wildcard?

[codecov]

Codecov Report

Merging #2875 (1b91fb0) into main (b1d57e9) will decrease coverage by 0.02%.
The diff coverage is 93.75%.

❗ Current head 1b91fb0 differs from pull request most recent head ad683fd. Consider uploading reports for the commit ad683fd to get more accurate results

@@            Coverage Diff             @@
##             main    #2875      +/-   ##
==========================================
- Coverage   84.22%   84.20%   -0.03%     
==========================================
  Files         250      251       +1     
  Lines        8883     8883              
==========================================
- Hits         7482     7480       -2     
- Misses       1401     1403       +2     

Impacted Files	Coverage Δ
src/OpenTelemetry/Internal/WildcardHelper.cs	88.88% <88.88%> (ø)
src/OpenTelemetry/Metrics/MeterProviderSdk.cs	87.02% <100.00%> (-0.11%)	⬇️
src/OpenTelemetry/Trace/TracerProviderSdk.cs	96.40% <100.00%> (-0.10%)	⬇️
...ter.ZPages/Implementation/ZPagesActivityTracker.cs	97.14% <0.00%> (-2.86%)	⬇️
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs	71.29% <0.00%> (-0.93%)	⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs	97.65% <0.00%> (+0.78%)	⬆️

[reyang]

1. Need changelog entry
2. I suppose we didn't have unit test for tracing's AddSource with wildcard?

Changelog is already included in the PR.
I'm working on the unit test for tracing part, got interrupted by other things, will update the PR later today.

[alanwest]

Do we need a to a contains check for * and ? now?

[reyang]

YES 👍

[reyang]

I've fixed this and added test cases.
Not sure if it'll be better to extract the s.Contains('*') || s.Contains('?') logic into a common helper, looks like it is currently used in 3 places.

[alanwest]

Not sure if it'll be better to extract the s.Contains('*') || s.Contains('?') logic into a common helper, looks like it is currently used in 3 places.

I think it'd be a decent minor improvement, so something like:

internal static bool ContainsWildcard(string s)
{
    return s.Contains('*') || s.Contains('?');
}

...

if (meterSources.Any(ContainsWildcard))

but, I'm ok 👍 without if you think it reads better without the helper

[reyang]

Maybe an extension method on String class? 😆

[alanwest]

That'd work too!

[reyang]

I've updated the PR by extracting the wildcard logics to WildcardHelper.

I didn't use String extension method because it could be too noisy (e.g. if someone is using Intellisense).

[alanwest]

I didn't use String extension method because it could be too noisy (e.g. if someone is using Intellisense).

Yea I think that's a good call. I was thinking that too, but thought maybe since it'd be internal it'd just affect us and not the whole 🌏

[alanwest]

LWTM (Looks wild to me)

[cijothomas]

Considering we are close to 1.2 stable release, i'll leave it open for a day to give more reviewers an opportunity to spot anything risky in this PR.