Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move InstrumentationLibrary key generation out of TracerProvider #619

Closed
tidal opened this issue Mar 19, 2022 · 1 comment · Fixed by #663
Closed

Move InstrumentationLibrary key generation out of TracerProvider #619

tidal opened this issue Mar 19, 2022 · 1 comment · Fixed by #663
Assignees
@ditsuke
Labels
bite sized This is a small chunk of work (good for new or time limited contributors!) good first issue Good for newcomers help wanted This issue is looking for someone to work on it refactoring Doesn't change behavior, but improves performance or maintainability or removes technical debt.

Comments

@tidal
Copy link
Member

tidal commented Mar 19, 2022 •
edited

The generation of a key for a InstrumentationLibrary should be moved out of the TracerProvider class into a utility class in SDK/Common/Util, since this funtionality is not only required in the Trace component/signal.

  • Move key generation logic in TracerProvider::getTracer into a dedicated Util class
@tidal tidal added help wanted This issue is looking for someone to work on it good first issue Good for newcomers bite sized This is a small chunk of work (good for new or time limited contributors!) refactoring Doesn't change behavior, but improves performance or maintainability or removes technical debt. labels Mar 19, 2022
@ditsuke
Copy link
Contributor

ditsuke commented Apr 14, 2022

@tidal I'd like to be assigned.

@ditsuke ditsuke mentioned this issue Apr 14, 2022
Merged
@tidal tidal linked a pull request Apr 15, 2022 that will close this issue
Move InstrumentationLibrary key generation logic out of TracerProvider/SpanConverter [#619] #663
Merged
tidal pushed a commit that referenced this issue Apr 15, 2022
@ditsuke
Move InstrumentationLibrary key generation logic out of TracerProvide…
6d99358 
…r/SpanConverter [#619] (#663)

* Extract InstrumentationLibrary key generation logic

Factors key generation logic for InstrumentationLibrary instances
out to a helper method in a utils class.

* Remove @see PHP annotation

Phan doesn't like it, couldn't see an acceptable workaround.

* Change key generator namespace and class

Changes the key generator's namespace to Util\Instrumentation and classname to
KeyGenerator, refactoring usages.

* Add unit tests for Instrumentation key generator

* Add test case for non-null version and null schema

Also renames the other 2 tests for descriptiveness.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ditsuke ditsuke

Labels
bite sized This is a small chunk of work (good for new or time limited contributors!) good first issue Good for newcomers help wanted This issue is looking for someone to work on it refactoring Doesn't change behavior, but improves performance or maintainability or removes technical debt.
Projects
None yet
Milestone
No milestone
2 participants
@tidal @ditsuke