fix(plugin-http): correct handling of WHATWG urls

[Flarna]

Which problem is this PR solving?

fixes #583

Short description of the changes

Add parsing and conversion of WHATWG URL objects for client http
requests to ensure semantics of HTTP request are not modifed and
tracestate header is correctly added.

[codecov-io]

Codecov Report

Merging #589 into master will increase coverage by 1.94%.
The diff coverage is 97.26%.

@@            Coverage Diff             @@
##           master     #589      +/-   ##
==========================================
+ Coverage   89.93%   91.88%   +1.94%     
==========================================
  Files         168      169       +1     
  Lines        8230     8215      -15     
  Branches      739      729      -10     
==========================================
+ Hits         7402     7548     +146     
+ Misses        828      667     -161

Impacted Files	Coverage Δ
packages/opentelemetry-plugin-http/src/http.ts	96.8% <100%> (-0.52%)	⬇️
...pentelemetry-plugin-http/test/utils/httpRequest.ts	95% <100%> (+4.09%)	⬆️
...lemetry-plugin-http/test/functionals/utils.test.ts	99.35% <100%> (ø)	⬆️
...y-plugin-http/test/functionals/http-enable.test.ts	96.1% <100%> (+0.01%)	⬆️
...-plugin-http/test/integrations/http-enable.test.ts	94.16% <100%> (+1.23%)	⬆️
packages/opentelemetry-plugin-http/src/utils.ts	98.43% <90.47%> (-1.57%)	⬇️
...telemetry-node/src/instrumentation/PluginLoader.ts	92.72% <0%> (ø)	⬆️
packages/opentelemetry-node/src/NodeTracer.ts	100% <0%> (ø)	⬆️
...es/opentelemetry-node/src/instrumentation/utils.ts	90.47% <0%> (-9.53%)	⬇️
.../opentelemetry-plugin-redis/test/assertionUtils.ts	95.83% <0%> (-4.17%)	⬇️
... and 60 more

[OlivierAlbertini]

Good work! Nice addition.

[OlivierAlbertini]

Feel free to do #590 otherwise I will do it.

[Flarna]

Do you mean as part of this PR or as followup?

[OlivierAlbertini]

Do you mean as part of this PR or as followup?

as followup

[obecny]

lgtm, I added few comments, thx

[dyladan]

LGTM

[dyladan]

@Flarna you just need to lerna run fix to fix the linting issues.

@mayurkale22 I wonder if there would be any value to adding lerna run check as a pre-commit hook using husky?

[mayurkale22]

@mayurkale22 I wonder if there would be any value to adding lerna run check as a pre-commit hook using husky?

It is possible to do that, but lerna run check takes around 20-30 secs to run. Also, no need to run check on all the packages every time, usually we touch 1-2 packages in the PR. WDYT?

[dyladan]

@mayurkale22 easy to fix :)

# only check packages changed in the PR
lerna run check --parallel --since master --exclude-dependents

# only check packages changed since last commit
lerna run check --parallel --since HEAD --exclude-dependents

[Flarna]

I was assuming that yarn run check only runs prettier as I reported only formatting stuff. I think the tslint rules should be stricter.
Looking into the code it seems they were stricter in the past as I can use any now without a tslint telling me that I should not (see https://github.com/open-telemetry/opentelemetry-js/pull/589/files#diff-033023ec74fe6fbd45196f184ba9aa10R47)

[Flarna]

I wonder if there would be any value to adding lerna run check as a pre-commit hook using husky?

Honestly speaking I would prefer to remove husky hooks and replace such checks by a CI/build task triggered on demand (e.g. like in node repo). During development I tent to do a log commits locally. At this time I think not that much about wording as I squash anyway before I create a the PR and go for a good wording. I know I can use --no-verify to bypass but it's cumbersome.

Usually the tslint plugin in VsCode or something similar gives faster and better feedback.

[dyladan]

Usually the tslint plugin in VsCode or something similar gives faster and better feedback.

This is my experience as well. For some reason though I haven't been able go get this project to work correctly with the vscode tslint plugin. I suspect it's either because of the lerna monorepo pattern or the fact that we're actually using gts which is the google specific typescript style checker.

[Flarna]

It works fine for me, at least if I open only a package. But it seems with current rule set it's difficult to get lint to complain; adding e.g. no-any immediately results in complains.

[dyladan]

I think the gts tool applies rules on top of the configured tslint configuration, so the tslint editor integration doesn't understand the gts specific rules.