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

GlobalStatsStream does not track all instances after update to Node v12.16.0 #120

Closed
stefanpernpaintner opened this issue Dec 11, 2020 · 5 comments

Comments

@stefanpernpaintner
Copy link

stefanpernpaintner commented Dec 11, 2020

Brakes version: 3.0.1

After having updated to Node v12.16.0 globalStatsStream does not track the instances correctly. I have two brakes instantiated but only the first brake does notify via stream for only one first event. Example:

        const brake1 = new Brakes(promiseCall, {
            name: "MyDefault",
            bucketSpan: 5000,
            bucketNum: 3,
            waitThreshold: 5,
            timeout: 200,
            threshold: 0.5,
            circuitDuration: 30000,
        });

        const brake2 = new Brakes(promiseCall, {
            name: "XXXX",
            bucketSpan: 5000,
            bucketNum: 3,
            waitThreshold: 5,
            timeout: 200,
            threshold: 0.5,
            circuitDuration: 30000,
        });

        Brakes.getGlobalStats().getHystrixStream().on('data', (stats) => {
            console.log('received global stats ->', stats);
        });

With Node v12.15.0 it is working as expected. Alle events are propagated via the stats stream. Any known issues about this?

@awolden
Copy link
Owner

awolden commented Dec 11, 2020

Hi @stefanpernpaintner thanks for raising this issue. I'm not aware of any issues with newer version of node. Do you see the same issue with other major versions of node such as 14 or 15?

@stefanpernpaintner
Copy link
Author

I've tested only with node 14. The issue exists with node 14 too.

@stefanpernpaintner
Copy link
Author

Here is a full example with express which shows that only "Brake 1" is considered by the stream if you run it on node > 12.16 :

const express = require('express')
const Brakes = require("brakes");
const app = express()

function promiseCall(foo) {
    return new Promise((resolve, reject) => {
        if (foo) resolve(foo);
        else reject(foo);
    });
}


const brake1 = new Brakes(promiseCall, {
    name: "Brake 1",
    bucketSpan: 5000,
    bucketNum: 3,
    waitThreshold: 5,
    timeout: 200,
    threshold: 0.5,
    circuitDuration: 30000,
});

const brake2 = new Brakes(promiseCall, {
    name: "Brake 2",
    bucketSpan: 5000,
    bucketNum: 3,
    waitThreshold: 5,
    timeout: 200,
    threshold: 0.5,
    circuitDuration: 30000,
});

Brakes.getGlobalStats().getHystrixStream().on('data', (stats) => {
    console.log('received global stats ->', stats);
});

app.get('/', function (req, res) {
    brake1.exec('bar').then(value => {
        console.log(value)
    })
    brake2.exec("test").then(value => {
        console.log(value)
    })
    res.send('Hello World')
})

app.listen(3000)

@UyumazHakan
Copy link
Contributor

The root cause of problem is after first snapshot is processed by _rawStream. The stream gets paused and cannot pass check in main/lib/globalStats.js#L54 . Therefore, it will not able to push anymore snapshots in the stream for newer versions of Node. Please see https://nodejs.org/api/stream.html#stream_compatibility_with_older_node_js_versions . I created a pull request #121 with a fix.

@awolden
Copy link
Owner

awolden commented Jan 20, 2021

This issue should be fixed in in version 3.1.0. Thank you @UyumazHakan for the collaboration.

@awolden awolden closed this as completed Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants