Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Upgrade xorshift dependency #423

Closed
wants to merge 1 commit into from

Conversation

yurishkuro
Copy link
Member

Resolves #422

Signed-off-by: Yuri Shkuro <ys@uber.com>
@yurishkuro
Copy link
Member Author

as I suspected, some of the (tchannel-related) tests are failing

node_1         | /node_modules/tchannel/v2/call.js:49
node_1         | var CN_VALUE = bufferFrom('cn').readUInt16BE(0, false);
node_1         |                ^
node_1         | 
node_1         | TypeError: this is not a typed array.
node_1         |     at from (native)
node_1         |     at Object.<anonymous> (/node_modules/tchannel/v2/call.js:49:16)
node_1         |     at Module._compile (module.js:435:26)
node_1         |     at Module._extensions..js (module.js:442:10)
node_1         |     at Object.require.extensions.(anonymous function) [as .js] (/node_modules/babel-register/lib/node.js:152:7)
node_1         |     at Module.load (module.js:356:32)
node_1         |     at Function.Module._load (module.js:311:12)
node_1         |     at Module.require (module.js:366:17)
node_1         |     at require (module.js:385:17)
node_1         |     at Object.<anonymous> (/node_modules/tchannel/v2/index.js:44:12)
node_1         | /node_modules/tchannel/v2/call.js:49
node_1         | var CN_VALUE = bufferFrom('cn').readUInt16BE(0, false);
node_1         |                ^
node_1         | 
node_1         | TypeError: this is not a typed array.
node_1         |     at from (native)
node_1         |     at Object.<anonymous> (/node_modules/tchannel/v2/call.js:49:16)
node_1         |     at Module._compile (module.js:435:26)
node_1         |     at Module._extensions..js (module.js:442:10)
node_1         |     at Object.require.extensions.(anonymous function) [as .js] (/node_modules/babel-register/lib/node.js:152:7)
node_1         |     at Module.load (module.js:356:32)
node_1         |     at Function.Module._load (module.js:311:12)
node_1         |     at Module.require (module.js:366:17)
node_1         |     at require (module.js:385:17)
node_1         |     at Object.<anonymous> (/node_modules/tchannel/v2/index.js:44:12)
jaeger_1       | 2020/02/26 22:25:50 maxprocs: Leaving GOMAXPROCS=2: CPU quota undefined
java_1         | Feb 26, 2020 10:25:55 PM org.glassfish.grizzly.http.server.HttpServer start
python_1       | DEBUG:jaeger_tracing:Tracing sampler set to ProbabilisticSampler(1.0)
jaeger_1       | {"level":"info","ts":1582755950.159294,"caller":"flags/service.go:115","msg":"Mounting metrics handler on admin server","route":"/metrics"}
java_1         | INFO: [HttpServer] Started.
node_1         | /node_modules/tchannel/v2/call.js:49
node_1         | var CN_VALUE = bufferFrom('cn').readUInt16BE(0, false);
node_1         |                ^
node_1         | 
node_1         | TypeError: this is not a typed array.
node_1         |     at from (native)
node_1         |     at Object.<anonymous> (/node_modules/tchannel/v2/call.js:49:16)
node_1         |     at Module._compile (module.js:435:26)
node_1         |     at Module._extensions..js (module.js:442:10)
node_1         |     at Object.require.extensions.(anonymous function) [as .js] (/node_modules/babel-register/lib/node.js:152:7)
node_1         |     at Module.load (module.js:356:32)
node_1         |     at Function.Module._load (module.js:311:12)
node_1         |     at Module.require (module.js:366:17)
node_1         |     at require (module.js:385:17)
node_1         |     at Object.<anonymous> (/node_modules/tchannel/v2/index.js:44:12)

@oliversalzburg
Copy link
Contributor

I wish I could help, but getting this module to build/test on Windows seems to be its very own challenge. :(

@yurishkuro yurishkuro closed this Aug 14, 2020
@yurishkuro yurishkuro deleted the upgrade-xorshift branch August 14, 2020 17:30
@oliversalzburg
Copy link
Contributor

@yurishkuro Why did you close this?

@yurishkuro
Copy link
Member Author

I don't have time to investigate / finish it (not sure it's even possible without serious refactoring).

@oliversalzburg
Copy link
Contributor

I don't really understand how that failure would be caused by the xorshift upgrade. I can now build and test the project locally with an upgraded xorshift just fine. Can you explain to me how to reproduce the failure you saw?

The CI failure seemed to only be in a single run and I can't really make sense of it. What's the actual failure here?

What is the lowest NodeJS Version that must be supported?

@oliversalzburg
Copy link
Contributor

So to my understanding, those are the crossdock tests that fail, which can be run with make crossdock-fresh. Those also pass for me locally, even when using Node 0.10, which appears to be the lowest supported version (why? 😢)

@yurishkuro
Copy link
Member Author

Feel free to make another PR if you think you can make it work. The CI failure was here: https://travis-ci.org/github/jaegertracing/jaeger-client-node/builds/655565023

Uber still uses Node 0.10 for some critical services, that's why we keep compatibility. I agree it's annoying.

@oliversalzburg
Copy link
Contributor

I created a PR at #442

Iuriy-Budnikov pushed a commit to agile-pm/jaeger-client-node that referenced this pull request Sep 25, 2021
* feat(plugin-dns): add lookup patches

ref: jaegertracing#423
add docs
add example
include promise patch for lookup

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>

* fix: add mayurkale22 recommendations
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Outdated xorshift dependency causes deprecation warning
2 participants