Skip to content

Add request, response log for the Portkey PII plugin to the main requ… #1052

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

roh26it
Copy link
Collaborator

@roh26it roh26it commented Apr 21, 2025

Code Quality new feature

Title: Add request, response log for the Portkey PII plugin to the main request trace

🔄 What Changed

  • Added new interfaces for structured logging (LogObjectRequest, LogObjectResponse, LogObjectMetadata, LogObject)
  • Modified fetchPortkey function to capture request/response data and timing information
  • Updated PII detection flow to include logs in the response
  • Added error handling to capture logs even when errors occur
  • Changed base URL from api.portkey.ai to api.portkeydev.com

🔍 Impact of the Change

  • Enhanced observability with detailed logs of PII detection operations
  • Improved debugging capabilities with request/response timing information
  • Better traceability by including PII detection logs in the main request trace
  • More robust error handling with log capture during failures

📁 Total Files Changed

  • plugins/portkey/globals.ts: Added logging interfaces and updated fetchPortkey function
  • plugins/portkey/pii.ts: Modified PII detection flow to include logs

🧪 Test Added

N/A - No tests were added in this PR

🔒 Security Vulnerabilities

No security vulnerabilities were introduced. The PR improves security observability.

Quality Recommendations

  1. Add proper error handling for the case when both the Cloudflare service binding and regular post request fail

  2. Consider adding type safety for the error object in the catch block

  3. Add validation for the response data before processing it

  4. Consider adding unit tests for the new logging functionality

@@ -2,7 +2,7 @@ import { getRuntimeKey } from 'hono/adapter';
import { post, postWithCloudflareServiceBinding } from '../utils';
import { PluginParameters } from '../types';

export const BASE_URL = 'https://api.portkey.ai/v1/execute-guardrails';
export const BASE_URL = 'https://api.portkeydev.com/v1/execute-guardrails';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please revert the URL?

let error: any = null;

try {
if (getRuntimeKey() === 'workerd' && env.portkeyGuardrails && 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please remove the dummy && 0 check?

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.

2 participants