search

aspecto-io / opentelemetry-ext-js

js extensions for the open-telemetry project
https://www.aspecto.io
Apache License 2.0
172 stars 40 forks source link

opentelemetry-instrumentation-neo4j throwing an error! Cannot read property '_host' of undefined at Object.getAttributesFromNeo4jSession #77

Closed bokjo closed 3 years ago

bokjo commented 3 years ago

Hello Aspecto people,

First of all thanks for the Neo4j opentelemetry plugin!

I'm testing it now and have the following issue...

{
    "stacktrace": [
     "TypeError: Cannot read property '_host' of undefined",
         "    at Object.getAttributesFromNeo4jSession ([redacted]/node_modules/opentelemetry-instrumentation-neo4j/src/utils.ts:14:6)",
         "    at Transaction.<anonymous> ([redacted]/node_modules/opentelemetry-instrumentation-neo4j/src/neo4j.ts:62:46)",
         "    at _callee3$ ([redacted]/node_modules/neo4j-graphql-js/dist/index.js:226:35)",
         "    at tryCatch ([redacted]/node_modules/regenerator-runtime/runtime.js:63:40)",
         "    at Generator.invoke [as _invoke] ([redacted]/node_modules/regenerator-runtime/runtime.js:293:22)",
         "    at Generator.next ([redacted]/node_modules/regenerator-runtime/runtime.js:118:21)",
         "    at asyncGeneratorStep ([redacted]/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:5:24)",
         "    at _next ([redacted]/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:27:9)",
         "    at [redacted]/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:34:7",
         "    at new Promise (<anonymous>)"
    ]
}

I did a bit of digging around and make it work on my end since I need to test the whole flow

https://github.com/aspecto-io/opentelemetry-ext-js/blob/f75dd4f82c449707d792e2cc8d295bf94f4370f4/packages/instrumentation-neo4j/src/utils.ts#L5

On _connectionProvider: RoutingConnectionProvider I don't have _address coming from the neo4j driver!

Instead, there is _seedRouter: ServerAddress with the fields needed (_host and _port)

_seedRouter: ServerAddress {
      _host: '[redacted].databases.neo4j.io',
      _resolved: null,
      _port: [redacted],
      _hostPort: '[redacted].databases.neo4j.io:[redacted]',
      _stringValue: '[redacted].databases.neo4j.io:[redacted]'
    }

So I have patched it locally with the code below and it is working fine

const address = connectionHolder._connectionProvider._seedRouter;

It would be nice if there are null/undefined check guards and some generic default values for attributes so it doesn't fail or let it fail but more gracefully!

eg: https://github.com/aspecto-io/opentelemetry-ext-js/blob/f75dd4f82c449707d792e2cc8d295bf94f4370f4/packages/instrumentation-neo4j/src/utils.ts#L9

Node version: v14.15.4 (npm v6.14.10) Neo4j driver version: 4.2.2 opentelemetry-instrumentation-neo4j version: 0.2.2 Neo4j DB version: Version: | 4.2.0 Edition: | Enterprise

I'm using Neo4j Aura managed DB from Neo4j

p.s haven't tested with local or self-managed Neo4j DB

Kind Regards, Bojanche S.

nirsky commented 3 years ago

Hey @bokjo! Thanks for finding this bug. I'll work on a fix today and add a bunch of null/undefined check guards to this piece of code. I'd appreciate it if you could add some usage examples so I can reproduce the issue. Especially how you initialize the driver.

Nir

nirsky commented 3 years ago

I was able to identify the issue. When using aura the URL starts with neo4j, the lib identifies this as routing mode: https://github.com/neo4j/neo4j-javascript-driver/blob/4.3/src/index.js#L206

This results in creating a RoutingDriver under the hood, which uses _seedRouter instead of _address.
I wasn't able to identify this on the unit tests due to using local neo4j only.

Anyway, fixed in this PR: https://github.com/aspecto-io/opentelemetry-ext-js/pull/78 Released in 0.2.3.

bokjo commented 3 years ago

Hey @bokjo! Thanks for finding this bug. I'll work on a fix today and add a bunch of null/undefined check guards to this piece of code. I'd appreciate it if you could add some usage examples so I can reproduce the issue. Especially how you initialize the driver.

Nir

I was able to identify the issue. When using aura the URL starts with neo4j, the lib identifies this as routing mode: https://github.com/neo4j/neo4j-javascript-driver/blob/4.3/src/index.js#L206

This results in creating a RoutingDriver under the hood, which uses _seedRouter instead of _address. I wasn't able to identify this on the unit tests due to using local neo4j only.

Anyway, fixed in this PR: #78 Released in 0.2.3.

Hello @nirsky,

Thank you very much for the quick fix.

Yes my scheme for connecting to Aura is neo4j+s ... neo4j+s://[database_uri]

The use case is a pretty simple POC API with Express Apollo GraphQL, GRANDStack with generated and custom resolvers and Neo4j as DB.

If some additional info is needed feel free to ping me.

This week I want to test every service with the new Opentelemetry 0.18.x, will keep you posted if there are some issues.

Kind Regards, Bojanche S.

nirsky commented 3 years ago

Cool, we'll probably release the plugins for 0.18.x in the next week or so.