fix: dont trace ourselves

[OlivierAlbertini]

Which problem is this PR solving?

• closes Avoid infinity loop when exporter use http to send spans #332

Short description of the changes

• Avoid to trace outgoing request coming from exporters

UPDATE:

• added more tests to explain the behaviour in http-plugin
• add a test to Zipkin

[mayurkale22]

Added a minor comment, otherwise lgtm

[codecov-io]

Codecov Report

Merging #335 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #335      +/-   ##
==========================================
+ Coverage    97.9%   97.94%   +0.03%     
==========================================
  Files         105      103       -2     
  Lines        5021     4965      -56     
  Branches      423      419       -4     
==========================================
- Hits         4916     4863      -53     
+ Misses        105      102       -3

Impacted Files	Coverage Δ
...ckages/opentelemetry-exporter-zipkin/src/zipkin.ts	100% <100%> (+1.78%)	⬆️
...ackages/opentelemetry-exporter-zipkin/src/utils.ts	100% <100%> (ø)
packages/opentelemetry-plugin-http/src/utils.ts	98.07% <100%> (+0.11%)	⬆️
...y-plugin-http/test/functionals/http-enable.test.ts	98.26% <100%> (+0.09%)	⬆️
...lemetry-plugin-http/test/functionals/utils.test.ts	99.3% <100%> (+0.05%)	⬆️
.../opentelemetry-exporter-zipkin/test/zipkin.test.ts	100% <100%> (ø)	⬆️
packages/opentelemetry-plugin-http/src/http.ts	96.38% <100%> (+0.04%)	⬆️
...ages/opentelemetry-core/src/platform/node/index.ts	100% <0%> (ø)	⬆️
...opentelemetry-core/src/platform/node/timer-util.ts
... and 4 more

[danielkhan]

@OlivierAlbertini do you think it would make sense to remove the header after it has been acknowledged by the HTTP plugin?
My rationale is that it has served its purpose and if we pass it downstream someone might have an idea to add some kind of logic that checks for this header to be present.

Other than that LGTM

[OlivierAlbertini]

@OlivierAlbertini do you think it would make sense to remove the header after it has been acknowledged by the HTTP plugin?
My rationale is that it has served its purpose and if we pass it downstream someone might have an idea to add some kind of logic that checks for this header to be present.

Other than that LGTM

I think that Utils.isOpenTelemetryRequest(options) will be quite used with Zipkin exporter. Removing the header add an operation (we could check the impact with a benchmark) and it could be more difficult to troubleshoot if there is some issues.

Other than that, I'm open to do it if everyone is agree. Feel free to open an issue and/or discuss about that in the SIG meeting.

[dyladan]

Should we save the OT_REQUEST_HEADER somewhere centralized so that https://github.com/open-telemetry/opentelemetry-js/pull/335/files#diff-d93b06c50896a34498308e756882b287R17 and https://github.com/open-telemetry/opentelemetry-js/pull/335/files#diff-05d5f58877593b982347e35d3bf78928R33 don't become out of sync? I just don't know which package it would go in as there are no packages other than types that are a shared dependency of the Zipkin exporter and the http plugin. This same header will also be required for the https and http2 plugins if I understand correctly.