Log creator and aggregate_id as extra

[jschlyter]

CodeAnt-AI Description

Return 422 for missing signature creator and include creator/aggregate_id in logs

What Changed

• If a create-aggregate request's signature lacks the creator (keyid), the server now responds 422 Unprocessable Entity instead of proceeding.
• Log entries for create requests, duplicate-detection, metadata saves, S3 object creation, and related errors now include structured extras: aggregate_creator and aggregate_id when available.
• Duplicate-aggregate responses and failure paths keep the same HTTP outcomes but now produce clearer logs tied to the creator and aggregate id.

Impact

✅ Clearer signature validation errors
✅ Easier debugging of aggregate uploads
✅ Clearer duplicate-aggregate logs

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

• Bug Fixes

  • Added validation to reject requests with missing creator information, returning HTTP 422 response.

• Chores

  • Enhanced logging and error tracking across critical operations for improved observability and diagnostics.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[codeant-ai]

CodeAnt AI is reviewing your PR.

---

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

[coderabbitai]

Walkthrough

A single file modification introducing creator presence validation (HTTP 422 if keyid missing) and enhanced contextual logging. Logger context containing aggregate_creator and aggregate_id is consistently passed through critical operations: creation request parsing, duplicate detection, metadata save, object creation, and error paths.

Changes

Cohort / File(s)	Summary
Logging & Validation Enhancements aggrec/aggregates.py	Added HTTP 422 validation for missing keyid (creator). Introduced logger_extra context dict with aggregate_creator and aggregate_id. Enhanced all logging statements with contextual extra parameter across creation flow, duplicate detection, metadata operations, object creation, and error handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

• Verify all log statements throughout the creation and error paths have been consistently updated with extra=logger_extra
• Confirm HTTP 422 validation is correctly positioned in the request flow
• Ensure aggregate_id is inserted into logger_extra before subsequent log statements that depend on it
• Check for any missed logging opportunities in edge cases or exception handlers

Poem

🐰 We hop through logs with contextual cheer,
Each aggregate now crystal clear!
Creators validated, IDs all traced,
Our logging journey—gracefully paced. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Log creator and aggregate_id as extra' directly and concisely describes the main change: adding creator and aggregate_id to logger context via the extra parameter.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch log_creator_aggregate_id

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codeant-ai]

Nitpicks 🔍

🔒 No security issues identified

⚡ Recommended areas for review Sensitive Logging The PR adds logging (and tracing span attributes) that include the raw creator ("keyid") and aggregate_id. Depending on what keyid contains, this may expose sensitive identifiers or secrets in logs and telemetry. Verify that creator is safe to log (not a secret or long credential), or redact/hash it before placing it in logger_extra and span attributes. Also ensure any logging/telemetry complies with retention/privacy requirements. Cleanup Failure Risk In the S3 upload error path the code logs the failure, then calls metadata.delete() and raises an HTTPException. If metadata.delete() itself raises (e.g. DB connectivity issues), that will mask the original S3 error and may cause unexpected exceptions during error handling. Consider making the cleanup robust (catch/delete failures and log them) so the original error path is preserved and failure modes are clearer.

[codeant-ai]

CodeAnt AI finished reviewing your PR.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

aggrec/aggregates.py (2)

239-242: Consider logging the existing aggregate ID for better traceability.

When a duplicate is detected, logging the existing aggregate's ID would improve audit trails and debugging capabilities.

🔎 Apply this diff to include the existing aggregate_id:

-        logger.warning("Received duplicate aggregate from %s", creator, extra=logger_extra)
+        logger.warning(
+            "Received duplicate aggregate from %s (existing aggregate_id=%s)", 
+            creator, 
+            metadata.id, 
+            extra=logger_extra
+        )

---

295-349: Optional: Consider adding structured context to remaining log statements.

For complete traceability and consistency, consider adding extra=logger_extra to the remaining log statements (lines 295, 338, 341, 346, 349). This would help correlate all logs related to a single aggregate creation request.

Example for line 295:

-    logger.debug("S3 object metadata: %s", s3_object_metadata)
+    logger.debug("S3 object metadata: %s", s3_object_metadata, extra=logger_extra)

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ddbde19 and ed71df3.

📒 Files selected for processing (1)

• aggrec/aggregates.py (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

aggrec/aggregates.py (1)

aggrec/db_models.py (1)

• AggregateMetadata (12-34)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: test (3.14)

🔇 Additional comments (5)

aggrec/aggregates.py (5)

228-230: LGTM! Appropriate validation for required creator information.

The validation ensures that the creator (keyid) is present before processing, using HTTP 422 for unprocessable entity, which is semantically correct for missing required signature parameters.

---

231-233: LGTM! Structured logging context initialized correctly.

The logger_extra dictionary provides structured context for log aggregation and filtering while maintaining human-readable messages.

---

244-247: LGTM! Aggregate ID correctly added to logging context.

The aggregate_id is added to logger_extra immediately after generation, ensuring all subsequent log statements include both creator and aggregate_id for complete traceability.

---

297-304: LGTM! Database operations logging enhanced with context.

Both success and error paths now include structured logging context while preserving exception information for troubleshooting.

---

306-328: LGTM! S3 operations logging enhanced with context.

Critical S3 operations now include structured logging context with proper exception handling. The error path correctly deletes metadata on S3 failure to maintain consistency.