fix(instr-undici): fix issue with config in constructor

[david-luna]

Which problem is this PR solving?

Fixes: #2391

Short description of the changes

• removed enabled internal config from the logic of the instrumentation. Now it relies only in enable/disable methods
• updated tests

[david-luna]

Note for reviewer: here I was mixing UndiciInstrumentationConfig with InstrumentationConfig. This side effect shouldn't be there

[david-luna]

Note for reviewer: no need to call super since the parent class method deals with patching/unpatching and does not apply here

[trentm]

super.disable() is also setting this._enabled = false, so I wonder if it still should be called.
Those calls to super. in the enable() and disable() were added in this PR: https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2289/files#diff-8e6dd9963dd47a60e25c9ac92d7a5ae25ce79c40340cb4721eefd203bec81a60R86

[trentm]

I've restored the super.disable() and super.enable() calls, to allow InstrumentationBase to maintain its internal this._enabled setting.

[david-luna]

Note for reviewer: the test runner was loading all the test suites at once and also evaluating the 1st describe block. As a consequence there were 3 instances of the instrumentation before any test was ran. Leading to errors because all instances where getting messages from diagnostics channels

[trentm]

Nice find!

[codecov]

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.52%. Comparing base (dfb2dff) to head (e473df6).
Report is 217 commits behind head on main.

Files	Patch %	Lines
plugins/node/instrumentation-undici/src/undici.ts	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2395      +/-   ##
==========================================
- Coverage   90.97%   90.52%   -0.45%     
==========================================
  Files         146      157      +11     
  Lines        7492     7622     +130     
  Branches     1502     1571      +69     
==========================================
+ Hits         6816     6900      +84     
- Misses        676      722      +46     

Files	Coverage Δ
plugins/node/instrumentation-undici/src/undici.ts	95.08% <80.00%> (ø)

... and 74 files with indirect coverage changes

[trentm]

Slightly related, I've opened open-telemetry/opentelemetry-js#4941 to eventually deal with a confusion that can come from using instr.setConfig({}).

[trentm]

Here is my test script for manual testing of the usage via NodeSDK, with getNodeAutoInstrumentations (as shown in the original reported issue), and without getNodeAutoInstrumentations in a few different cases:

use-undici-request.js

const path = require('path');
const {NodeSDK} = require('@opentelemetry/sdk-node');
const {getNodeAutoInstrumentations} = require('@opentelemetry/auto-instrumentations-node');
const {UndiciInstrumentation} = require('./');

// Instrument undici.
process.env.OTEL_TRACES_EXPORTER = 'console'
const sdk = new NodeSDK({
  serviceName: path.parse(process.argv[1]).name,
  instrumentations: [
    // 1.
    // new UndiciInstrumentation(),
    // 2.
    // new UndiciInstrumentation({enabled: false}),
    // 3.
    // new UndiciInstrumentation({enabled: true}),
    // 4.
    getNodeAutoInstrumentations({
      '@opentelemetry/instrumentation-undici': {
        enabled: true,
      },
    }),
  ],
});
process.once('beforeExit', () => {
  sdk.shutdown();
});
sdk.start();

// Use undici.
const undici = require('undici');
async function main() {
  const res = await undici.request('http://www.google.com/');
  console.log('response: %s', res.statusCode);
  const body = await res.body.text();
  console.log('response body: %s...', body.slice(0, 40));
}
main();

All four cases work as I'd expect. Case 3 results in no undici span; all other cases result in an undici span.

[trentm]

All four cases work as I'd expect.

... they work after I get the commit in that I had. Still trying to get that pushed.