upstash / issues

Issue Tracker for Upstash
https://upstash.com
2 stars 0 forks source link

[RateLimit] Nothing displayed on `Rate Limit Analytics` panel because the `identifier` is not a valid ip? #52

Open dreamer2q opened 1 year ago

dreamer2q commented 1 year ago

My Code as follows:

  1. create a common function for reuse
import { Ratelimit } from '@upstash/ratelimit';
import redis from '../redis';
import { getRealIp } from './ip';

/**
 * @param {string} key
 * @param {number} token
 * @param {string} duration
 * @returns
 */
export function createRateLimit(key, tokens, duration) {
  const limiter = new Ratelimit({
    redis,
    limiter: Ratelimit.fixedWindow(tokens, duration),
  });

  return {
    check: async (req, res) => {
      const reqIp = getRealIp(req);
      const reqIdentifier = `${reqIp}:${key}`; // please note here, its not a valid ip, it got a suffix~
      const { success, limit, remaining, reset } = await limiter.limit(
        reqIdentifier,
        req
      );
      res.setHeader('X-RateLimit-Limit', limit);
      res.setHeader('X-RateLimit-Remaining', remaining);
      res.setHeader('X-RateLimit-Reset', reset);
      return success;
    },
  };
}
  1. use it in any api route I want to setup ratelimit

const limit = createRateLimit('search', 50, '1h'); // please note the key

/**

after that, deployed, and worked, but nothing in Rate Limit Analytics panel.

I have checked the rawData, its working ~

It looks that the Panel not recoginized with what I set as an identifier~ Bad news.

dreamer2q commented 1 year ago

Sample Picture of Data View here, please take note of the key !!!

image
chronark commented 1 year ago

hey, you need to opt into analytics:

new Ratelimit({
    redis,
    limiter: Ratelimit.fixedWindow(tokens, duration),
    analytics: true // <-  
  });
dreamer2q commented 1 year ago

hey, you need to opt into analytics:

new Ratelimit({
    redis,
    limiter: Ratelimit.fixedWindow(tokens, duration),
    analytics: true // <-  
  });

The analytics default is true, am I wrong?

image
chronark commented 1 year ago

It was true on accident, we have set it to false in the latest version, since we don't want to add any cost to our users by default what version are you on?

dreamer2q commented 1 year ago

I will set analytics: true and redeploy to see if working.

It was true on accident, we have set it to false in the latest version, since we don't want to add any cost to our users by default what version are you on?

package.json says "@upstash/ratelimit": "^0.4.2" as well as yarn.lock

dreamer2q commented 1 year ago

Thanks for your attention. Its working now~ closed.

Docs should follow the latest code as it suggests or whatever

dreamer2q commented 1 year ago

It was true on accident, we have set it to false in the latest version, since we don't want to add any cost to our users by default what version are you on?

OMG I need to know how much cost it will increase? or I would consider altertives ...

dreamer2q commented 1 year ago

The Github seems to experience an accident just now :(

chronark commented 1 year ago

it adds 1 command per limit invocation

dreamer2q commented 1 year ago

it adds 1 command per limit invocation

So the usage is doubled if analytics enabled?

chronark commented 1 year ago

that depends on the algorithm used. you can check here where all the algorithms are implemented