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

About rate limiting #45

Open
arggh opened this issue Jan 6, 2018 · 8 comments
Open

About rate limiting #45

arggh opened this issue Jan 6, 2018 · 8 comments

Comments

@arggh
Copy link
Contributor

arggh commented Jan 6, 2018

I have ended up customizing my fork of FiLog to have the option to limit the amount of exceptions that are logged by logger.arm(), by giving it as an argument like so: logger.arm(5).

One scenario why I implemented that: sometimes a user has a really outdated browser that might throw 1000's of syntax errors in a very short amount of time. The easy fix for scenario was to create a counter to disarm logger after the specified limit is reached.

Would this be something you'd consider merging to FiLog?

Another way to approach this would be "rate limiting" on the server side much like DDP Rate Limiter does, which would be safer and could possibly be applied to all logs, if needed.

Currently anybody can "bombard" the logger endpoint and fill the logs with garbage.

@fgm fgm added the enhancement label Jan 7, 2018
@fgm
Copy link
Owner

fgm commented Jan 7, 2018

Sure: rate-limiting is a must for any server accepting "wild" output like browsers. The difficulty IMHO is the strategy used to choose which requests to drop, and possibly still do something with them like counting to get an idea of the drop rate, at the very least.

@arggh
Copy link
Contributor Author

arggh commented Jan 7, 2018

Some kind of aggregation on the client side, combined with server ride rate limiting would be interesting. Maybe FiLog could have two endpoints, one rate limited for the current logs, and another one to receive a message such as Client xx.xx.xx.xx logged 998989 times error "Script error.", in case the first endpoint is bogged down due to gazillion errors coming through.

Related to this, is there some reasoning why FiLog is not using DDP and Meteor methods for logging? If it were, a developer could easily apply DDP Rate Limiter rules to FiLog.

@fgm
Copy link
Owner

fgm commented Jan 7, 2018

About DDP, the issues I had at the time were

  • We really wanted to have the ability to connect clients to a logging server separate from the main application server, to avoid swamping the app server with the logging load
  • but I could not figure how to get a browser connected to one Meteor app to connect to Meteor on a separate Meteor server for a logging service instead of the one it is already connected to. Hence the plain HTTP separate connexion.

In retrospect, it may well have been a case of incorrect premature optimization, as for our actual use case with at max a few 100 users, load turned out not to be a problem. But on the plus side, nothing prevents adding a MeteorDDPSender and a meteor method in addition to the MeteorHttpSender and WebApp (Connect) route, and using it instead.

@arggh
Copy link
Contributor Author

arggh commented Jan 7, 2018

...but I could not figure how to get a browser connected to one Meteor app to connect to Meteor on a separate Meteor server for a logging service instead of the one it is already connected to.

That should be as simple as this:

const connection = DDP.connect(anotherUrl);
connection.call('logMethod', logArgs);

I think I might implement that as a MeteorDDPSender.

@fgm fgm added the PR welcome label Jan 7, 2018
@fgm
Copy link
Owner

fgm commented Jan 7, 2018

Don't forget to add the matching method on the server side too. Configuration on both sides should work the same as the other components, i.e. passed to the constructor. At some point, we're likely to have some sort of configuration helper from settings, I guess.

@arggh arggh closed this as completed Nov 4, 2018
@arggh arggh reopened this Nov 4, 2018
@arggh
Copy link
Contributor Author

arggh commented Nov 4, 2018

Got confused about the actual reason this issue was opened. Rate limiting (AFAIK) has not yet been implemented, just a Meteor method sender, so I'm re-opening this.

@TLMcNulty
Copy link

I was just reading about all this. I don't know how the project would feel about this, but building this could be worked around with an install script that couple implement traffic shaping with commands for tc or iptables or something.

You're going to be routing your traffic across an IP connection to a specific port right? So sender machines/nodes could be tuned to max TCP/IP traffic to specific port. For elasticseach this is typically 9200 or 9300 I think but we could make that read by a configuration in an install script.

Changes to the how to/use of the program would mean pulling an install script that would take care of the system but it means we'd have to decide what we want people to run this on. Setting up FiLog for docker? Blah blah docker-compose. RPMs? We have to maintain a repo or something. I'm personally messing with this on a straight xenial system and if this gets to be a problem with the performance of my loggers, I'll do some nfconntrack or limit or whatever testing to try to drop blast traffic.

Could actually just do that too, intentionally burst traffic to shape it. Could lead to memory issues on the hosts if the buffers aren't clearing properly and like discarding things that way.

@fgm
Copy link
Owner

fgm commented Feb 26, 2019

The thing is, when you are using the DDP MethodSender, you can activate rate-limiting using the Meteor built-in rate limiter that comes with DDP, as described at https://docs.meteor.com/api/methods.html#ddpratelimiter

So this does solve the rate-limiting, although only for that specific sender. I tend to think this is sufficient, as multi-servers using central logging (which IIRC is your case @arggh ?) can rely on external measures like described by @TLMcNulty without adding more transport-level logic in JS.

Picking your thoughts on this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants