Datausage plugin

[jeevanrd]

I have started to implement plugin for datausage and coded some basic stuff. Please review it and let me know your thoughts.

[thibaultcha]

I have a lot of questions and observations to make about this:

• Why do you have the changes from @tamizhgeek in your PR? Those changes should be separate.
• If I understand correctly what you described in Metric counter #227 you want 2 things:
  • Be able to increment by X instead of 1
  • Only do this increment in the upstream response (different from current ratelimiting)
• If that is your goal, then that should maybe be implemented as options in the ratelimiting plugin, not be made a new plugin as this duplicates way too much code and makes the configuration too complicated for a user who can get confused by those 2 plugins.
• You cannot edit the existing migrations to add your column family because those are already shipped. If schema changes are needed, you will have to create a new migration, that is the purpose of migrations: they are built on top of each other.

The way I see it, the ratelimiting plugin could have 2 options (probably with better names): increment_in_log (boolean) if true, only increment after upstream was called. That could be a valuable feature.
And response_counter_override_name (string) if set, then ratelimiting reads this header from the upstream response and expects a number that will increment by X instead of 1.

How does that sound? Less confusion as to what each plugin is doing, less code duplication.

[tamizhgeek]

I was not a big fan of mixing up rate-limiting with the data usage counter. There will be(actually we even have a scenarios in our API) scenarios, where you set a API call limit of 10 reqs/second and also set a data limit of only 10,000 records per month. It is then more confusing have different type of limits for the
same plugin.

Also this data usage is very specific to our use case, most people out there would use '# of calls' based ratelimiting only. So better to keep this out of core.

[jeevanrd]

@thibaultcha. Please check my responses inline.

I have a lot of questions and observations to make about this:

Why do you have the changes from @tamizhgeek in your PR? Those changes should be separate.

We have a forked repo (mashape/kong) master branch at our org and started customizing code based on our needs and @tamizhgeek has already merged his changes (PR) to this forked master after which I checked out the code to work on this. That's why you are seeing his changes. May be i can create a fork from your latest master branch and will work on for this.

If I understand correctly what you described in #227 you want 2 things:
Be able to increment by X instead of 1
Only do this increment in the upstream response (different from current ratelimiting)
If that is your goal, then that should maybe be implemented as options in the ratelimiting plugin, not be made a new plugin as this duplicates way too much code and makes the configuration too complicated for a user who can get confused by those 2 plugins.

Doing rate limiting on data/api call should be separate, since there is a lot of complexity involved because it is beast with lots of use cases.
Ex: Even just for ratelimiting on api call, there will be different use cases for which the user want to do ratelimit based on request parameters/headers..etc.

I agree with you that we can modify the same plugin and can achieve the intended behaviour but eventually will ended up with a huge plugin which can become unmanageable.

I agree with @thefosk having micro plugins is good idea which is simple to understand, easy to configure Move the common code into a library and can reuse it.

You cannot edit the existing migrations to add your column family because those are already shipped. If schema changes are needed, you will have to create a new migration, that is the purpose of migrations: they are built on top of each other.

I know about migrations and its purpose. But this was not intentionally done. I have added a migration for this. Looks like something wrong with my merging changes.

The way I see it, the ratelimiting plugin could have 2 options (probably with better names): increment_in_log (boolean) if true, only increment after upstream was called. That could be a valuable feature.
And response_counter_override_name (string) if set, then ratelimiting reads this header from the upstream response and expects a number that will increment by X instead of 1.

How does that sound? Less confusion as to what each plugin is doing, less code duplication.

[thibaultcha]

Then we need to find a way to not duplicate that much code.

[sonicaghi]

Another node @jeevan1986: You will be surprised how many ppl need limits per custom object (you're not an edge case). On Mashape marketplace 30%-40% of limits are custom. Like SMS companies charge per SMS, Video API is by minutes, a Machine Learning API charge per tweet processed, words, etc.

[sonicaghi]

@tamizhgeek @jeevanrd just heads up. We're working on this one.

Data Usage as terminology is not widely recognized in the API mgmt space. Will be: Rate Limiting by Response

[jeevanrd]

@sinzone thanks for keeping posted.

[thibaultcha]

See #493

[jeevanrd]

Thanks!