brooktrails/forgejo-runner-operator
2
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

feat: send structured logs to GlitchTip via Sentry Logs API #10

Merged
rcsheets merged 1 commit from feat/sentry-logs into master 2026-03-26 01:53:54 +00:00
Conversation 2 Commits 1 Files changed 4 +42
rcsheets commented 2026-03-26 01:50:57 +00:00
Owner
Copy link

Enable Sentry Logs (EnableLogs: true) in both admin-registrar and
controller. Add sentryLog helper and emit info-level logs at each
handler lifecycle step (user created, collaborator added, token
created, credentials stored, etc.) alongside existing stdout logging.

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

Enable Sentry Logs (EnableLogs: true) in both admin-registrar and controller. Add sentryLog helper and emit info-level logs at each handler lifecycle step (user created, collaborator added, token created, credentials stored, etc.) alongside existing stdout logging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
feat: send structured logs to GlitchTip via Sentry Logs API
Some checks failed
CI / ci (push) Has been cancelled
Details
CI / ci (pull_request) Has been cancelled
Details
E2E smoke test / e2e (pull_request) Has been cancelled
Details
44b75d31ac 
Enable Sentry Logs (EnableLogs: true) in both admin-registrar and
controller. Add sentryLog helper and emit info-level logs at each
handler lifecycle step (user created, collaborator added, token
created, credentials stored, etc.) alongside existing stdout logging.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pr-reviewer-bot commented 2026-03-26 01:50:59 +00:00
Collaborator
Copy link

Automated review by pr-reviewer v0.15.0 | Full Review | anthropic | tracking id r-c49102-502434
This is an AI-generated review and may contain mistakes.

Status: ✅ Completed

✅ Verdict: Approve — The changes are well-implemented with proper error handling and consistent logging patterns.
Complexity: moderate

Code Review: Enable Sentry Logs API Integration

This PR successfully adds structured logging to GlitchTip via the Sentry Logs API. The implementation is clean, consistent, and follows good practices.

Positive Aspects

Clean Implementation

  • Proper configuration: Both services enable Sentry logs with EnableLogs: true in their client initialization
  • Helper function: The sentryLog helper provides a clean abstraction for sending structured logs with proper level handling and no-op behavior when Sentry isn't configured
  • Consistent usage: Logs are emitted at appropriate lifecycle steps alongside existing stdout logging, providing good operational visibility

Error Handling & Safety

  • Null safety: All Sentry operations properly check if the client is initialized before attempting operations
  • Graceful degradation: When Sentry is not configured, the logging calls are no-ops that don't impact functionality
  • Proper level mapping: The switch statement in sentryLog handles all Sentry log levels correctly with a sensible default

Code Quality

  • Good separation of concerns: Sentry-specific logic is isolated in internal/registrar/sentry.go
  • Consistent patterns: The logging calls follow the same pattern throughout the handler methods
  • Appropriate context: Tags include relevant metadata (owner, repo, username, action) for effective log filtering and analysis

Architecture & Design

The approach of adding structured logs at each handler step is sound:

  • Repository creation/deletion events are tracked end-to-end
  • Each major operation (user creation, collaborator addition, token creation, credential storage) gets logged
  • Error contexts are preserved with the existing captureError function

Minor Observations

No Major Issues

  • The hardcoded TracesSampleRate: 1.0 in controller/main.go is acceptable for this use case
  • No breaking changes introduced
  • The implementation is consistent with existing Sentry integration patterns

Test Coverage

While there are no tests for the new functionality, this is consistent with the existing codebase pattern. The Sentry logging is primarily observability infrastructure that's difficult to unit test meaningfully.

Summary

This is a solid implementation that adds valuable operational visibility without introducing complexity or risk. The structured logs will provide good insights into the admin-registrar workflow execution, which should help with debugging and monitoring.

The changes are backwards compatible and degrade gracefully when Sentry is not configured, making this a safe addition to the system.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.15.0 | Full Review | anthropic | tracking id `r-c49102-502434`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- **✅ Verdict: Approve** — The changes are well-implemented with proper error handling and consistent logging patterns. **Complexity:** moderate # Code Review: Enable Sentry Logs API Integration This PR successfully adds structured logging to GlitchTip via the Sentry Logs API. The implementation is clean, consistent, and follows good practices. ## Positive Aspects ### Clean Implementation - **Proper configuration**: Both services enable Sentry logs with `EnableLogs: true` in their client initialization - **Helper function**: The `sentryLog` helper provides a clean abstraction for sending structured logs with proper level handling and no-op behavior when Sentry isn't configured - **Consistent usage**: Logs are emitted at appropriate lifecycle steps alongside existing stdout logging, providing good operational visibility ### Error Handling & Safety - **Null safety**: All Sentry operations properly check if the client is initialized before attempting operations - **Graceful degradation**: When Sentry is not configured, the logging calls are no-ops that don't impact functionality - **Proper level mapping**: The switch statement in `sentryLog` handles all Sentry log levels correctly with a sensible default ### Code Quality - **Good separation of concerns**: Sentry-specific logic is isolated in `internal/registrar/sentry.go` - **Consistent patterns**: The logging calls follow the same pattern throughout the handler methods - **Appropriate context**: Tags include relevant metadata (owner, repo, username, action) for effective log filtering and analysis ## Architecture & Design The approach of adding structured logs at each handler step is sound: - Repository creation/deletion events are tracked end-to-end - Each major operation (user creation, collaborator addition, token creation, credential storage) gets logged - Error contexts are preserved with the existing `captureError` function ## Minor Observations ### No Major Issues - The hardcoded `TracesSampleRate: 1.0` in controller/main.go is acceptable for this use case - No breaking changes introduced - The implementation is consistent with existing Sentry integration patterns ### Test Coverage While there are no tests for the new functionality, this is consistent with the existing codebase pattern. The Sentry logging is primarily observability infrastructure that's difficult to unit test meaningfully. ## Summary This is a solid implementation that adds valuable operational visibility without introducing complexity or risk. The structured logs will provide good insights into the admin-registrar workflow execution, which should help with debugging and monitoring. The changes are backwards compatible and degrade gracefully when Sentry is not configured, making this a safe addition to the system.
pr-reviewer-bot commented 2026-03-26 01:51:56 +00:00
Collaborator
Copy link

Automated review by pr-reviewer v0.15.0 | Full Review | anthropic | tracking id r-c4913b-a17697
This is an AI-genrated review and may contain mistakes.

Status: ✅ Completed

✅ Verdict: Approve — Clean implementation that adds structured logging to Sentry with proper safeguards and no breaking changes.
Complexity: trivial

Code Review: feat: send structured logs to GlitchTip via Sentry Logs API

This PR adds structured logging to GlitchTip/Sentry by enabling the Logs feature and adding info-level log events at key handler lifecycle steps. The implementation is clean and well-thought-out.

✅ Positive Aspects

  1. Clean Implementation: The sentryLog helper function is well-designed with proper level handling and attribute attachment
  2. Safety First: All Sentry operations include null checks (CurrentHub().Client() == nil) to prevent crashes when Sentry is disabled
  3. Strategic Placement: Log events are placed at meaningful lifecycle points (user created, collaborator added, token created, etc.)
  4. Consistent Pattern: Uses the same sentryTags map across all log calls within each handler method
  5. No Breaking Changes: Only additive changes that won't affect existing deployments
  6. Idempotent Design: Maintains the existing idempotent behavior of the handlers

📝 Code Quality

internal/registrar/sentry.go

  • The sentryLog function properly handles all Sentry log levels with a sensible default fallback
  • Good use of the builder pattern for constructing log entries
  • Early return pattern keeps the code clean when Sentry is disabled

Handler Integration (internal/registrar/handler.go)

  • Log calls are placed immediately after successful operations, providing good visibility
  • Uses existing sentryTags for consistency
  • Doesn't interfere with existing audit logging or stdout logging

Configuration Changes (main.go files)

  • Simple one-line addition (EnableLogs: true) to existing Sentry configuration
  • Maintains backward compatibility

🔍 Technical Considerations

  1. Performance Impact: Minimal - log calls only execute when Sentry is configured and include early returns
  2. Error Handling: Proper - all Sentry operations are wrapped with safety checks
  3. Context Propagation: Correctly uses the provided context for log operations
  4. Memory Usage: Efficient - reuses existing tag maps without unnecessary allocations

📊 Missing Test Coverage

While the diff shows no tests were added, this is acceptable for this change because:

  • The functionality is primarily about external service integration (Sentry/GlitchTip)
  • The code includes proper null checks making it safe to call even when unconfigured
  • The logging is supplementary to existing functionality and doesn't change business logic

🎯 Recommendations

Consider adding a simple test to verify the sentryLog function handles the nil client case gracefully, but this is not blocking for merge.

Summary

This is a well-implemented feature that adds valuable observability without introducing risk. The structured logs will provide better visibility into the admin-registrar workflow in production environments while maintaining all existing safeguards.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.15.0 | Full Review | anthropic | tracking id `r-c4913b-a17697`* *This is an AI-genrated review and may contain mistakes.* **Status:** ✅ Completed --- **✅ Verdict: Approve** — Clean implementation that adds structured logging to Sentry with proper safeguards and no breaking changes. **Complexity:** trivial # Code Review: feat: send structured logs to GlitchTip via Sentry Logs API This PR adds structured logging to GlitchTip/Sentry by enabling the Logs feature and adding info-level log events at key handler lifecycle steps. The implementation is clean and well-thought-out. ## ✅ Positive Aspects 1. **Clean Implementation**: The `sentryLog` helper function is well-designed with proper level handling and attribute attachment 2. **Safety First**: All Sentry operations include null checks (`CurrentHub().Client() == nil`) to prevent crashes when Sentry is disabled 3. **Strategic Placement**: Log events are placed at meaningful lifecycle points (user created, collaborator added, token created, etc.) 4. **Consistent Pattern**: Uses the same `sentryTags` map across all log calls within each handler method 5. **No Breaking Changes**: Only additive changes that won't affect existing deployments 6. **Idempotent Design**: Maintains the existing idempotent behavior of the handlers ## 📝 Code Quality ### internal/registrar/sentry.go - The `sentryLog` function properly handles all Sentry log levels with a sensible default fallback - Good use of the builder pattern for constructing log entries - Early return pattern keeps the code clean when Sentry is disabled ### Handler Integration (internal/registrar/handler.go) - Log calls are placed immediately after successful operations, providing good visibility - Uses existing `sentryTags` for consistency - Doesn't interfere with existing audit logging or stdout logging ### Configuration Changes (main.go files) - Simple one-line addition (`EnableLogs: true`) to existing Sentry configuration - Maintains backward compatibility ## 🔍 Technical Considerations 1. **Performance Impact**: Minimal - log calls only execute when Sentry is configured and include early returns 2. **Error Handling**: Proper - all Sentry operations are wrapped with safety checks 3. **Context Propagation**: Correctly uses the provided context for log operations 4. **Memory Usage**: Efficient - reuses existing tag maps without unnecessary allocations ## 📊 Missing Test Coverage While the diff shows no tests were added, this is acceptable for this change because: - The functionality is primarily about external service integration (Sentry/GlitchTip) - The code includes proper null checks making it safe to call even when unconfigured - The logging is supplementary to existing functionality and doesn't change business logic ## 🎯 Recommendations Consider adding a simple test to verify the `sentryLog` function handles the nil client case gracefully, but this is not blocking for merge. ## Summary This is a well-implemented feature that adds valuable observability without introducing risk. The structured logs will provide better visibility into the admin-registrar workflow in production environments while maintaining all existing safeguards.
rcsheets merged commit d1f828e811 into master 2026-03-26 01:53:54 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
pr-reviewer-bot
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
brooktrails/forgejo-runner-operator!10
No description provided.