Move InstrumentationLibrary key generation logic out of TracerProvider/SpanConverter [#619]

[ditsuke]

PR for Issue #619.

Factors key generation logic for InstrumentationLibrary instances out to a helper method in a utils class, replacing generation in TracerProvider and SpanConverter with calls to the new method.

[welcome]

Thanks for opening your first pull request! If you haven't yet signed our Contributor License Agreement (CLA), then please do so that we can accept your contribution. A link should appear shortly in this PR if you have not already signed one.

[ditsuke]

Could use some feedback on the new Helpers class, can we do a better name?

[brettmc]

@ditsuke welcome aboard! re: class naming, I think SDK\Common\Instrumentation is a better location. As to class name, KeyGenerator? It will also need unit tests.

[codecov]

Codecov Report

Merging #663 (0f3665e) into main (7ce599b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #663      +/-   ##
============================================
+ Coverage     84.27%   84.28%   +0.01%     
- Complexity     1152     1153       +1     
============================================
  Files           127      128       +1     
  Lines          2786     2788       +2     
============================================
+ Hits           2348     2350       +2     
  Misses          438      438              

Flag	Coverage Δ
7.4	84.23% <100.00%> (+0.01%)	⬆️
8.0	84.28% <100.00%> (+0.01%)	⬆️
8.1	84.28% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Contrib/Otlp/SpanConverter.php	100.00% <100.00%> (ø)
src/SDK/Common/Instrumentation/KeyGenerator.php	100.00% <100.00%> (ø)
src/SDK/Trace/TracerProvider.php	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ce599b...0f3665e. Read the comment docs.

[ditsuke]

@ditsuke welcome aboard! re: class naming, I think SDK\Common\Instrumentation is a better location. As to class name, KeyGenerator? It will also need unit tests.

@brettmc thanks! I've moved and renamed the generator and added unit tests, PTAL

[tidal]

@ditsuke
Thank you. LGTM
I have added one little comment for a test you could add, just to make sure.
However other than t, I think we can merge the PR, if you are ok with it.

[ditsuke]

@ditsuke Thank you. LGTM I have added one little comment for a test you could add, just to make sure. However other than t, I think we can merge the PR, if you are ok with it.

@tidal thanks for reviewing. I've added the third test with 0f3665e

[ditsuke]

Thanks!