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 PgPlugin - PosgreSQL #31

Merged
merged 9 commits into from
Feb 27, 2021
Merged

Add PgPlugin - PosgreSQL #31

merged 9 commits into from
Feb 27, 2021

Conversation

tom-pytel
Copy link
Contributor

Some tweaks to the mysql plugin here as well. Will see about tests for both this and mysql plugins, maybe I will add them to this PR if I have time but if no then will have to be a separate one.

@kezhenxu94 kezhenxu94 added this to the 0.2.0 milestone Feb 23, 2021
@kezhenxu94
Copy link
Member

Hope the tests can be added ASAP

@tom-pytel
Copy link
Contributor Author

Hope the tests can be added ASAP

Yes, am trying for this PR.

@tom-pytel
Copy link
Contributor Author

Well, I am not having much luck even running the baseline tests locally, so without that can't actually add any:

2021-02-23T16:42:59.490Z testcontainers WARN Failed to stop DockerCompose environment after failed start
 FAIL  tests/plugins/axios/test.ts (123.094 s)
  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/axios/test.ts

    The Compose file './docker-compose.yml' is invalid because:
    Unsupported config option for services.client: 'extends'
    Unsupported config option for services.collector: 'extends'
    Unsupported config option for services.server: 'extends'
    services.server.depends_on contains an invalid type, it should be an array
    services.client.depends_on contains an invalid type, it should be an array

      at node_modules/testcontainers/dist/docker-compose-environment.js:189:15
          at Generator.throw (<anonymous>)
      at rejected (node_modules/testcontainers/dist/docker-compose-environment.js:25:65)

  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/axios/test.ts

    connect ECONNREFUSED 127.0.0.1:5001



 FAIL  tests/plugins/express/test.ts (123.199 s)
  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/express/test.ts

    The Compose file './docker-compose.yml' is invalid because:
    Unsupported config option for services.client: 'extends'
    Unsupported config option for services.collector: 'extends'
    Unsupported config option for services.server: 'extends'
    services.server.depends_on contains an invalid type, it should be an array
    services.client.depends_on contains an invalid type, it should be an array

      at node_modules/testcontainers/dist/docker-compose-environment.js:189:15
          at Generator.throw (<anonymous>)
      at rejected (node_modules/testcontainers/dist/docker-compose-environment.js:25:65)

  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/express/test.ts

    connect ECONNREFUSED 127.0.0.1:5001

interval: 5s
timeout: 60s
retries: 120
image: "docker.io/mysql:latest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mysql issue It seems that the npm package mysql can't support mysql8 well. Trying to use a lower version should be successful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the authentication thing, but no, I think its just the test writing sw logging to console, this requires @kezhenxu94 intervention.

@tom-pytel tom-pytel mentioned this pull request Feb 25, 2021
@kezhenxu94
Copy link
Member

The failure reason seems to be very trivial

image

@tom-pytel
Copy link
Contributor Author

What about my problem of running tests locally?

@kezhenxu94
Copy link
Member

What about my problem of running tests locally?

From the logs, I suspect you may be running a rather old-version docker?

@tom-pytel
Copy link
Contributor Author

From the logs, I suspect you may be running a rather old-version docker?

tom@tom-VirtualBox:~/src/revdebugjs/skywalking-nodejs {master} $ docker --version
Docker version 20.10.3, build 48d30b5

@kezhenxu94
Copy link
Member

@tom-pytel can you change those version back to 2.1 and try again? I can reproduce this on a Ubuntu machine, though with different error message

@tom-pytel
Copy link
Contributor Author

@tom-pytel can you change those version back to 2.1 and try again? I can reproduce this on a Ubuntu machine, though with different error message

2.1, the errors are different but still erroring. Also, 17 minutes to test 4 simple plugins?

 FAIL  tests/plugins/mysql/test.ts (933.259 s)
  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/mysql/test.ts

    Creating network "testcontainers-61d5207f762731b72fc2a68e41ebe3a9_traveling-light" with the default driver
    Building collector
    Image for service collector was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
    Pulling mysql (docker.io/mysql:5.7.33)...
    Get https://registry-1.docker.io/v2/library/mysql/manifests/sha256:853105ad984a9fe87dd109be6756e1fbdba8b003b303d88ac0dda6b455f36556: net/http: TLS handshake timeout

      at node_modules/testcontainers/dist/docker-compose-environment.js:189:15
      at fulfilled (node_modules/testcontainers/dist/docker-compose-environment.js:24:58)
          at runMicrotasks (<anonymous>)

  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/mysql/test.ts

    connect ECONNREFUSED 127.0.0.1:5001



2021-02-25T16:41:36.150Z testcontainers ERROR Failed to start DockerCompose environment: Creating network "testcontainers-53a586624c68a4aca6c99d79e6247822_traveling-light" with the default driver
Building collector
Image for service collector was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
Building server
Service 'server' failed to build: The command '/bin/sh -c npm install request && npm install' returned a non-zero code: 1

2021-02-25T16:41:36.150Z testcontainers INFO Downing DockerCompose environment
2021-02-25T16:41:36.894Z testcontainers INFO Downed DockerCompose environment
 FAIL  tests/plugins/express/test.ts (971.688 s)
  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/express/test.ts

    Creating network "testcontainers-c481cfb7ca6a39b81b77b8fa108a7284_traveling-light" with the default driver
    Building collector
    Image for service collector was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
    Building server
    Service 'server' failed to build: The command '/bin/sh -c npm install request && npm install' returned a non-zero code: 1

      at node_modules/testcontainers/dist/docker-compose-environment.js:189:15
      at fulfilled (node_modules/testcontainers/dist/docker-compose-environment.js:24:58)
          at runMicrotasks (<anonymous>)

  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/express/test.ts

    connect ECONNREFUSED 127.0.0.1:5001



 FAIL  tests/plugins/axios/test.ts (974.59 s)
  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/axios/test.ts

    Creating network "testcontainers-b66388486c6223686148ce4a19677f16_traveling-light" with the default driver
    Building collector
    Image for service collector was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
    Building server
    Service 'server' failed to build: The command '/bin/sh -c npm install request && npm install' returned a non-zero code: 1

      at node_modules/testcontainers/dist/docker-compose-environment.js:189:15
      at fulfilled (node_modules/testcontainers/dist/docker-compose-environment.js:24:58)
          at runMicrotasks (<anonymous>)

  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/axios/test.ts

    connect ECONNREFUSED 127.0.0.1:5001



 FAIL  tests/plugins/http/test.ts (1060.006 s)
  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/http/test.ts

    Creating network "testcontainers-53a586624c68a4aca6c99d79e6247822_traveling-light" with the default driver
    Building collector
    Image for service collector was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
    Building server
    Service 'server' failed to build: The command '/bin/sh -c npm install request && npm install' returned a non-zero code: 1

      at node_modules/testcontainers/dist/docker-compose-environment.js:189:15
      at fulfilled (node_modules/testcontainers/dist/docker-compose-environment.js:24:58)
          at runMicrotasks (<anonymous>)

  ● plugin tests › /home/tom/src/revdebugjs/skywalking-nodejs/tests/plugins/http/test.ts

    connect ECONNREFUSED 127.0.0.1:5001



Test Suites: 4 failed, 4 total
Tests:       4 failed, 4 total
Snapshots:   0 total
Time:        1060.214 s
Ran all test suites.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test: `DEBUG=testcontainers* jest`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/tom/.npm/_logs/2021-02-25T16_43_37_124Z-debug.log

@kezhenxu94
Copy link
Member

Service 'server' failed to build: The command '/bin/sh -c npm install request && npm install' returned a non-zero code: 1

I have encountered this sometimes when my network speed is slow.

Also, 17 minutes to test 4 simple plugins?

This depends on many factors, the network speed, the spec of the machine, etc., but usually, I only run the one test that I'm concerned of, e.g. npm run test tests/plugins/mysql.

src/config/AgentConfig.ts Outdated Show resolved Hide resolved
tests/plugins/mysql/init/init.sql Show resolved Hide resolved
kezhenxu94
kezhenxu94 previously approved these changes Feb 26, 2021
@tom-pytel
Copy link
Contributor Author

Note that this doesn't have the postgre test yet, will add before merge if sql test runs ok.

@tom-pytel
Copy link
Contributor Author

I officially give up

@kezhenxu94
Copy link
Member

I got this from the GitHub Actions logs, open the raw logs and search console.info, and compare with the expected.data.yaml in this PR

@@ -33,9 +33,12 @@ segmentItems:
             peer: not null
             skipAnalysis: false
             tags:
-              - { key: http.url, value: 'http://server:5000/postgre' }
-              - { key: http.method, value: GET }
-              - { key: http.status.code, value: '200' }
+              - key: http.url
+                value: http://server:5000/postgre
+              - key: http.method
+                value: GET
+              - key: http.status.code
+                value: "200"
             refs:
               - parentEndpoint: ""
                 networkAddress: server:5000
@@ -52,20 +55,23 @@ segmentItems:
             spanLayer: Database
             startTime: gt 0
             endTime: gt 0
-            componentId: 22
+            componentId: 5
             spanType: Exit
             peer: postgres:5432
             skipAnalysis: false
             tags:
-              - { key: db.type, value: PostgreSQL }
-              - { key: db.instance, value: test }
-              - { key: db.statement, value: SELECT * FROM "user" where name = 'u1' }
+              - key: db.type
+                value: PostgreSQL
+              - key: db.instance
+                value: test
+              - key: db.statement
+                value: SELECT * FROM `user` WHERE `name` = "u1"
   - serviceName: client
     segmentSize: 1
     segments:
       - segmentId: not null
         spans:
-          - operationName: /postgre
+          - operationName: /postgres
             operationId: 0
             parentSpanId: -1
             spanId: 0
@@ -77,10 +83,13 @@ segmentItems:
             peer: not null
             skipAnalysis: false
             tags:
-              - { key: http.url, value: 'http://localhost:5001/postgre' }
-              - { key: http.method, value: GET }
-              - { key: http.status.code, value: '200' }
-          - operationName: /postgre
+              - key: http.url
+                value: http://localhost:5001/postgres
+              - key: http.method
+                value: GET
+              - key: http.status.code
+                value: "200"
+          - operationName: /postgres
             operationId: 0
             parentSpanId: 0
             spanId: 1
@@ -92,7 +101,11 @@ segmentItems:
             peer: server:5000
             skipAnalysis: false
             tags:
-              - { key: http.url, value: 'server:5000/postgre' }
-              - { key: http.method, value: GET }
-              - { key: http.status.code, value: '200' }
-              - { key: http.status.msg, value: OK }
+              - key: http.url
+                value: server:5000/postgres
+              - key: http.method
+                value: GET
+              - key: http.status.code
+                value: "200"
+              - key: http.status.msg
+                value: OK

@tom-pytel
Copy link
Contributor Author

I got this from the GitHub Actions logs, open the raw logs and search console.info, and compare with the expected.data.yaml in this PR

Ok, what tool are you using to diff visualize this?

@kezhenxu94
Copy link
Member

I got this from the GitHub Actions logs, open the raw logs and search console.info, and compare with the expected.data.yaml in this PR

Ok, what tool are you using to diff visualize this?

I was actually using WebStorm to compare and eliminate those not null, gt 0, etc. and finally use git diff to generate the 👆 text diff

@tom-pytel
Copy link
Contributor Author

I was actually using WebStorm to compare and eliminate those not null, gt 0, etc. and finally use git diff to generate the text diff

I wonder if there is a plugin for VSCode, because without it those logs are unreadable.

@kezhenxu94
Copy link
Member

I was actually using WebStorm to compare and eliminate those not null, gt 0, etc. and finally use git diff to generate the text diff

I wonder if there is a plugin for VSCode, because without it those logs are unreadable.

I'm not a fan of VS Code so I don't know whether or not there is such plugin. But I may consider building that into the CI workflow (we have similar in skywalking-python repo) to diff the expected data when failure,

@tom-pytel
Copy link
Contributor Author

I'm not a fan of VS Code so I don't know whether or not there is such plugin. But I may consider building that into the CI workflow (we have similar in skywalking-python repo) to diff the expected data when failure,

That would be good because for someone without your setup trying to find an error in these raw logs is problematic. Speaking of which, what is it that is failing now?

@tom-pytel
Copy link
Contributor Author

OH MY GOD!

@tom-pytel
Copy link
Contributor Author

Quick! Merge it before it breaks again!

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@kezhenxu94 kezhenxu94 merged commit 78d3a2a into apache:master Feb 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants