Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add knex support for 1.x.x and 2.x.x #861

Open
blumamir opened this issue Jan 30, 2022 · 2 comments
Open

add knex support for 1.x.x and 2.x.x #861

blumamir opened this issue Jan 30, 2022 · 2 comments
Assignees

Comments

@blumamir
Copy link
Member

blumamir commented Jan 30, 2022

What version of OpenTelemetry are you using?

latest (0.27.1)

What version of Node are you using?

14

What did you do?

Knex instrumentation supports versions >= 0.22.0

export const SUPPORTED_VERSIONS = [
  // use "lib/execution" for runner.js, "lib" for client.js as basepath, latest tested 0.95.6
  '>=0.22.0',
  // use "lib" as basepath
  '>=0.10.0 <0.18.0',
  '>=0.19.0 <0.22.0',
  // use "src" as basepath
  '>=0.18.0 <0.19.0',
];

Knex published version 1.0.0 on 16/01/2022.
When renovate tried to update the version in this PR, tests failed for v1:

  2) Knex instrumentation
       "after each" hook for "should record spans from query builder":
     TypeError: Cannot read property 'schema' of undefined
      at Context.<anonymous> (test/index.test.ts:69:12)
      at processImmediate (internal/timers.js:464:21)
      at process.callbackTrampoline (internal/async_hooks.js:130:17)

and more.

I don't know if it's only the tests, or the instrumentation itself should be updated, but we should examine this and add test-all-versions.
cc @rauno56

@blumamir blumamir added the bug Something isn't working label Jan 30, 2022
@blumamir blumamir changed the title knex instrumentation report support for >= 0.22.0 but tests fails for v1.0.0 knex instrumentation claims support for >= 0.22.0 but tests fails for v1.0.0 Jan 30, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2022

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 github-actions bot added the stale label Apr 4, 2022
@rauno56 rauno56 added never-stale and removed stale labels Apr 13, 2022
@blumamir blumamir changed the title knex instrumentation claims support for >= 0.22.0 but tests fails for v1.0.0 add knex support for 1.x.x and 2.x.x Apr 22, 2022
@blumamir
Copy link
Member Author

There is version 2 as of 21/04/2022.
Renovate tried to bump the version in #977 and tests failed:

> @opentelemetry/instrumentation-knex@0.27.1 test /home/runner/work/opentelemetry-js-contrib/opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-knex
> nyc ts-mocha -p tsconfig.json 'test/**/*.ts'



  Knex instrumentation
    Instrumenting
      1) should record spans from query builder
      ✓ should collect spans from raw
      ✓ should truncate the query
      ✓ should catch errors
    Disabling instrumentation
      ✓ should not create new spans


  4 passing (135ms)
  1 failing

  1) Knex instrumentation
       Instrumenting
         should record spans from query builder:
     AssertionError [ERR_ASSERTION]: At span[1]: Input A expected to strictly equal input B:
+ expected - actual

- undefined
+ 'c796c8a7[68](https://github.com/open-telemetry/opentelemetry-js-contrib/runs/6121999735?check_suite_focus=true#step:9:68)3d1b5a'
      at actualSpans.forEach (test/index.test.ts:245:14)
      at Array.forEach (<anonymous>)
      at assertSpans (test/index.test.ts:227:15)
      at api_1.context.with (test/index.test.ts:94:11)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants