jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.02k stars 6.42k forks source link

hooking to jest require mechanism #11295

Closed blumamir closed 1 year ago

blumamir commented 3 years ago

Hello jest people and thanks for open-sourcing and maintining this delightful framework.

I am working on an auto-instrumentation library for jest for the opentelemetry project, to support automatic collection of traces while running local/distributed tests with jest. By collecting traces, a developer can examine internal and remote events that occurred for each test, which can aid debugging (and maybe in the future also trace-based assertions).

to implement the "auto-instrumentation" magic, open-telemetry-js sdk is using [require-in-the-middle] package to be notified on module imports and patch instrumented libraries. This is done by modifying the moudleExports object before it is returned to the "require"ing app. To my understanding, jest is replacing the "require" implementation as well, in a way that breaks require-in-the-middle integration.

Is there any mechanism in jest that can be used to register and get notified on requires that jest is performing? Tried to look into the implementation in jest-runtime package, but couldn't spot anything relevant.

Maybe jest can be configured to use the original nodejs require function? (hoping it will not break anything else in the framework)

Any help on that will be very appreciated

Other discussions relevant to this issue: https://github.com/facebook/jest/issues/7722 https://github.com/elastic/require-in-the-middle/issues/50

blumamir commented 3 years ago

Another option that crossed my mind is for "require in the middle" to somehow patch the jest require function like it is currently doing:

  this._require = Module.prototype.require = function (id) {

Maybe the Module.prototype.require patching can be replaced with a relevant jest object that exposes the same interface? Can someone experienced with jest recommend what this object and function might be?

SimenB commented 3 years ago

Using the "real" require is not gonna happen as that breaks the sandbox, mocking, transpilation etc (basically all that makes Jest different from Mocha or Jasmine bar watch mode).

What you could do here is use a custom transformer. You'd then insert source code which does the instrumentation. This would also work with native ESM, which a require based approach cannot.

If a custom transformer is not feasible (i.e. you want people to be able to use TS or whatever) you'll probably need to plug in your own runtime as you briefly mention (option is called moduleLoader). This is undocumented, but I'd start by extending the one exported from jest-runtime. In there you can add code to either _loadModule (for CJS) or loadEsmModule (for ESM) to do whatever you want. I'm not sure how much you can really do, though. Please experiment! 🙂

FauxFaux commented 3 years ago

You can work around require with a jest-runtime, and you can get the tracing code called. However, it still doesn't produce spans, for me. The tracing code believes there's no active context, so creates NoopSpans. I wonder if jest is also interfering with the async context (I'm on 14 so using the 'modern' provider), or if the library is fundamentally broken with Contexts.

Some experiments https://github.com/FauxFaux/jest-otel , but you can't really see anything unless you breakpoint around https://github.com/open-telemetry/opentelemetry-js/blob/c0b021be5782ebee6a23015c992dc496b105dad2/packages/opentelemetry-instrumentation-http/src/http.ts#L537-L538 (node_modules/@opentelemetry/instrumentation-http/build/src/http.js:336), all the logging is broken too.

Wild guess: http module is in a different Context (i.e. globals are different?), so it can't find itself, so it does nothing. I don't understand Contexts nor what otel is trying to do, enough to be sure about that.

(Or, much more likely, the comically awful hack is wrong.)

SimenB commented 3 years ago

Tests are definitely running in a different context (vm.Context). Globals in node core libraries will be different (#2549, https://github.com/nodejs/node/issues/31852), but globals in user code is consistent

FauxFaux commented 3 years ago

Thinking out loud here. otel needs to be able to look up various components in the globals object.

Options are: 1) runtime's constructor initialises otel (in the "outside-tests" Context), and ... sets it ... so the test's otel code can see it? So .. adds a global function in the inside-tests globals (or attaches it to .. the test object) which you can get the real globals from, and then .. get otel to use that? 2) when we patch the http client library, we ... get access to the inside-tests' Context's globals, and reference them from the patch, otel doesn't look like it supports this at all, tonnes of globals references all over the place 3) when we patch the http client library we .. pass in the test's globals, and copy them on to the outside-tests' globals.

First thought on 3, make the prelude be function (blah, blah, require) { require = () => require(globals, ...arguments);, which I'm sure has some other unbelievably horrible edge cases.

SimenB commented 3 years ago

this._environment.global from runtime will be the global in tests.

3 sounds impossible unless you never run tests in parallel again as different tests will override what's in the core module (as that's outside the sandbox)

blumamir commented 3 years ago

Thanks for taking the time to comment and research the issue 👍

What you could do here is use a custom transformer. You'd then insert source code which does the instrumentation. This would also work with native ESM, which a require based approach cannot.

This sounds to me like the most robust solution which also requires minimal changes to setup. Do you know any resources that explain how it works maybe? is it possible to concatenate transformers so I can run my own transformer and then run whatever other transformer user has configured?

You can work around require with a jest-runtime, and you can get the tracing code called.

Cool, I wasn't aware that private functions can be overridden in subclasses this way. definitely can be useful, although dangerous.

The tracing code believes there's no active context, so creates NoopSpans

It will not surprise me if the async context will also need some adjustments to jest. Currently, I'm trying to resolve the auto-instrumentation loading and patching issue, hope to look deeper into your example later.

Thinking out loud here. otel needs to be able to look up various components in the globals object.

Now that you bring it up, the global context will probably also break opentelemetry as it will sometimes use the VM global and sometimes the core global. good catch.

Looks like this issue is much more complicated than I anticipated. I'll move to other tasks in the meantime and try to crack it again later.

FauxFaux commented 3 years ago

3 sounds impossible unless you never run tests in parallel again as different tests will override what's in the core module (as that's outside the sandbox)

The http's "outside-tests" context would be different in different workers (as they're different node processes), and each worker is only running one test suite at a time? I'm okay with it only working on a test-suite basis, and/or the test-suite having to handle it if it want to change the context on a per-test basis.

FauxFaux commented 3 years ago

It appears that enough of the state is stored in the API package, which is designed for abuse. You can just directly expose it to tests, like https://github.com/FauxFaux/jest-otel/commit/82d11eb5deb95788d00712650293583cb322f3f5 , or presumably by fiddling with one of the other require calls.

With that, I get HTTP spans from the needle test, although not from the http test (I think it finishes too quickly, and stop() isn't sufficiently flushing it, not a jest/jest-otel problem).

FauxFaux commented 3 years ago

Can use the same technique to provide a working mock for enough of require-in-the-middle, making the runtime not depend on test code(!). https://github.com/FauxFaux/jest-otel/commit/914da973fcd52b188d360f6091382b4a6a9b9788

alper commented 2 years ago

@blumamir What happened here? Do you have a wip somewhere?

alper commented 2 years ago

@blumamir What happened here? Do you have a wip somewhere?

blumamir commented 2 years ago

@blumamir What happened here? Do you have a wip somewhere?

Unfortunately, I was not able to integrate them together. Spent a decent amount of time browsing jest implementation and trying to patch it somehow but had no luck :\

blumamir commented 2 years ago

@FauxFaux I now see that for some reason I missed your comments on this issue. Thanks for taking the time and effort to look into this. I'm having busy days with other tasks at the moment but will review your suggestions soon I hope.

alper commented 2 years ago

I actually don't think I'm that interested in the auto instrumentation as much as I would probably like a jest plugin that emits spans for a test run.(describe/test etc. with success and failure marked).

I think you're after also having the contents of each test traced? While that would definitely be nice, I'd settle for the granularity of just the test suite. That should be fairly doable with jest-plugins, no?

blumamir commented 2 years ago

Yeah I was looking to also have the test content traced with the existing auto instrumentation libraries

FauxFaux commented 2 years ago

I was definitely looking for the test content to be traced, my usecase was that I wanted to use our existing prod performance tooling to investigate tests.

renchap commented 2 years ago

@alper I stumbled upon this blog post by the Sentry team: https://blog.sentry.io/2021/07/27/instrumenting-our-frontend-test-suite-and-fixing-what-we-found

They are using a custom Jest environment (https://github.com/billyvg/jest-sentry-environment/) to receive the various test lifecycle events and create spans as needed. If you are looking on getting a trace per test run, with spans for each test, it seems to be the way to go.

GraemeF commented 2 years ago

I think someone has had a crack at this, but there are no docs and it seems to be published from a private repo: https://www.npmjs.com/package/@heliosphere/opentelemetry-instrumentation-jest

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

github-actions[bot] commented 11 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.