opencensus-node icon indicating copy to clipboard operation
opencensus-node copied to clipboard

Fix circular import

Open StoneDot opened this issue 5 years ago • 4 comments

This PR fixes the circular import problems occurring when you load http2 or https module before http module.

For example, the following code reproduces a problem @opencensus-instrumentation-http module doesn't load properly.

const tracing = require('@opencensus/nodejs');
const zipkin = require('@opencensus/exporter-zipkin');
const assert = require('assert');

const tracer = tracing.start({samplingRate: 1.0}).tracer;

const https = require('https');
const http = require('http');

tracer.registerSpanEventListener(new zipkin.ZipkinTraceExporter({
    url: 'http://localhost:9411/api/v2/spans',
    serviceName: 'node.js-open-census-bug'
}));

http.get('http://example.com/', (res) => {
  res.setEncoding('utf8');
  let rawData = '';
  res.on('data', (chunk) => { rawData += chunk; });
  res.on('end', () => {
    console.log(rawData);
  });
});

The process of problem is as follows schematically:

  1. Load http2 module.
  2. require-in-the-middle load @opencensus-instrumentation-http2 module.
  3. @opencensus-instrumentation-http2 load @opencensus-instrumentation-http module.
  4. @opencensus-instrumentation-http load http.
  5. require-in-the-middle try to load @opencensus-instrumentation-http, but loading of @opencensus-instrumentation-http is already in progress. Therefore require statement return partial @opencensus-instrumentation-http.
  6. The problem happens.

StoneDot avatar Aug 27 '19 22:08 StoneDot

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

googlebot avatar Aug 27 '19 22:08 googlebot

@googlebot I signed it!

StoneDot avatar Aug 27 '19 22:08 StoneDot

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

googlebot avatar Aug 27 '19 22:08 googlebot

@draffensperger and @hekike what do you think about this?

mayurkale22 avatar Sep 13 '19 17:09 mayurkale22