Marc Hassan

Results 62 comments of Marc Hassan

I've moved it out of Draft. I see some failing checks that I will address now.

I've cleaned up the PR and added more tests. This is ready for review.

Unit tests for `@opentelemetry/instrumentation-http` are failing in the pipeline and on my machine. I am looking into it. It seems to be a timing issue between when the `require-in-the-middle` singleton...

This PR seems to introduce a change in behavior that may not be acceptable without a fix (and this is the reason why the `@opentelemetry/instrumentation-http` tests are failing). Assume this...

Maybe a reasonable solution would be to instantiate RITM only when the first instance of `InstrumentationBase` is created. With that change in place, I see another problem: `@opentelemetry/instrumentation-http` test suites...

We could figure out how to remove it from the RITM cache, although we would need to reach inside its internal cache to do that. Also, RITM caches by filename,...

To be clear, `disable` does what we expect (it removes the instrumentation patch); this is the scenario that does not work: ```js const instrumentation1 = new HttpInstrumentation() instrumentation1.enable() require('http') //...

This is a hack that gets the tests passing locally: ```ts public disable(): void { // ... if (process.env.npm_lifecycle_script?.startsWith('nyc ts-mocha')) { // @ts-expect-error TS2339: Property 'cache' does not exist on...

This PR supports multiple instrumentations of the same modules, as long as they are all enabled before the first `require` that gets hooked by RITM. The problem we are having...