Skip to content

Introduce "flat_usage" extension#142

Merged
unleashed merged 18 commits intomasterfrom
flat-usage-extension
Jan 7, 2020
Merged

Introduce "flat_usage" extension#142
unleashed merged 18 commits intomasterfrom
flat-usage-extension

Conversation

@unleashed
Copy link
Copy Markdown
Contributor

@unleashed unleashed commented Nov 8, 2019

This introduces the flat_usage extension. It is used to disregard hierarchy information when processing methods/metrics. The assumption is that the client takes responsibility, ie. "I know what I'm doing", on computing how metrics have been affected taking the hierarchy in consideration, and Apisonator just processes the given usage data as if metrics had no parents.

This is very useful for caching, as caches need to report changes in metrics back to Apisonator after some time, but that implies having to undo the work Apisonator will do to add values from children metrics to parent metrics, because such caches already need to compute these relationships to be able to authorize their own clients.

Closes #140.

We could potentially add different levels for the value taken by this extension so we would run sanity checks and refuse to process the usage data if such checks fail, but the potential benefits are marginal given this is mostly meant to be used with reporting calls (which we don't check at request time, and doing this would mean checking at that time to avoid clients flushing their usage data).

PS. we should document the hierarchy extension.
PS2. the name is not really set on stone, suggestions welcome.

Comment thread docs/extensions.md
Comment thread test/integration/authorize/basic_test.rb Outdated
Comment thread test/integration/authorize/basic_test.rb Outdated
Comment thread test/integration/authrep/basic_test.rb
Comment thread test/integration/authorize/basic_test.rb Outdated
Comment thread test/integration/report_test.rb
request_info = context_info['request'.freeze] || {}

transactions = parse_transactions(service_id, raw_transactions, request_info)
ProcessJob.perform(transactions, context_info) if !transactions.nil? && transactions.size > 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The context info sent here is not used. Actually, ProcessJob.perform receives a hash of options but it only cares about whether it's a report for the master account. Maybe we should change that to simply receive a boolean.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was done this way because it follows a similar structure to other jobs' .perform methods, and avoids tying us to the specific usage the job makes of the context info. It's not a big deal, the work has to be done either there or here, but doing it here means knowing about what ProcessJob does inside.

@unleashed unleashed force-pushed the flat-usage-extension branch from 8773a33 to 07052ba Compare January 7, 2020 15:18
@unleashed unleashed requested a review from davidor January 7, 2020 15:18
@unleashed unleashed force-pushed the flat-usage-extension branch from 07052ba to 291f0c5 Compare January 7, 2020 17:15
Comment thread test/integration/report_test.rb Outdated
@unleashed unleashed force-pushed the flat-usage-extension branch from 291f0c5 to 82a86de Compare January 7, 2020 17:16
@davidor
Copy link
Copy Markdown
Contributor

davidor commented Jan 7, 2020

bors r+

bors Bot added a commit that referenced this pull request Jan 7, 2020
142: Introduce "flat_usage" extension r=davidor a=unleashed

This introduces the `flat_usage` extension. It is used to disregard hierarchy information when processing methods/metrics. The assumption is that the client _takes responsibility_, ie. `"I know what I'm doing"`, on computing how metrics have been affected taking the hierarchy in consideration, and Apisonator just processes the given usage data as if metrics had no parents.

This is very _useful for caching_, as caches need to report changes in metrics back to Apisonator after some time, but that implies having to undo the work Apisonator will do to add values from children metrics to parent metrics, because such caches already need to compute these relationships to be able to authorize their own clients.

Closes #140.

We could potentially add different levels for the value taken by this extension so we would run sanity checks and refuse to process the usage data if such checks fail, but the potential benefits are marginal given this is mostly meant to be used with reporting calls (which we don't check at request time, and doing this would mean checking at that time to avoid clients flushing their usage data).

PS. we should document the `hierarchy` extension.
PS2. the name is not really set on stone, suggestions welcome.

Co-authored-by: Alejandro Martinez Ruiz <alex@flawedcode.org>
@unleashed
Copy link
Copy Markdown
Contributor Author

Looks like bors has not recovered? :/

@unleashed unleashed merged commit dacc2a6 into master Jan 7, 2020
@bors bors Bot deleted the flat-usage-extension branch January 7, 2020 17:35
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Jan 7, 2020

Timed out

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

Successfully merging this pull request may close these issues.

Extension to report metrics without taking hierarchies into account

2 participants