-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
Remove last dependencies on SDK package, improve version detection #461
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @raphael-theriault-swi please see my comments. One other question i'll ping you tomorrow on is regarding the HostID and __Init message when in OTLP mode.
@@ -14,363 +14,296 @@ See the License for the specific language governing permissions and | |||
limitations under the License. | |||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, the big payoff--just skimmed this file but seeing what you did to get waitUntilReady working for this OTLP/AO switch gave me hint of the effort that went into the rewrite, kudos!
For the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the explanation and revisits @raphael-theriault-swi! We'll tackle the HostID item as a follow on.
This is a pretty big change since all references to the old config types had to be removed. Now everything is self contained in the
solarwinds-apm
package. What remains after this is deleting the SDK package and any logic to target CommonJS.