Skip to content

feat: added support for ioredis cluster #1292

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

Merged
merged 4 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions packages/collector/test/tracing/database/ioredis/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,24 @@ require('../../../..')();
const bodyParser = require('body-parser');
const express = require('express');
const morgan = require('morgan');
const Redis = require('ioredis');
const ioredis = require('ioredis');
const fetch = require('node-fetch-v2');
const port = require('../../../test_util/app-port')();
const connect = require('./connect-via');
const app = express();
const logPrefix = `Express / Redis App (${process.pid}):\t`;
let connectedToRedis = true;
const logPrefix = `Express / IORedis App (${process.pid}):\t`;

const client = new Redis(`//${process.env.REDIS}`);
const client2 = new Redis(`//${process.env.REDIS_ALTERNATIVE}`);
let connectedToRedis = false;
let client;
let client2;

let clientReady = false;
let client2Ready = false;
client.on('ready', () => {
clientReady = true;
if (client2Ready) {
connectedToRedis = true;
}
});
client2.on('ready', () => {
client2Ready = true;
if (clientReady) {
connectedToRedis = true;
}
});
(async () => {
const { connection, connection2 } = await connect(ioredis, log);

client = connection;
client2 = connection2;
connectedToRedis = true;
})();

if (process.env.WITH_STDOUT) {
app.use(morgan(`${logPrefix}:method :url :status`));
Expand Down
30 changes: 12 additions & 18 deletions packages/collector/test/tracing/database/ioredis/app.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,25 @@ const agentPort = process.env.INSTANA_AGENT_PORT;
import bodyParser from 'body-parser';
import express from 'express';
import morgan from 'morgan';
import Redis from 'ioredis';
import ioredis from 'ioredis';
import fetch from 'node-fetch';
import portFactory from '../../../test_util/app-port.js';
import connect from './connect-via/index.js';
const port = portFactory();
const app = express();
const logPrefix = `Express / Redis App (${process.pid}):\t`;
let connectedToRedis = true;

const client = new Redis(`//${process.env.REDIS}`);
const client2 = new Redis(`//${process.env.REDIS_ALTERNATIVE}`);
let connectedToRedis = false;
let client;
let client2;

let clientReady = false;
let client2Ready = false;
client.on('ready', () => {
clientReady = true;
if (client2Ready) {
connectedToRedis = true;
}
});
client2.on('ready', () => {
client2Ready = true;
if (clientReady) {
connectedToRedis = true;
}
});
(async () => {
const { connection, connection2 } = await connect(ioredis, log);

client = connection;
client2 = connection2;
connectedToRedis = true;
})();

if (process.env.WITH_STDOUT) {
app.use(morgan(`${logPrefix}:method :url :status`));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* (c) Copyright IBM Corp. 2024
*/

'use strict';

// NOTE: We run the tests locally and on CI against azure redis cluster.
// NOTE: We cannot run redis cluster on Tekton https://github.com/bitnami/charts/issues/28894
// NOTE: We cannot use a docker based redis cluster at the moment!
// See https://github.com/redis/node-redis/issues/2815.
// These commands are useful as soon as we switch to a docker based cluster.
// node bin/start-test-containers.js --redis-node-0 --redis-node-1 --redis-node-2
// docker exec -it 2aaaac7b9112 redis-cli -p 6379 cluster info
module.exports = async function connect(ioredis, log) {
if (!process.env.AZURE_REDIS_CLUSTER || !process.env.AZURE_REDIS_CLUSTER_PWD) {
Copy link
Contributor

Choose a reason for hiding this comment

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

While running the tests locally, I initially encountered the following issue:

[ioredis] Unhandled error event: ClusterAllFailedError: Failed to refresh slots cache.

The issue seems to have resolved itself after some time. It might be related to the cluster since we use the same cluster for both Redis and ioredis testing. Running tests in parallel could potentially cause conflicts. We should consider alternative solutions, such as using namespaces, to prevent this issue in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: We can create a card for now and do it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

log(
'Please set the environment variables AZURE_REDIS_CLUSTER and AZURE_REDIS_CLUSTER_PWD ' +
'to connect to the cloud redis cluster.'
);

process.exit(1);
}

const hostAndPort = process.env.AZURE_REDIS_CLUSTER.split(':');
const cluster = new ioredis.Cluster(
[
{
host: hostAndPort[0],
port: hostAndPort[1]
}
],
{
redisOptions: {
tls: true,
password: process.env.AZURE_REDIS_CLUSTER_PWD,
connectTimeout: 10000
},
retryDelayOnFailover: 1000,
maxRetriesPerRequest: 10
}
);

log(`Connecting to cluster host: ${hostAndPort[0]}, port: ${hostAndPort[1]}.`);

return new Promise(resolve => {
cluster.on('ready', () => {
log('Connected to cluster.');
resolve({ connection: cluster });
});
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* (c) Copyright IBM Corp. 2024
*/

'use strict';

module.exports = async function (ioredis, log) {
const client = new ioredis(`//${process.env.REDIS}`);
const client2 = new ioredis(`//${process.env.REDIS_ALTERNATIVE}`);

return new Promise(resolve => {
let clientReady = false;
let client2Ready = false;

client.on('ready', () => {
clientReady = true;
log(`Connected to client 1 (${process.env.REDIS}).`);

if (client2Ready) {
resolve({
connection: client,
connection2: client2
});
}
});

client2.on('ready', () => {
client2Ready = true;
log(`Connected to client 2 (${process.env.REDIS_ALTERNATIVE}).`);

if (clientReady) {
resolve({
connection: client,
connection2: client2
});
}
});
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* (c) Copyright IBM Corp. 2024
*/

'use strict';

module.exports = function (redis, log) {
if (process.env.REDIS_CLUSTER === 'true') {
return require('./cluster')(redis, log);
}

return require('./default')(redis, log);
};
Loading