Remove unpatching functionality from instrumentation package

[rauno56]

Unpatching in many cases is a "lie" because to be able to do that we need indirection between the patched function and the caller. For functions we cannot assure that for, we either have to

• add a toggle into the instrumentation that dynamically decides whether to short-circuit into the original function or not based on whether the instrumentation is enabled during the call and/or(if the patched function is short-lived = less than the duration of the span)
• patch the function that returns the function we actually need to patch.

Even this:

const { readFileSync } = require('fs');
// now there's no way to unpatch but to add a toggle inside the instrumentation.

... renders unpatching useless. Requiring with de-structuring is an increasingly common pattern. What makes it even worse is that TS compiler currently hides that by referencing objects dynamically in the compiled asset:

// typescript:
import { readFileSync } from 'node:fs';
readFileSync(...);
// compiles into something like
const _temp_fs = require('node:fs');
_temp_fs.readFileSync(...);

Other languages don't have functionality to dynamically "unpatch", in many of them it's even unthinkable. The only use-case I know of are the tests if one has to run all tests in one process, but that can be mitigated by emptying the require cache between the tests and calling require again.

I propose we move towards removing it in the future(with a proper deprecation process, etc). WDYT?

Additional questions:

1. Which are the use cases unpatching is required for?
2. Could we live without any toggling mechanism(instrumentation enable/disable) and say that needs to be decided on boot and is then immutable. Note that it doesn't necessarily mean that whether the application is exporting telemetry data or not is immutable, but it just needs to be done in a different part of the pipeline.

• Deprecate unpatching,
• Adding a test utility that cleans the require cache to contrib,
• Discourage implementing unpatching functionality,
• Decide if we need disable and how it should be implemented across instrumentations,
• Removing unpatching altogether

[Flarna]

There were discussions about this in the past, see #618 and #732.
As you can read there I was never a fan of unpath for the same reasons you write here so fine for me to remove it.

As instrumentation and instrumentations are still experimental no deprecation cylce is needed usually. But we might want some replacement for tests.

I think two flags per instrumentation make sense:

• instrument if true instrument if false don't instrument. Clearly changing this flag for a running application makes no sense
• active if true create spans,... if false pass through everything unmodified, act like if you were not there. changeable at runtime at any time.

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stale for 14 days with no activity.