feat(instrumentation): add support for esm module #2846

vmarchaud · 2022-03-20T09:33:28Z

Re-opening following #2640 + #2763

I'm marking this as draft since we have a semi problem with this: its really not easy to test this. Granted there wasn't a lot of test in place before that for require in the middle but i think it make sense to do it anyway.
I've looked around and if we want to run in esm mode and verify that we can correctly hook esm module, we need to mark the package itself as being ESM: mochajs/mocha-examples#47 (comment)
I'm pretty sure doing this will cause downstream problem for users so i don't think this is the way to go, do someone have an idea ?

codecov · 2022-03-20T09:36:31Z

Codecov Report

Merging #2846 (992c18c) into main (d1621d2) will increase coverage by 0.41%.
The diff coverage is n/a.

❗ Current head 992c18c differs from pull request most recent head f6384aa. Consider uploading reports for the commit f6384aa to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2846      +/-   ##
==========================================
+ Coverage   93.26%   93.67%   +0.41%     
==========================================
  Files         247      242       -5     
  Lines        7346     7181     -165     
  Branches     1512     1477      -35     
==========================================
- Hits         6851     6727     -124     
+ Misses        495      454      -41

Impacted Files	Coverage Δ
...n/src/platform/node/RequireInTheMiddleSingleton.ts
...src/platform/node/instrumentationNodeModuleFile.ts
...nstrumentation/src/platform/node/ModuleNameTrie.ts
...atform/node/instrumentationNodeModuleDefinition.ts
...strumentation/src/platform/node/instrumentation.ts

Flarna · 2022-03-21T07:33:02Z

Why is it needed to mark @opentelemetry/instrumentation as ESM? Is it needed only for our tests or also for production?
I think datadog APM uses import-in-the-middle but I don't think it's marked as module.

vmarchaud · 2022-03-21T08:49:50Z

Is it needed only for our tests or also for production?

Apparently for tests only but i didn't test it yet

vmarchaud · 2022-04-18T16:39:05Z

Added a simple test that patches a esm module and verify that we get the patched version, however making it work with our typescript tooling was pretty much a nightmare. Would like input of everyone to discuss if we ship it like this or we want to do more ?

PS: Wil fix tests for older node version / changelog after settling above comment

cc @open-telemetry/javascript-approvers

experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts

vmarchaud · 2022-04-24T15:05:29Z

I've fixed the tests (sorry for the bash hack, i didn't see another way to not run those tests for node < 12) and added a comment in the readme to explain that user need to use choose the iitm loader so we can hook into modules

rauno56

Good job! I didn't expect this to require so few changes.

sorry for the bash hack, i didn't see another way to not run those tests for node < 12

I love a good bash hack(sarcasm), but if you don't, an less-bash-hacky alternative could be something like:

node -e 'process.exit(process.versions.node.split(".")[0] >= 12)' || echo "running it"

Q: Have you tried running any contrib package against this?

...tal/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/package.json

experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts

vmarchaud · 2022-04-27T06:26:15Z

Have you tried running any contrib package against this?

Well i believe most of our instrumentation target cjs module so i didn't try, do you have one in mind that have a esm bundle ?

I love a good bash hack(sarcasm), but if you don't, an less-bash-hacky alternative could be something like:

I prefer your suggestion, i will update my PR

dyladan

Seems fine to me. Overall less changes than I expected. Thanks for doing the legwork on this.

experimental/packages/opentelemetry-instrumentation/package.json

dyladan · 2022-05-03T20:58:54Z

experimental/packages/opentelemetry-instrumentation/README.md

@@ -261,6 +261,10 @@ If nothing is specified the global registered provider is used. Usually this is
 There might be usecase where someone has the need for more providers within an application. Please note that special care must be takes in such setups
 to avoid leaking information from one provider to the other because there are a lot places where e.g. the global `ContextManager` or `Propagator` is used.

+## ESM within Node.JS


I wonder if we should mark this explicitly an experimental feature and solicit feedback? Might be nice to know if this is actually working for people or if it is being used once its released.

4437550 (#2846)

experimental/packages/opentelemetry-instrumentation/package.json

dyladan · 2022-05-03T21:03:41Z

Dismissed my approval because we need to make sure a basic npm test works for windows users. This means either making test:esm work in windows, only running test:cjs for a basic test script, or both.

rauno56 · 2022-05-04T07:33:15Z

Well i believe most of our instrumentation target cjs module so i didn't try, do you have one in mind that have a esm bundle ?

I assumed the new paths will also kick in if my application is esm, but not the library. Isn't that the case?

vmarchaud · 2022-10-30T17:45:36Z

All that activity over #1946 motivated me to take some time from my holiday to get back to this. Good news actually some of the issues i've had has been resolved by other PRs:

feat(instrumentation): add support for esm module #2846 (comment): no longer needed with newer IITM version
feat(instrumentation): implement require-in-the-middle singleton #3161 extracted the logic to a singleton so i've moved the logic there too
feat(instrumentation): add support for esm module #2846 (comment) we dropped support for node 12 so the hack to run esm only when available isnt needed anymore

I assumed the new paths will also kick in if my application is esm, but not the library. Isn't that the case?

From my understanding, the new path for esm only kicks in when the ESM module loader is used so for ESM modules, you can find more information there: https://nodejs.org/dist/latest-v18.x/docs/api/packages.html#determining-module-system

weyert · 2022-10-31T02:13:41Z

Thank you for taking time of your holiday working on this. Let me know, how I can help. I think that agitated is a popular ESM package that could be a test target?

Qard · 2022-10-31T06:43:15Z

experimental/packages/opentelemetry-instrumentation/README.md

@@ -263,6 +263,11 @@ If nothing is specified the global registered provider is used. Usually this is
 There might be usecase where someone has the need for more providers within an application. Please note that special care must be takes in such setups
 to avoid leaking information from one provider to the other because there are a lot places where e.g. the global `ContextManager` or `Propagator` is used.

+## Instrumentation for ESM Module In NodeJS (experimental)
+
+As the module loading mechanism for ESM is different than CJS, you need to select a custom loader so instrumentation can load hook on the esm module it want to patch. To do so, you must provide the `--experimental-loader=import-in-the-middle/hook.mjs` flag to the `node` binary, alternatively you can set the `NODE_OPTIONS` environment variable to `--experimental-loader=import-in-the-middle/hook.mjs`.


I would recommend creating a wrapper for the hook so you can have a loader hook name that's clearly in the otel namespace rather than the unfamiliar import-in-the-middle naming. That's what we do at Datadog. We tell users to use dd-trace/loader-hook.js which just loads the IITM hook: https://github.com/DataDog/dd-trace-js/blob/master/loader-hook.mjs

Thanks for the suggestion, i would like other opinion on this since this would result in using --experimental-loader=@opentelemetry-instrumentation/hook.mjs instead. Also since the instrumentation is built for multiple platforms i'm not sure this would not create more problems as we support more (deno comes to my mind)

The hook could be a separate package. My suggestion is just to make the connection to otel clearer in some way.

@open-telemetry/javascript-maintainers I would like in put on this, i'm not sure what's the best way to do this:

have a dedicated package that re-export ittm

exporting it from the instrumentation package but it need to be excluded from the browser build, however i'm not sure where to place the file in this case ? inside /plaform/node ? at the root ? Since user would need to require it directly, that means we need it to be at the root of build

Between those options i'd rather have a dedicated package. Honestly though I think it's fine to just use RITM directly. It's just one more thing for us to build, maintain, and publish

I can guess that @Qard being at the other end by maintaining ITTM doesn't want to get questions about ESM instrumentation within otel but maybe i'm mistaken ? I agree that its more complicated to have one empty package on our end just for this.

--experimental-loader=@opentelemetry-instrumentation/hook.mjs looks good to me. I didn't find a new dedicated package necessary here. The hook entry files to be published will be rather small. Reducing the number of OTEL packages can alleviate the burden of setting up an OTEL application.

--experimental-loader=@opentelemetry-instrumentation/hook.mjs looks good to me. I didn't find a new dedicated package necessary here. The hook entry files to be published will be rather small. Reducing the number of OTEL packages can alleviate the burden of setting up an OTEL application.

Do you mean --experimental-loader=@opentelemetry/instrumentation/hook.mjs

My concern is more just users being confused about needing --experimental-loader=import-in-the-middle/hook.mjs which is loading some other module than opentelemetry which they are likely unfamiliar with. Having an opentelemetry-specific proxy to it makes it clearer that this is a thing opentelemetry wants from the user. It also doesn't leave you tied to some specific external library you don't control. If at some point in the future IITM no longer does what you need or maintenance dies or whatever else, you can simply change the contents of your hook file without needing to ask all your users to update their CLI args. Changing CLI args sounds like it should be a trivial thing, but stuff like that often gets encoded deep in infra configs and places like that which creates unnecessary friction if changes need to be made in the future. It's just a better idea to have an entrypoint for that which you control.

I would also choose the way of having own wrapper for loading a non-core external library. If we were speaking about browser, it would probably make sense to keep the external dep as is to reduce the generated client's bundle size in case this lib is reused outside of opentelemetry as well. For node it's irrelevant, there are no cons of wrapping it

vmarchaud · 2022-10-31T07:30:58Z

Thank you for taking time of your holiday working on this. Let me know, how I can help. I think that agitated is a popular ESM package that could be a test target?

My simple unit test shows that you can mutate exports with the PR so yeah i think once merged it would be nice to have a esm-only instrumentation created

weyert · 2022-10-31T14:35:55Z

Thank you for taking time of your holiday working on this. Let me know, how I can help. I think that agitated is a popular ESM package that could be a test target?

My simple unit test shows that you can mutate exports with the PR so yeah i think once merged it would be nice to have a esm-only instrumentation created

I think the got package is an interesting test case as its a package that is widely used 11.8.5 which still CommonJS while the >= 12.0.0 became a ESM pure module. I think both versions are still heavily used

legendecas

Still LGTM.

legendecas · 2022-11-15T06:18:19Z

experimental/packages/opentelemetry-instrumentation/README.md

@@ -263,6 +263,11 @@ If nothing is specified the global registered provider is used. Usually this is
 There might be usecase where someone has the need for more providers within an application. Please note that special care must be takes in such setups
 to avoid leaking information from one provider to the other because there are a lot places where e.g. the global `ContextManager` or `Propagator` is used.

+## Instrumentation for ESM Module In NodeJS (experimental)
+
+As the module loading mechanism for ESM is different than CJS, you need to select a custom loader so instrumentation can load hook on the esm module it want to patch. To do so, you must provide the `--experimental-loader=import-in-the-middle/hook.mjs` flag to the `node` binary, alternatively you can set the `NODE_OPTIONS` environment variable to `--experimental-loader=import-in-the-middle/hook.mjs`.


--experimental-loader=@opentelemetry-instrumentation/hook.mjs looks good to me. I didn't find a new dedicated package necessary here. The hook entry files to be published will be rather small. Reducing the number of OTEL packages can alleviate the burden of setting up an OTEL application.

jeremysf · 2022-11-28T05:36:20Z

I gave this a PR try on node v18.12.1. I know it's totally work in progress, but just in case it's helpful, I thought I'd post my results here.

Short version: I'm not having much luck :/

$ node --experimental-loader=import-in-the-middle/hook.mjs dist/user.mjs
(node:17) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
@opentelemetry/api: Registered a global for diag v1.3.0.
Exporter "otlp" requested through environment variable is unavailable.
@opentelemetry/api: Registered a global for trace v1.3.0.
@opentelemetry/api: Registered a global for context v1.3.0.
@opentelemetry/api: Registered a global for propagation v1.3.0.
@opentelemetry/instrumentation-express Module express has been loaded before @opentelemetry/instrumentation-express so it might not work, please initialize it before requiring express
@opentelemetry/instrumentation-grpc Module @grpc/grpc-js has been loaded before @opentelemetry/instrumentation-grpc so it might not work, please initialize it before requiring @grpc/grpc-js
@opentelemetry/instrumentation-mysql2 Module mysql2 has been loaded before @opentelemetry/instrumentation-mysql2 so it might not work, please initialize it before requiring mysql2
@opentelemetry/instrumentation-aws-sdk Module aws-sdk has been loaded before @opentelemetry/instrumentation-aws-sdk so it might not work, please initialize it before requiring aws-sdk
@opentelemetry/instrumentation-redis Module redis has been loaded before @opentelemetry/instrumentation-redis so it might not work, please initialize it before requiring redis
@opentelemetry/instrumentation-memcached Module memcached has been loaded before @opentelemetry/instrumentation-memcached so it might not work, please initialize it before requiring memcached
Patching mysql@2.3.3
Patching Connection.prototype.query
MySQL2Instrumentation: patched mysql query/execute
MySQL2Instrumentation: patched mysql query/execute
items to be sent [
  Span {
    attributes: {
      'db.system': 'mysql',
...

I'm using esbuild to bundle my TypeScript app's source code, but am excluding all node_modules from bundling (so I'm just bundling my app's transpiled TypeScript source code).

Despite the debug error logs above, I am not importing anything before loading the instrumentations as can be seen from the source of my bundled app:

import {createRequire} from 'module';const require=createRequire(import.meta.url);
var __defProp = Object.defineProperty;
var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
var __export = (target, all) => {
  for (var name in all)
    __defProp(target, name, { get: all[name], enumerable: true });
};
var __decorateClass = (decorators, target, key, kind) => {
  var result = kind > 1 ? void 0 : kind ? __getOwnPropDesc(target, key) : target;
  for (var i = decorators.length - 1, decorator; i >= 0; i--)
    if (decorator = decorators[i])
      result = (kind ? decorator(target, key, result) : decorator(result)) || result;
  if (kind && result)
    __defProp(target, key, result);
  return result;
};

// backend/common/tracing/initialize.ts
import * as openTelemetryAPI from "../node_modules/@opentelemetry/api/build/src/index.js";
import { registerInstrumentations } from "../node_modules/@opentelemetry/instrumentation/build/src/index.js";
import { NodeTracerProvider } from "../node_modules/@opentelemetry/sdk-trace-node/build/src/index.js";
import { OTLPTraceExporter } from "../node_modules/@opentelemetry/exporter-trace-otlp-proto/build/src/index.js";
import { Resource } from "../node_modules/@opentelemetry/resources/build/src/index.js";
import { SemanticResourceAttributes } from "../node_modules/@opentelemetry/semantic-conventions/build/src/index.js";
import { SimpleSpanProcessor } from "../node_modules/@opentelemetry/sdk-trace-base/build/src/index.js";
import { GrpcInstrumentation } from "../node_modules/@opentelemetry/instrumentation-grpc/build/src/index.js";
import { HttpInstrumentation } from "../node_modules/@opentelemetry/instrumentation-http/build/src/index.js";
import { ExpressInstrumentation } from "../node_modules/@opentelemetry/instrumentation-express/build/src/index.js";
import { MySQL2Instrumentation } from "../node_modules/@opentelemetry/instrumentation-mysql2/build/src/index.js";
import { AmqplibInstrumentation } from "../node_modules/@opentelemetry/instrumentation-amqplib/build/src/index.js";
import { AwsInstrumentation } from "../node_modules/@opentelemetry/instrumentation-aws-sdk/build/src/index.js";
import { RedisInstrumentation } from "../node_modules/@opentelemetry/instrumentation-redis/build/src/index.js";
import { MemcachedInstrumentation } from "../node_modules/@opentelemetry/instrumentation-memcached/build/src/index.js";

// backend/common/tracing/config.ts
var serviceName = process.env["SERVICE_NAME"] ?? "unknown";
var tracingEnabled = process.env["TRACING_ENABLED"] === "true";
var tracingDebugLoggingEnabled = process.env["TRACING_DEBUG_LOGGING"] === "true";

// backend/common/tracing/initialize.ts
if (tracingEnabled) {
  if (tracingDebugLoggingEnabled) {
    openTelemetryAPI.diag.setLogger(new openTelemetryAPI.DiagConsoleLogger(), openTelemetryAPI.DiagLogLevel.DEBUG);
  }
  const provider = new NodeTracerProvider({
    resource: new Resource({
      [SemanticResourceAttributes.SERVICE_NAME]: serviceName
    })
  });
  provider.addSpanProcessor(new SimpleSpanProcessor(new OTLPTraceExporter()));
  provider.register();
  registerInstrumentations({
    instrumentations: [
      new HttpInstrumentation({
        enabled: true,
        ignoreIncomingRequestHook(req) {
          return !!req.url?.match(/chkhealth|chkready/);
        }
      }),
      new ExpressInstrumentation({ enabled: true }),
      new GrpcInstrumentation({ enabled: true }),
      new MySQL2Instrumentation({ enabled: true }),
      new AmqplibInstrumentation({ enabled: true }),
      new AwsInstrumentation({ enabled: true }),
      new RedisInstrumentation({ enabled: true }),
      new MemcachedInstrumentation({ enabled: true })
    ]
  });
}
...

I am seeing auto-instrumented traces in the debug logs of my app and in Jaeger for mysql and http, but I'm not seeing debug logs or traces for any of the other instrumentations.

I recently switched the app from CommonJS to ESM due to several of my dependencies dropping support for CommonJS. I previously saw traces in Jaeger, and then (unsurprisingly) stopped seeing traces after switching my app to ESM.

Anyway, thank you so much for working on this feature, and hopefully my experience is just user error and will work out of the box once this PR lands.

vmarchaud · 2022-11-28T10:26:17Z

Anyway, thank you so much for working on this feature, and hopefully my experience is just user error and will work out of the box once this PR lands.

I believe this show that my PR doesn't work when we try to patch CJS module from ESM (since most of instrumentation we are patching are still CJS). My test only does ESM loading ESM modules. I'm gonna try to reproduce when i've time but in the mean time:

could you test to bundle all of your node_modules to ESM see if it's better ?
make a small repro of your build so i can test against ?

Thanks for your help

JamieDanielson · 2022-11-29T21:29:33Z

I branched and created an example app to try some things out using HTTP and Express instrumentation. I don't know if it was all necessary, but in a few places I updated the package.json to help ensure I was using the local instrumentation package.

There are instructions in a README for running this app as CommonJS and as ESModules - it defaults to CommonJS. When I run as CommonJS, I see 5 total spans: 1 http, 3 express, 1 manual. When I run as ESM, I only see the manual span.

I notice these in the console output for CommonJS and not ESM:

@opentelemetry/instrumentation-http Applying patch for http@16.15.0
Applying patch for express@4.18.2

JamieDanielson · 2023-04-04T21:48:14Z

As discussed outside of this PR and in SIG meetings, here is the new PR for this work: #3698

vmarchaud added the enhancement New feature or request label Mar 20, 2022

vmarchaud requested a review from a team as a code owner March 20, 2022 09:33

vmarchaud self-assigned this Mar 20, 2022

vmarchaud marked this pull request as draft March 20, 2022 10:08

vmarchaud force-pushed the esm-instrumentation branch 2 times, most recently from ee0d04d to 48fce71 Compare March 20, 2022 17:51

vmarchaud mentioned this pull request Apr 11, 2022

Add "@opentelemetry/sdk-node/register" for an agent-like experience #2891

Closed

8 tasks

vmarchaud force-pushed the esm-instrumentation branch from 3caafdf to 0801865 Compare April 18, 2022 16:33

vmarchaud marked this pull request as ready for review April 18, 2022 16:37

legendecas reviewed Apr 24, 2022

View reviewed changes

experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts Outdated Show resolved Hide resolved

vmarchaud force-pushed the esm-instrumentation branch from 0801865 to 629ef1d Compare April 24, 2022 15:00

vmarchaud force-pushed the esm-instrumentation branch from 629ef1d to aac74b6 Compare April 24, 2022 15:07

vmarchaud requested review from legendecas and a team April 24, 2022 15:07

rauno56 reviewed Apr 26, 2022

View reviewed changes

...tal/packages/opentelemetry-instrumentation/test/node/node_modules/my-esm-module/package.json Outdated Show resolved Hide resolved

experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts Outdated Show resolved Hide resolved

vmarchaud force-pushed the esm-instrumentation branch from b2654a5 to d93cdb3 Compare April 27, 2022 06:35

vmarchaud requested a review from rauno56 April 27, 2022 06:35

legendecas approved these changes Apr 29, 2022

View reviewed changes

vmarchaud requested a review from a team April 29, 2022 12:08

vmarchaud force-pushed the esm-instrumentation branch from d93cdb3 to c649189 Compare May 3, 2022 15:39

dyladan approved these changes May 3, 2022

View reviewed changes

dyladan self-requested a review May 3, 2022 21:02

vmarchaud mentioned this pull request Oct 29, 2022

Question: Plans for Node.js ESM Module Support? #1946

Closed

1 task

vmarchaud force-pushed the esm-instrumentation branch from c649189 to 4256872 Compare October 30, 2022 17:40

vmarchaud requested review from legendecas and JamieDanielson October 30, 2022 17:43

vmarchaud force-pushed the esm-instrumentation branch 7 times, most recently from 4437550 to 2c79d08 Compare October 30, 2022 20:30

Qard reviewed Oct 31, 2022

View reviewed changes

dyladan mentioned this pull request Nov 2, 2022

CJS - End of life Effect-TS/effect#1291

Closed

vmarchaud added 3 commits November 11, 2022 13:29

feat(instrumentation): add support for esm module

9c7bfcd

chore: fix windows + lint

1cf08c6

chore: documentation

f6384aa

vmarchaud force-pushed the esm-instrumentation branch from 2c79d08 to f6384aa Compare November 11, 2022 12:41

vmarchaud requested a review from a team November 11, 2022 12:45

legendecas reviewed Nov 15, 2022

View reviewed changes

JamieDanielson mentioned this pull request Mar 22, 2023

feat: ESM support for instrumentation #3698

Merged

8 tasks

vmarchaud closed this Apr 5, 2023

tobiasmuehl mentioned this pull request Feb 17, 2024

Support instrumentation of express when using ESModules #4490

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(instrumentation): add support for esm module #2846

feat(instrumentation): add support for esm module #2846

vmarchaud commented Mar 20, 2022

codecov bot commented Mar 20, 2022 •

edited

Flarna commented Mar 21, 2022

vmarchaud commented Mar 21, 2022

vmarchaud commented Apr 18, 2022 •

edited

vmarchaud commented Apr 24, 2022

rauno56 left a comment

vmarchaud commented Apr 27, 2022

dyladan left a comment

dyladan May 3, 2022

vmarchaud Oct 30, 2022

dyladan commented May 3, 2022

rauno56 commented May 4, 2022

vmarchaud commented Oct 30, 2022 •

edited

weyert commented Oct 31, 2022

Qard Oct 31, 2022

vmarchaud Oct 31, 2022

Qard Oct 31, 2022

vmarchaud Nov 11, 2022

dyladan Nov 11, 2022

vmarchaud Nov 11, 2022

legendecas Nov 15, 2022

dyladan Nov 29, 2022

Qard Jan 12, 2023

smnbbrv Jan 13, 2023 •

edited

vmarchaud commented Oct 31, 2022

weyert commented Oct 31, 2022

legendecas left a comment

legendecas Nov 15, 2022

jeremysf commented Nov 28, 2022

vmarchaud commented Nov 28, 2022

JamieDanielson commented Nov 29, 2022 •

edited

JamieDanielson commented Apr 4, 2023

feat(instrumentation): add support for esm module #2846

feat(instrumentation): add support for esm module #2846

Conversation

vmarchaud commented Mar 20, 2022

codecov bot commented Mar 20, 2022 • edited

Codecov Report

Flarna commented Mar 21, 2022

vmarchaud commented Mar 21, 2022

vmarchaud commented Apr 18, 2022 • edited

vmarchaud commented Apr 24, 2022

rauno56 left a comment

Choose a reason for hiding this comment

vmarchaud commented Apr 27, 2022

dyladan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dyladan commented May 3, 2022

rauno56 commented May 4, 2022

vmarchaud commented Oct 30, 2022 • edited

weyert commented Oct 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smnbbrv Jan 13, 2023 • edited

Choose a reason for hiding this comment

vmarchaud commented Oct 31, 2022

weyert commented Oct 31, 2022

legendecas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremysf commented Nov 28, 2022

vmarchaud commented Nov 28, 2022

JamieDanielson commented Nov 29, 2022 • edited

JamieDanielson commented Apr 4, 2023

codecov bot commented Mar 20, 2022 •

edited

vmarchaud commented Apr 18, 2022 •

edited

vmarchaud commented Oct 30, 2022 •

edited

smnbbrv Jan 13, 2023 •

edited

JamieDanielson commented Nov 29, 2022 •

edited