Ofrich/add logs to multitenant violations

[ofrichen]

User description

Added warn logs on tenant changes, with an ability to allow tenant changes, to be used in the beginning of flows (for example - when an HTTP request comes in)

---

PR Type

Enhancement

---

Description

• Add logging for multitenant violations with sampling

• Implement skip mechanism for allowed operations

• Add comprehensive test coverage for logging

---

Diagram Walkthrough

flowchart LR
  A["Tenant Operation"] --> B["Check Skip Flag"]
  B --> C{Skip Flag Set?}
  C -->|Yes| D["Clear Flag & Skip Log"]
  C -->|No| E["Check Sample Rate"]
  E --> F{Should Log?}
  F -->|Yes| G["Log Violation"]
  F -->|No| H["Continue Silently"]

Loading

File Walkthrough

	Relevant files
Enhancement	multitenant.rb Add multitenant violation logging system                                  lib/multitenant.rb Add configurable sample rate for violation logging Implement allow_next_tenant_operation skip mechanism Add logging to tenant change operations Extract private methods for thread-local setters +69/-6
Tests	multitenant_spec.rb Add comprehensive logging tests                                                    spec/multitenant_spec.rb Add comprehensive test suite for logging functionality Test skip mechanism behavior and flag consumption Verify logging for all tenant operations Mock logger setup and teardown +172/-0

---

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Ensure logger compatibility The code assumes a global $logger that accepts keyword arguments, which will break with standard Ruby Logger or other loggers and silently drop logs due to the rescue path. Provide a configurable logger (e.g., Multitenant.logger defaulting to Rails.logger/Logger) and a single logging helper that detects structured logging support and falls back to plain-string logging when unsupported, rather than calling $logger.warn/error with keyword args directly. This prevents runtime errors and ensures violations are consistently recorded across environments. Examples: lib/multitenant.rb [103-121] def log_multitenant_violation_if_needed(kind) if Thread.current[ALLOW_NEXT_TENANT_OPERATION] Thread.current[ALLOW_NEXT_TENANT_OPERATION] = false return end return unless Random.rand < Multitenant.multitenant_violation_log_sample_rate $logger.warn( tag: 'multitenant_violation', ... (clipped 9 lines) Solution Walkthrough: Before: # lib/multitenant.rb def log_multitenant_violation_if_needed(kind) # ... # Assumes $logger supports keyword arguments, which is not standard. $logger.warn( tag: 'multitenant_violation', message: 'multitenant usage outside allowed contexts', kind: kind, ) rescue => e begin # This error logging also assumes the same non-standard interface. $logger.error(tag: 'multitenant_violation', message: 'error while logging', error: e.message) rescue # The violation is silently ignored if the logger is incompatible. end end After: # lib/multitenant.rb class << self # Allow the logger to be configured. attr_writer :logger def logger @logger ||= (defined?(Rails) ? Rails.logger : Logger.new($stdout)) end end private def log_multitenant_violation_if_needed(kind) # ... payload = { ... } # Use a robust logging helper. _log_warning(payload) end def _log_warning(payload) # Detect if the configured logger supports structured logging. if logger_supports_kwargs?(Multitenant.logger) Multitenant.logger.warn(**payload) else # Fall back to a plain string format if not supported. Multitenant.logger.warn(payload.to_s) end end Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical design flaw where the new logging feature relies on a non-standard logger interface, causing it to fail silently in many environments and undermining the PR's main purpose.	High
Possible issue	Log tenant clears too Also log when clearing the tenant context. Setting the tenant to nil outside approved wrappers can be a violation and should be captured. This avoids missing critical state changes. lib/multitenant.rb [32-38] def current_tenant=(value) - if current_tenant != value && value != nil + if current_tenant != value log_multitenant_violation_if_needed('current_tenant_set') end _set_current_tenant(value) end Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that clearing the tenant by setting it to nil is not logged, while it should be considered a violation if done outside of approved wrappers. Applying this change would make the violation detection more comprehensive.	Medium
General	Safely guard logger calls Guard against nil or incompatible loggers before calling. This avoids raising and rescuing exceptions on every call and ensures error logging only if supported. lib/multitenant.rb [103-121] def log_multitenant_violation_if_needed(kind) if Thread.current[ALLOW_NEXT_TENANT_OPERATION] Thread.current[ALLOW_NEXT_TENANT_OPERATION] = false return end return unless Random.rand < Multitenant.multitenant_violation_log_sample_rate - $logger.warn( + logger = $logger + return unless logger && logger.respond_to?(:warn) + + logger.warn( tag: 'multitenant_violation', message: 'multitenant usage outside allowed contexts', kind: kind, ) rescue => e begin - $logger.error(tag: 'multitenant_violation', message: 'error while logging multitenant violation', error: e.message) + logger = $logger + logger.error(tag: 'multitenant_violation', message: 'error while logging multitenant violation', error: e.message) if logger && logger.respond_to?(:error) rescue end end Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion improves code robustness by checking if $logger exists and can respond to :warn or :error before being called, avoiding inefficient exception handling if the logger is not configured.	Medium
	Clamp logging sample rate Clamp the sample rate to [0.0, 1.0]. This prevents misconfiguration from disabling logging or flooding logs unexpectedly. lib/multitenant.rb [16-18] def set_multitenant_violation_log_sample_rate(sample_rate) - @@multitenant_violation_log_sample_rate = sample_rate + rate = sample_rate.to_f + rate = 0.0 if rate < 0.0 + rate = 1.0 if rate > 1.0 + @@multitenant_violation_log_sample_rate = rate end Apply / Chat Suggestion importance[1-10]: 6 __ Why: This is a good defensive programming suggestion that makes the set_multitenant_violation_log_sample_rate method more robust by ensuring the sample_rate is always within the valid range of [0.0, 1.0], preventing unexpected logging behavior from misconfiguration.	Low
Update