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

Staging sync #263

Open
wants to merge 20 commits into
base: prod
Choose a base branch
from
Open

Staging sync #263

wants to merge 20 commits into from

Conversation

mohit-nagaraj
Copy link
Collaborator

Summary: What does this PR do?

Which issue(s) this PR fixes:

Changes Made

Describe the changes you've made in this PR:

  • Feature implementation
  • Bug fix
  • Documentation update
  • Code refactoring
  • Configuration changes
  • Other (please specify)

Type of Change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update
  • Performance improvement
  • Code cleanup or refactor

How Has This Been Tested?

Describe the tests you ran:

  • Unit tests
  • Integration tests
  • Manual tests
  • Other (please specify)

Please describe the test cases and expected behavior:
1.
2.

Screenshots (if applicable)

Dependencies

Documentation

  • I have updated the documentation accordingly
  • Documentation update is not required

Comments:

Reviewer Notes

ShreyashSri and others added 20 commits February 12, 2025 15:21
Updated layout.tsx with meta tag for verification
* Updated activities on landing page (#244)

* Fix: Events page header (#245)

* Updated activities (#246)

* Updated cp desc again (#247)

* Added achievements to scrolling animation on main page

---------

Co-authored-by: Rudraksha Singh Sengar <[email protected]>
Co-authored-by: Alfiya Fatima <[email protected]>
Removed robots.txt from root directory
removed sitemap.xml from root directory.
moved files from root directory to public
* images can be directly uploaded to cloudinary

* Improved UI of the forms

* Added Achievements to scrolling animation on main page (#251)

* Updated activities on landing page (#244)

* Fix: Events page header (#245)

* Updated activities (#246)

* Updated cp desc again (#247)

* Added achievements to scrolling animation on main page

---------

Co-authored-by: Rudraksha Singh Sengar <[email protected]>
Co-authored-by: Alfiya Fatima <[email protected]>

---------

Co-authored-by: ShreyashSri <[email protected]>
Co-authored-by: J R VANISHA <[email protected]>
Co-authored-by: Rudraksha Singh Sengar <[email protected]>
@SkySingh04
Copy link
Member

Based on the PR, I'm focusing on instrumenting the new API routes for events, particularly the /api/events/upload/route.ts file which contains significant new functionality. My recommendations include:

High Priority:

  1. Add OpenTelemetry spans to track API request handling in the events upload route
  2. Add error logging with context for error handling blocks
  3. Add event tracking for successful uploads and errors

Medium Priority:

  1. Add attribute enrichment to spans to track metadata about uploads
  2. Add performance measurement for file processing operations

Low Priority:

  1. Add debug logging for request validation steps

Here are my specific recommendations:

FILE: app/(default)/api/events/upload/route.ts
LINE: 10
SUGGESTION:
```diff
// Add after imports or at the beginning of the file
+ import { trace } from '@opentelemetry/api';
+ import { logger } from '@/lib/logger'; // Assuming logger is available
+ import { trackEvent } from '@/lib/analytics'; // Assuming analytics tracking is available
FILE: app/(default)/api/events/upload/route.ts
LINE: 12
SUGGESTION:
```diff
export async function POST(req: Request) {
+  const tracer = trace.getTracer('events-api');
+  const span = tracer.startSpan('events-upload');
+  
+  logger.info('Processing event upload request');
+  
   try {
     const formData = await req.formData();
FILE: app/(default)/api/events/upload/route.ts
LINE: 19
SUGGESTION:
```diff
     if (!file) {
+      logger.warn('No file provided in upload request');
+      span.setStatus({ code: SpanStatusCode.ERROR });
+      span.setAttribute('error.type', 'missing_file');
+      span.end();
+      trackEvent('event_upload_error', { error_type: 'missing_file' });
       return NextResponse.json(
         { success: false, message: "No file provided" },
         { status: 400 }
FILE: app/(default)/api/events/upload/route.ts
LINE: 26
SUGGESTION:
```diff
     if (!file.type.includes("image")) {
+      logger.warn(`Invalid file type: ${file.type}`);
+      span.setStatus({ code: SpanStatusCode.ERROR });
+      span.setAttribute('error.type', 'invalid_file_type');
+      span.setAttribute('file.type', file.type);
+      span.end();
+      trackEvent('event_upload_error', { error_type: 'invalid_file_type', file_type: file.type });
       return NextResponse.json(
         { success: false, message: "File must be an image" },
         { status: 400 }
FILE: app/(default)/api/events/upload/route.ts
LINE: 33
SUGGESTION:
```diff
     const bytes = await file.arrayBuffer();
     const buffer = Buffer.from(bytes);
+    
+    logger.debug('Processing file upload', { 
+      fileName: file.name, 
+      fileSize: buffer.length, 
+      fileType: file.type 
+    });
+    span.setAttribute('file.name', file.name);
+    span.setAttribute('file.size', buffer.length);
+    span.setAttribute('file.type', file.type);
FILE: app/(default)/api/events/upload/route.ts
LINE: 50
SUGGESTION:
```diff
     const uploadResponse = await s3.upload(params).promise();
+    
+    logger.info('File successfully uploaded to S3', { 
+      location: uploadResponse.Location,
+      key: uploadResponse.Key
+    });
+    span.setAttribute('s3.location', uploadResponse.Location);
+    span.setAttribute('s3.key', uploadResponse.Key);
+    trackEvent('event_upload_success', { 
+      file_type: file.type,
+      file_size: buffer.length
+    });
FILE: app/(default)/api/events/upload/route.ts
LINE: 156
SUGGESTION:
```diff
     return NextResponse.json({ success: true, data: uploadResponse });
   } catch (error) {
+    logger.error('Error processing event upload', { 
+      error: error instanceof Error ? error.message : String(error)
+    });
+    span.recordException(error instanceof Error ? error : new Error(String(error)));
+    span.setStatus({ code: SpanStatusCode.ERROR });
+    trackEvent('event_upload_error', { 
+      error_type: 'server_error',
+      error_message: error instanceof Error ? error.message : String(error)
+    });
+    
     console.error("Error uploading file:", error);
     return NextResponse.json(
       { success: false, message: "Error uploading file" },
       { status: 500 }
     );
+  } finally {
+    span.end();
   }
FILE: app/(default)/api/events/route.ts
LINE: 5
SUGGESTION:
```diff
// Add after imports or at the beginning of the file
+ import { trace } from '@opentelemetry/api';
+ import { logger } from '@/lib/logger'; // Assuming logger is available
+ import { trackEvent } from '@/lib/analytics'; // Assuming analytics tracking is available
FILE: app/(default)/api/events/route.ts
LINE: 7
SUGGESTION:
```diff
export async function GET() {
+  const tracer = trace.getTracer('events-api');
+  const span = tracer.startSpan('get-events');
+  
+  logger.info('Processing get events request');
+  
   try {
     const events = [
FILE: app/(default)/api/events/route.ts
LINE: 33
SUGGESTION:
```diff
     return NextResponse.json({ events });
+    
+    span.setAttribute('events.count', events.length);
+    trackEvent('events_fetched', { count: events.length });
+    span.end();
+    
   } catch (error) {
+    logger.error('Error fetching events', { 
+      error: error instanceof Error ? error.message : String(error)
+    });
+    span.recordException(error instanceof Error ? error : new Error(String(error)));
+    span.setStatus({ code: SpanStatusCode.ERROR });
+    span.end();
+    trackEvent('events_fetch_error', { 
+      error_message: error instanceof Error ? error.message : String(error)
+    });
+    
     console.error("Error fetching events:", error);
     return NextResponse.json(
       { error: "Failed to fetch events" },
       { status: 500 }
     );
   }

These recommendations provide a comprehensive observability approach for the new API routes, including:

  1. OpenTelemetry spans to track request flow
  2. Structured logging for debugging and monitoring
  3. Event tracking for business analytics
  4. Error handling with proper context
  5. Performance tracking for critical operations

The instrumentation follows best practices by capturing relevant context, properly handling errors, and ensuring spans are always closed even in error cases.

* Handles file uploads and uploads the file to Cloudinary.
*
* @param {Request} request - The incoming HTTP request
* @returns {Promise<Response>} - A response containing the uploaded image URL or an error message
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @returns {Promise<Response>} - A response containing the uploaded image URL or an error message
import { trace } from '@opentelemetry/api';
import { logger } from '@/lib/logger'; // Assuming logger is available
import { trackEvent } from '@/lib/analytics'; // Assuming analytics tracking is available

* @param {Request} request - The incoming HTTP request
* @returns {Promise<Response>} - A response containing the uploaded image URL or an error message
*/
/**
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/**
const tracer = trace.getTracer('events-api');
const span = tracer.startSpan('events-upload');
logger.info('Processing event upload request');

* summary: Upload an image file to Cloudinary
* description: This endpoint handles image file uploads and uploads the file to Cloudinary.
* tags:
* - Events
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* - Events
logger.warn('No file provided in upload request');
span.setStatus({ code: SpanStatusCode.ERROR });
span.setAttribute('error.type', 'missing_file');
span.end();
trackEvent('event_upload_error', { error_type: 'missing_file' });

* multipart/form-data:
* schema:
* type: object
* properties:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* properties:
logger.warn(`Invalid file type: ${file.type}`);
span.setStatus({ code: SpanStatusCode.ERROR });
span.setAttribute('error.type', 'invalid_file_type');
span.setAttribute('file.type', file.type);
span.end();
trackEvent('event_upload_error', { error_type: 'invalid_file_type', file_type: file.type });

* description: The image file to be uploaded.
* responses:
* 200:
* description: Successfully uploaded the image
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* description: Successfully uploaded the image
logger.debug('Processing file upload', {
fileName: file.name,
fileSize: buffer.length,
fileType: file.type
});
span.setAttribute('file.name', file.name);
span.setAttribute('file.size', buffer.length);
span.setAttribute('file.type', file.type);

* type: object
* properties:
* message:
* type: string
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* type: string
logger.info('File successfully uploaded to S3', {
location: uploadResponse.Location,
key: uploadResponse.Key
});
span.setAttribute('s3.location', uploadResponse.Location);
span.setAttribute('s3.key', uploadResponse.Key);
trackEvent('event_upload_success', {
file_type: file.type,
file_size: buffer.length
});

} catch (parseError) {
// Log and handle errors while parsing form data
console.error("Error parsing form data:", parseError);
return NextResponse.json(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return NextResponse.json(
logger.error('Error processing event upload', {
error: error instanceof Error ? error.message : String(error)
});
span.recordException(error instanceof Error ? error : new Error(String(error)));
span.setStatus({ code: SpanStatusCode.ERROR });
trackEvent('event_upload_error', {
error_type: 'server_error',
error_message: error instanceof Error ? error.message : String(error)
});
} finally {
span.end();

@@ -2,6 +2,7 @@ import { NextResponse } from "next/server";
import Eventmodel from "@/models/Events";
import connectMongoDB from "@/lib/dbConnect";
import { v4 as uuidv4 } from "uuid";
import { cloudinary } from '@/Cloudinary';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { cloudinary } from '@/Cloudinary';
import { trace } from '@opentelemetry/api';
import { logger } from '@/lib/logger'; // Assuming logger is available
import { trackEvent } from '@/lib/analytics'; // Assuming analytics tracking is available

@@ -2,6 +2,7 @@ import { NextResponse } from "next/server";
import Eventmodel from "@/models/Events";
import connectMongoDB from "@/lib/dbConnect";
import { v4 as uuidv4 } from "uuid";
import { cloudinary } from '@/Cloudinary';
/**
* @swagger
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @swagger
const tracer = trace.getTracer('events-api');
const span = tracer.startSpan('get-events');
logger.info('Processing get events request');

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.

8 participants