feat: Added transformer injection

[jsumners-nr]

This PR adds the ability to inject your own transformer module. This allows for alternate transformers to be written, or for different versions of the one we maintain, to be used.

[timfish]

I have concerns about using an environment variable to allow arbitrary module loading.

For the sync hook the module name can be passed directly as an argument and for the async hook it can be passed in the initialize data!

[jsumners-nr]

I have concerns about using an environment variable to allow arbitrary module loading.

The included hook.mjs is a convenience script. The readme doesn't even mention it, and instead instructs people to write their own hook script. For these reasons, I don't see the issue. The env var is provided for completeness, and to help those who can get by with the convenience script.

For the sync hook the module name can be passed directly as an argument and for the async hook it can be passed in the initialize data!

I don't follow.

[timfish]

help those who can get by with the convenience script

Who are these people? This library is primarily consumed by the SDKs of APM providers.

We are unlikely to want to ship code that allows arbitrary code execution by setting an environment variable, especially when there other options.

If you need this specific feature can you not have your own hook wrapper?

[jsumners-nr]

We are unlikely to want to ship code that allows arbitrary code execution by setting an environment variable, especially when there other options.

What are the other options?

If you need this specific feature can you not have your own hook wrapper?

At the moment, the sole driver of this PR is to solve your woes with publishing new Orchestrion versions.

[timfish]

What are the other options?

initialize and initializeSync take parameters so there is no need to use environment variables.

the sole driver of this PR is to solve your woes with publishing new Orchestrion versions.

I can't really see how this relevant. This PR doesn't contain any breaking changes and could be published under a minor release.

[timfish]

Ah ok, just looked through this again after opening #81 and realised you are likely not talking about the transforms that can be applied per instrumentation?

This allows for alternate transformers to be written

It seems like this PR allows you to completely swap out the @apm-js-collab/code-transformer module at runtime via an environment variable? Can you explain the problem you're trying to solve? That way I might better understand why this is the only solution!

[jsumners-nr]

I can't really see how this relevant. This PR doesn't contain any breaking changes and could be published under a minor release.

With this change you do not have to release a new version of this module in order to supply a newer version of Orchestrion.

[jsumners-nr]

It seems like this PR allows you to completely swap out the @apm-js-collab/code-transformer module at runtime via an environment variable? Can you explain the problem you're trying to solve? That way I might better understand why this is the only solution!

I just changed the approach to a DI approach.

[timfish]

With this change you do not have to release a new version of this module in order to supply a newer version of Orchestrion.

So instead of releasing a v1 so every feature addition is not a semver breaking change, you're suggesting this as an alternative?

This is worse than releasing a v1 in every way:

• Users will still download and install 2 versions of the library but only one will get used
• Consumers now have to know about create, manage their own code-transformer version, and wire it through both call sites — this is more steps than the current version bump on every release.
• It adds an unversioned contract. The shape of the injected object is tied to code-transformer's internals, but isn't covered by tracing-hooks' own semver — a future internal refactor could silently break injectors.

I knew you were against releasing a v1 but I'm surprised you would suggest this as a viable alternative!

[jsumners-nr]

This is required if we are to ever support alternative transformers. The fact that it solves a current headache is a bonus. Issuing a v1 of everything right now introduces high probability of breakage from ongoing development of the overall architecture. The project simply is not ready for a v1.

[timfish]

This is required if we are to ever support alternative transformers.

Who is asking for this?

The fact that it solves a current headache is a bonus.

This doesn't solve any headaches regarding being pre v1.

As I've already said above, your solution is more steps than just bumping the version for every minor release. We would never use this feature because of the numerous reasons I've listed.

Issuing a v1 of everything right now introduces high probability of breakage from ongoing development of the overall architecture.

If we need to make breaking changes we can just release another major. The only cost to releasing a new major is that downstream consumers will need to manually bump their version and we're already doing that for every single minor release.