Add Usage Tracking feature with OpenAI support#1060
Add Usage Tracking feature with OpenAI support#1060rahulsprajapati wants to merge 19 commits intodevelopfrom
Conversation
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
There was a problem hiding this comment.
Left a decent number of comments here that need addressed.
That said, I still don't know if this PR matches my expectations in terms of extensibility (same comment I had on the first PR).
If I wanted to add usage tracking for another Provider, I'd have to duplicate basically all the code here. I still think there's a better approach where we can extract out all of the shared functionality, making it much easier and cleaner to add support in the future.
I'm also not sure on the use of our Feature and Provider classes here. I know that's how the rest of ClassifAI is set up but I don't think these things really fit neatly into that paradigm. Doesn't necessarily need changed but something to consider.
Edit: at the very least, I think if we want to follow this structure we need to make the Usage Tracking Feature generic, it should not be tied to OpenAI at all (this is how all other Features work). Any OpenAI specific things can then be in the Usage Provider. I'd also recommend we create a general Usage Provider that contains all shared functionality. Then the OpenAI specific Usage Provider can extend that (or similar)
Second edit: Thinking about this some more, the above wouldn't quite work as we only support a single Feature -> Provider setup, where here ideally we can turn on usage tracking for multiple Providers. So I go back to my original thought that not sure our standard Feature / Provider setup works for this, at least not without some changes to how we render the settings and allow things to be turned on and configured
| * @return array|WP_Error | ||
| */ | ||
| public function get( string $url, array $options = [] ) { | ||
| if ( ! $this->is_request_allowed() ) { |
There was a problem hiding this comment.
See my comment from the other PR: #1054 (comment). I know we have a new check using OpenAIUsage::get_openai_provider_ids but I'd still suggest this sort of disabling shouldn't happen here as this class won't exist much longer
There was a problem hiding this comment.
Let's discuss this over sync. Not sure what other possible solution for this. Disabling feature all together seems confusing for end users.
| const CRON_HOOK = 'classifai_openai_usage_refresh'; | ||
|
|
||
| /** | ||
| * Cron hook for the usage refresh. |
There was a problem hiding this comment.
This is the same docblock as the one above. Should update these to be unique
| switch ( $this->feature_instance::ID ) { | ||
| case FeatureOpenAIUsage::ID: | ||
| return array_merge( | ||
| $common_settings, | ||
| [ | ||
| 'api_key' => '', | ||
| ] | ||
| ); | ||
| } |
There was a problem hiding this comment.
This is redundant right now so I'd remove it
| * | ||
| * @return bool | ||
| */ | ||
| $allowed = (bool) apply_filters( 'classifai_can_make_request', true, $this->feature, $this->provider ); |
There was a problem hiding this comment.
| $allowed = (bool) apply_filters( 'classifai_can_make_request', true, $this->feature, $this->provider ); | |
| $allowed = (bool) apply_filters( 'classifai_openai_can_make_request', true, $this->feature, $this->provider ); |
| if ( | ||
| strpos( $route, '/classifai/v1/openai-usage/force-refresh' ) !== 0 | ||
| || ! function_exists( 'as_enqueue_async_action' ) | ||
| || ( | ||
| function_exists( 'as_has_scheduled_action' ) | ||
| && \as_has_scheduled_action( self::FORCE_CRON_HOOK, [], 'classifai' ) | ||
| ) | ||
| ) { | ||
| return parent::rest_endpoint_callback( $request ); | ||
| } |
There was a problem hiding this comment.
Not sure I understand what this logic is trying to do? Particularly in the case where the action is already scheduled, why do we need to call the parent method?
I tried using that approach, but with that it will be whole different structure compare to current ClassifAI structure. Because of that we don't get to reuse same UI/UX and need lots of changes to make UI/UX match current one. So I tried to use existing Provider + Feature classes, which help reuse existing UI/UX. Maybe we need to separate out UI/UX code from current structure and allow any new/extend feature with same UI/UX without having to reuse Provider+Feature structure.
I think since I'm not sure what other provide feature looks like (how we get usage data, do we need to calculate similar way or get it directly from one endpoint, or require diff admin api key only) because of that I'm unsure to keep which feature/code shared. Like we can have fetch_period method shared but again not sure if we will have same type of api calls. So we can have interface without defined method but not sure what else we can do here.
For this, looks like we might need full refactor on plugin itself. Since without using this Feature and Provider we don't get to reuse existing UI/UX and require lots of changes in style and scripts. About the blocking direct api calls:
But let me know what you think. If this doesn't make sense I can move this block to disable feature itself or whichever other place we can do this. I'll update other changes and setup a sync to discuss more on this and get this feature ready asap. |
Co-authored-by: Darin Kotter <darin.kotter@gmail.com>
Co-authored-by: Darin Kotter <darin.kotter@gmail.com>
Co-authored-by: Darin Kotter <darin.kotter@gmail.com>
Co-authored-by: Darin Kotter <darin.kotter@gmail.com>
Co-authored-by: Darin Kotter <darin.kotter@gmail.com>
Co-authored-by: Darin Kotter <darin.kotter@gmail.com>
Co-authored-by: Darin Kotter <darin.kotter@gmail.com>
| */ | ||
| public static function get_service_providers(): array { | ||
| /** | ||
| * Filter the service providers for Recommendation service. |
There was a problem hiding this comment.
This isn't accurate, referencing the Recommendation service
Description of the Change
This PR adds OpenAI Usage Tracking to ClassifAI: the ability to monitor usage/costs from the OpenAI API and enforce soft (alert-only) and hard (disable features) spending limits.
Design overview:
UsageTrackingservice andOpenAIUsagefeature are introduced, with providerOpenAI\UsageTrackingthat calls the OpenAI organization costs API.Soft threshold — when usage exceeds the configured amount for the chosen scope, a dismissible admin notice is shown and an optional email can be sent (once per period).
Hard threshold — when exceeded, an option is set, all OpenAI API requests are blocked in
APIRequest::get/post/post_form(returning aWP_Error), and a non-dismissible error notice is shown with a link to re-enable from the pricing/usage page. Email can be sent once per period.Plugin.phpregisters the Usage Tracking service and OpenAI Usage feature and addsopenai_usage_trackingsupport to load the Action Scheduler.APIRequestchecksis_request_allowed()(which respects the hard-limit option and theclassifai_can_make_requestfilter) before anyget/post/post_formcall for OpenAI providers.Notifications.phpgainsrender_openai_threshold_notice()so admins see soft/hard threshold warnings in the admin.Verification:
/classifai/v1/openai-usage/force-refreshis only allowed when the feature is enabled.Benefits: Visibility into OpenAI spend (MTD/YTD/all time), alerts and optional automatic disabling to avoid cost overruns, and one email per period per threshold to avoid spam.
Possible drawback / note: Uses an organization-level Admin API key; docs should clarify this is required and separate from project keys.
Closes #122 (comment)
How to test the Change
Changelog Entry
Credits
Props @rahulsprajapati
Checklist: